diff --git a/.github/workflows/extralit-frontend.yml b/.github/workflows/extralit-frontend.yml index 89e5187fc..818359cd7 100644 --- a/.github/workflows/extralit-frontend.yml +++ b/.github/workflows/extralit-frontend.yml @@ -41,11 +41,6 @@ jobs: run: | npm install - - name: Check generated v2 API types are current πŸ”’ - run: | - npm run gen:api:types - git diff --exit-code -- v2/infrastructure/api/generated - - name: Run lint 🧹 continue-on-error: true run: | diff --git a/.github/workflows/extralit-server.yml b/.github/workflows/extralit-server.yml index f0a84681d..1b5f736f6 100644 --- a/.github/workflows/extralit-server.yml +++ b/.github/workflows/extralit-server.yml @@ -15,7 +15,6 @@ on: - releases/** paths: - "extralit-server/**" - - "extralit-frontend/v2/infrastructure/api/**" permissions: id-token: write @@ -97,12 +96,6 @@ jobs: - name: Install dependencies run: uv sync --dev --extra postgresql - - name: Check frontend v2 OpenAPI snapshot is current πŸ”’ - run: | - uv run python -m extralit_server.cli openapi-dump --output /tmp/openapi-v2.json - diff -u ../extralit-frontend/v2/infrastructure/api/openapi.json /tmp/openapi-v2.json \ - || { echo "::error::v2 OpenAPI drift β€” run 'npm run gen:api' in extralit-frontend and commit"; exit 1; } - - name: Run tests πŸ“ˆ id: run-tests continue-on-error: true diff --git a/docs/superpowers/plans/2026-06-27-schema-registry-and-versioning.md b/docs/superpowers/plans/2026-06-27-schema-registry-and-versioning.md index 5df7ad48a..5015f541f 100644 --- a/docs/superpowers/plans/2026-06-27-schema-registry-and-versioning.md +++ b/docs/superpowers/plans/2026-06-27-schema-registry-and-versioning.md @@ -1,5 +1,7 @@ # Schema Registry & Versioning Implementation Plan (Phase 1 of 6) +> **Historical note (2026-07-26):** The `/api/v2` parallel tree described in this document was folded back into `/api/v1`. See `docs/superpowers/plans/2026-07-26-fold-v2-into-v1.md`. This document is kept as a historical record; its API paths, models, and file references may no longer exist. +> > **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. **Goal:** Stand up the `Schema` entity (the v2 "Dataset") and its object-store-backed, versioned Pandera body β€” schema CRUD, version publishing, derived column cache, and server-side validation β€” as an isolated `/api/v2` module alongside untouched v1. diff --git a/docs/superpowers/plans/2026-07-03-v2-records.md b/docs/superpowers/plans/2026-07-03-v2-records.md index f860a88ad..d5de7d6de 100644 --- a/docs/superpowers/plans/2026-07-03-v2-records.md +++ b/docs/superpowers/plans/2026-07-03-v2-records.md @@ -1,5 +1,7 @@ # v2 Records Implementation Plan (Phase 2 of 6) +> **Historical note (2026-07-26):** The `/api/v2` parallel tree described in this document was folded back into `/api/v1`. See `docs/superpowers/plans/2026-07-26-fold-v2-into-v1.md`. This document is kept as a historical record; its API paths, models, and file references may no longer exist. +> > **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Executed 2026-07-03 on branch `feat/v2-records` (stacked on `feat/v2-schema-registry`, PR #219). **Goal:** Add the v2 `record` entity β€” one typed row pinned to a `schema_version`, carrying the cross-schema `reference` join key and Pandera-validated `fields` JSONB β€” with a validated bulk-upsert, paginated listing, bulk delete, and the `GET /references/{reference}` cross-schema document view (spec Β§5 record row, Β§6 write flow, Β§7 API surface). diff --git a/docs/superpowers/plans/2026-07-07-v2-lancedb-index.md b/docs/superpowers/plans/2026-07-07-v2-lancedb-index.md index 60b2c120c..5dbca9010 100644 --- a/docs/superpowers/plans/2026-07-07-v2-lancedb-index.md +++ b/docs/superpowers/plans/2026-07-07-v2-lancedb-index.md @@ -1,5 +1,7 @@ # v2 LanceDB Index Engine Implementation Plan (Phase 3 of 6) +> **Historical note (2026-07-26):** The `/api/v2` parallel tree described in this document was folded back into `/api/v1`. See `docs/superpowers/plans/2026-07-26-fold-v2-into-v1.md`. This document is kept as a historical record; its API paths, models, and file references may no longer exist. +> > **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. **Goal:** Add a LanceDB-backed index engine that derives one Lance table per v2 schema from Postgres, exposing full-text (BM25) + scalar-filter search over records via `POST /api/v2/schemas/{id}/records:search`, kept in sync best-effort on write and rebuildable from Postgres. diff --git a/docs/superpowers/plans/2026-07-08-v2-annotation.md b/docs/superpowers/plans/2026-07-08-v2-annotation.md index 7ae662e83..719220b2c 100644 --- a/docs/superpowers/plans/2026-07-08-v2-annotation.md +++ b/docs/superpowers/plans/2026-07-08-v2-annotation.md @@ -1,5 +1,7 @@ # v2 Annotation (Phase 4) Implementation Plan +> **Historical note (2026-07-26):** The `/api/v2` parallel tree described in this document was folded back into `/api/v1`. See `docs/superpowers/plans/2026-07-26-fold-v2-into-v1.md`. This document is kept as a historical record; its API paths, models, and file references may no longer exist. +> > **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. **Goal:** Build the schema-centric annotation layer β€” questions (reviewable column bindings), suggestions (LLM pre-populated values), responses (human submissions), and a query-time projection view that resolves each reviewable cell β€” on top of the v2 records built in Phases 1–3. diff --git a/docs/superpowers/plans/2026-07-10-v2-frontend-vertical-slice.md b/docs/superpowers/plans/2026-07-10-v2-frontend-vertical-slice.md index 3c306401a..2f835c705 100644 --- a/docs/superpowers/plans/2026-07-10-v2-frontend-vertical-slice.md +++ b/docs/superpowers/plans/2026-07-10-v2-frontend-vertical-slice.md @@ -1,5 +1,7 @@ # v2 Frontend Vertical Slice Implementation Plan +> **Historical note (2026-07-26):** The `/api/v2` parallel tree described in this document was folded back into `/api/v1`. See `docs/superpowers/plans/2026-07-26-fold-v2-into-v1.md`. This document is kept as a historical record; its API paths, models, and file references may no longer exist. +> > **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. **Spec:** `docs/superpowers/specs/2026-07-09-v2-frontend-vertical-slice-design.md` (parent: `2026-06-27-schema-centric-data-model-design.md`) diff --git a/docs/superpowers/plans/2026-07-13-sdk-v2-vertical-slice.md b/docs/superpowers/plans/2026-07-13-sdk-v2-vertical-slice.md index 686cb4201..65c3040ef 100644 --- a/docs/superpowers/plans/2026-07-13-sdk-v2-vertical-slice.md +++ b/docs/superpowers/plans/2026-07-13-sdk-v2-vertical-slice.md @@ -1,5 +1,7 @@ # Python SDK v2 Vertical Slice Implementation Plan +> **Historical note (2026-07-26):** The `/api/v2` parallel tree described in this document was folded back into `/api/v1`. See `docs/superpowers/plans/2026-07-26-fold-v2-into-v1.md`. This document is kept as a historical record; its API paths, models, and file references may no longer exist. +> > **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. **Goal:** Build the parallel `extralit.v2` SDK package (generated DTOs from the server's `openapi-dump` snapshot, async-native transport + sync facade, five resources for the extraction loop) and top-level agentic CLI verbs (JSON-first) that replace the v1 `schemas` subcommand. diff --git a/docs/superpowers/plans/2026-07-20-extraction-table.md b/docs/superpowers/plans/2026-07-20-extraction-table.md index 593a51392..79f178dec 100644 --- a/docs/superpowers/plans/2026-07-20-extraction-table.md +++ b/docs/superpowers/plans/2026-07-20-extraction-table.md @@ -1,5 +1,7 @@ # Extraction Table Implementation Plan +> **Historical note (2026-07-26):** The `/api/v2` parallel tree described in this document was folded back into `/api/v1`. See `docs/superpowers/plans/2026-07-26-fold-v2-into-v1.md`. This document is kept as a historical record; its API paths, models, and file references may no longer exist. +> > **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. **Goal:** Ship a workspace-level denormalized extraction table β€” new `GET /api/v2/projection` endpoint with enriched provenance cells, a `/extractions` page rendered by Perspective 4.x, an additive `list[dict]` table-value contract, and deletion of the superseded reference-review page. diff --git a/docs/superpowers/plans/2026-07-26-fold-followups.md b/docs/superpowers/plans/2026-07-26-fold-followups.md new file mode 100644 index 000000000..f6fed410e --- /dev/null +++ b/docs/superpowers/plans/2026-07-26-fold-followups.md @@ -0,0 +1,474 @@ +# Fold v2-into-v1 β€” Deferred Follow-ups + +> Source: final whole-branch review of `feat/ENG-36-server-v2-to-v1` (base plan +> `docs/superpowers/plans/2026-07-26-fold-v2-into-v1.md`), plus the `minor (deferred)` items +> logged throughout `.superpowers/sdd/2026-07-26-fold-v2-into-v1/progress.md`. Nothing here +> blocks merge β€” the two Critical and the Important-3/5 findings from that review were fixed +> in the same commit(s) that added this file. This is the punch list of what was deliberately +> left for later, with enough context to act on each item without re-deriving it. +> +> **Update (2026-07-28):** the 19 open roborev reviews on this branch (jobs 283–301, one per +> commit) were triaged and closed. Roughly half of their findings were already stale β€” fixed by +> a later commit on the same branch. Of the rest, six were fixed in the roborev pass (see +> Β§0), two Highs need a product decision and are recorded as Β§7 and Β§8, and the remainder are +> folded into Β§9 below. Section 1's description of the republish consequence was wrong and has +> been corrected. + +## 0. Fixed in the 2026-07-28 roborev pass (no longer open) + +For the record, so these are not re-derived from the closed reviews: + +- **`SchemaQuestion` rename** (job 297): the fold left two `export class Question` and two + incompatible `QuestionType` symbols side by side under `v1/domain/entities/` β€” the + annotation one (`question/QuestionType.ts`, a `class extends String`) and the schema one + (`schema/Question.ts`, a string union). Renamed to `SchemaQuestion`/`SchemaQuestionType`/ + `SchemaQuestionOption` in `schema/SchemaQuestion.ts`, matching the `SchemaRecord` precedent, + and updated the three importers. +- **Republish now emits `dataset.updated`** (job 301): gating `published` on the draftβ†’ready + transition left a republish emitting *nothing*, so a consumer never learned versions 2..n + existed. `publish_version` now emits `updated` on the `else` branch, with context- and + handler-level tests asserting the event type differs between first publish and republish. +- **Webhook assertion on the `PUT /publish`-then-schema-version flow** (job 301): that flow is + what distinguishes the implemented `was_already_ready` guard from a "this is not version 1" + alternative; it now has its own `WebhookFactory` + `HIGH_QUEUE.count` assertions. +- **`"boolean"` added to `_ES_TYPE_BY_COLUMN_DTYPE`** (job 284): pandas emits `bool` for a numpy + bool column and `boolean` for the nullable extension dtype. Only the former was mapped, so the + same logical type took two incompatible ES mappings depending on the Pandera spelling. +- **`test_list_dataset_questions` unskipped** (job 290): its skip reason (`missing 'use_table'`) + no longer held; the expected settings dict gained `use_table`/`columns` and the test passes. +- **e2e `auth-smoke` matcher** (job 299): the `waitForResponse` was armed *before* `signIn`, and + `/api/v1/me/datasets` has a second in-app caller (`DatasetRepository.fetchFeedbackDatasets`) + that fires param-less on the post-login landing page β€” so the assertions were latching onto the + home page's request, not the schemas page's. Now armed after `signIn` and matched on the + `workspace_id` param only `getSchemas` sends. Also dropped the dead `/references/` guard in + `extractions-grid.spec.ts` and the stale "first bearer-token client" rationale. + +## 0b. Closed in the 2026-08-01 simplification pass + +Four items below were closed by simplifying `contexts/schema_versions.publish_version` rather +than by completing them. The change: **`publish_version` no longer flips `dataset.status`.** +`PUT /datasets/{id}/publish` is now the sole draft -> ready transition and the sole +`create_index` caller, so a schema-backed dataset gets the same lifecycle, the same +`DatasetPublishValidator` checks and the same index-creation path as an annotation one. + +The working order is now the obvious one, and it needs no PATCH-after dance: + +```text +POST /datasets -> draft +POST /datasets/{id}/schema-versions -> columns materialized as Field rows, still draft +POST /datasets/{id}/questions -> settings["columns"] bound at creation time +PUT /datasets/{id}/publish -> ready + create_index +PUT /datasets/{id}/records/bulk +``` + +- **Β§8 (`ready` with zero questions) β€” resolved** by option (c). `publish_version` cannot put + a dataset in any status, so the permanently-unconfigurable state is unreachable and the + create-before-publish ordering is gone. `e2e/extraction/seed/seed_v2_e2e.py` follows the + order above; one question per dataset is now `required` because `DatasetPublishValidator` + demands it. +- **Β§1 (republish leaves the dataset unwritable) β€” resolved** by option (b). A column that is + not already a `Field` is rejected with a 422 when `dataset.is_ready`, in the same pre-write + pass as the dtype check. The mapping-evolution work itself is still open; see Β§1 below for + what lifting the restriction would take. Corollary worth knowing: an annotation dataset + already published via `PUT /publish` can no longer become schema-backed, because every + column of a first version is a new column. +- **Β§3 (column/annotation field name collision) β€” resolved.** A column whose name matches an + existing non-column `Field` is rejected in the same pass instead of silently overwriting it. +- **Β§9's `ColumnFieldSettingsUpdate` untested dict-merge β€” resolved by deletion.** The schema + is gone and `column` is no longer a member of the `FieldSettingsUpdate` union, so + `PATCH /fields/{id}` can no longer change a column's dtype out of band β€” which had directly + contradicted the immutability enforced at publish. Republishing the schema version is the + only way to change a column. + +The three checks now live in `contexts/schema_versions._reject_incompatible_columns` (which +replaces `_reject_dtype_changes`), one query and one pass, before any write. + +## 1. ES mapping evolution: `put_mapping` for republish-added columns is not implemented + +**The contract today (2026-08-01, option (b)):** a schema version that declares a column the +dataset does not already have is rejected with a 422 once the dataset is `ready` +(`contexts/schema_versions._reject_incompatible_columns`). A published dataset therefore +cannot gain columns at all, and an annotation dataset already published via +`PUT /datasets/{id}/publish` cannot retroactively become schema-backed, since every column of +a first version is a new column. See Β§0b. + +**What's still missing:** the mapping evolution itself. Until it lands, that 422 is the whole +story β€” there is no path that adds a column to a published dataset's index. + +**Why the 422 exists (historical analysis, 2026-07-28).** Kept because it is what the +implementer of `put_mapping` needs, and because it is the argument for the restriction. The +index mapping is built once, on the draft β†’ ready transition, and nothing evolves it +afterwards. `_configure_index_mappings` sets `"dynamic": "strict"` at the mapping root and +`_mapping_for_fields` adds no override of its own (unlike `metadata`), while +`_map_record_to_es_document` β†’ `_map_record_fields_to_es` emits an entry for *every* +`dataset.fields` row. So a record write after a column was added would carry +`fields.` into a strict index with no mapping for it, and Elasticsearch would +reject it with `strict_dynamic_mapping_exception` β€” `PUT /datasets/{id}/records/bulk` failing +outright, at an endpoint unrelated to the publish that caused it. Rejecting at publish keeps +the failure where it belongs. The alternative considered and not taken was to accept the +unwritable state and document it. + +**Why the real fix wasn't done here:** explicitly out of scope per the human partner's +decision for this fix wave β€” "Do not attempt to fix ES mapping evolution now." Lifting the +422 is gated on it. + +**What it would take:** on a republish where `index_exists` is true, diff the newly-derived +`field_payloads` against the fields that existed before the upsert, and call +`search_engine.put_index_mapping_request` (already implemented on both backends β€” +`elasticsearch.py:116-117`, `opensearch.py:108-109` β€” and already used by +`configure_metadata_property`/`configure_index_vectors` in `commons.py`) with the mapping for +just the new columns, built the same way `_configure_index_mappings` builds column mappings +today. Needs a `_ES_TYPE_BY_COLUMN_DTYPE`-driven mapping-fragment builder factored out of +`_configure_index_mappings` so it can be reused for both the whole-index and the incremental +case. + +**Note:** ENG-36 (registering the LanceDB engine as a `SearchEngine` implementation) may +supersede Elasticsearch entirely for schema-backed extraction datasets. If ENG-36 lands first, +check whether this ticket is still needed before implementing it β€” LanceDB's column-add story +may already be simpler (Arrow schema evolution) than hand-rolling `put_mapping` diffing. + +## 2. Field pruning: a column dropped from a new Pandera body leaves its `Field` row behind + +**What's missing:** `derive_column_fields` + `Field.upsert_many` only ever *add or update* +fields. If a republish's new Pandera body no longer declares a column that a prior version +did, the old `Field` row for that column is never deleted β€” it stays in `GET /datasets/{id}/fields` +and in the ES mapping (if 1. above ever gets built) forever. + +**Why it wasn't done here:** deferred by explicit instruction β€” "Field pruning and +name-collision semantics are deferred to tickets." + +**Semantic note:** this is a real regression introduced by the fold, not a pre-existing v1 +gap. v2's `SchemaVersion.columns_cache` was a wholesale replacement of the column list on every +publish β€” there was nothing to prune because there was no persisted-and-forgotten row. v1's +`Field` table is append/upsert-only by construction (`Field.upsert_many`'s +`__upsertable_columns__` only ever updates `title`/`required`/`settings` for a matching +`(name, dataset_id)`, and nothing in this codebase issues a `DELETE` against `fields` for a +schema-version-driven column). Fixing this means either (a) diffing old vs. new column names on +every publish and deleting the `Field` rows that dropped out, checking first whether any +`Question.settings["columns"]` binds to the dropped name (and deciding what that means β€” dangle +the binding? reject the publish?), or (b) deciding pruning is undesirable and instead exposing +"declared by current version" vs. "declared by some prior version" as a `Field` attribute. + +## 3. Name collision: a Pandera column colliding with an existing annotation field β€” RESOLVED 2026-08-01 + +**Resolved** in `contexts/schema_versions._reject_incompatible_columns` exactly as the "what it +would take" paragraph below describes. Retained for the rationale. + +**What was missing:** `Field.upsert_many`'s conflict target is `(Field.name, Field.dataset_id)`. +If a Pandera schema declares a column whose name matches an existing `text`/`image`/`chat`/ +`custom`/`table` field (created via `POST /datasets/{id}/fields`, not via a schema version), +the upsert overwrites that field's `settings` with `{"type": "column", "dtype": ..., ...}` β€” +silently converting an annotation-input field into a schema column, which removes it from +record value validation (column fields are "deliberately not value-validated" per +`enums.py`'s `FieldType.column` docstring) without any warning. + +**Why it wasn't done here:** deferred by the same explicit instruction as item 2. + +**What it would take:** before the `Field.upsert_many` call in `publish_version`, check +whether any `field_payloads[i]["name"]` already exists as a `Field` with +`settings["type"] != FieldType.column`, and reject the publish (`UnprocessableEntityError` +naming the colliding field) rather than silently overwrite. Symmetrical with the dtype +immutability check added in this fix wave (`contexts/schema_versions._reject_dtype_changes`) β€” +could plausibly live in the same pre-write validation pass. + +## 4. Repointing `demo/seed_demo_workspace.py` + +**Current state:** the script now fails fast with a clear stderr message and `SystemExit(1)` +(see the header comment and the `if __name__ == "__main__":` guard) instead of hard-failing +obscurely on its first `/api/v2/*` request. It was NOT repointed at `/api/v1` β€” that's real, +separate work. + +**What it would take:** every one of its 13 raw HTTP calls targets a deleted `/api/v2/*` route +and needs a v1-shaped replacement: +- `POST/GET/DELETE /api/v2/schemas` β†’ the v1 `Dataset` CRUD (`POST /api/v1/datasets`, etc.) β€” + a "schema" is now just a `Dataset` with a schema version. +- `POST /api/v2/schemas/{id}/versions` β†’ `POST /api/v1/datasets/{id}/schema-versions` + (`api/handlers/v1/datasets/schema_versions.py`). +- `POST /api/v2/schemas/{id}/questions` β†’ `POST /api/v1/datasets/{id}/questions`, created with + their `columns` bindings inline, after the schema version and before `PUT /publish` β€” see the + lifecycle in Β§0b, and `e2e/extraction/seed/seed_v2_e2e.py` for a worked example. +- A `PUT /api/v1/datasets/{id}/publish` step, which has no v2 counterpart: v2's schema publish + implied it. Needs at least one `required` question. +- `POST /api/v2/schemas/{id}/records:bulk-upsert` β†’ the v1 records bulk-upsert endpoint, with + `reference` moved from being implicit to an explicit field (`api/schemas/v1/records.py`). +- `PUT /api/v2/records/{id}/suggestions` / `.../responses` β†’ the v1 suggestions/responses + endpoints, which take `question_id` rather than a schema-scoped question name. +- `POST /api/v2/schemas/{id}:rebuild-index` β†’ no v1 equivalent exists as a dataset action; + the closest analog is `cli/search_engine/reindex.py`'s `Reindexer`, not an HTTP route β€” decide + whether the demo script should shell out to the CLI or whether this step is simply dropped + once schema-version publish creates the index itself. +- `GET /api/v2/projection` β†’ `GET /api/v1/workspaces/{id}/projection` (or wherever + `contexts/projection.build_workspace_view` is mounted β€” check `api/handlers/v1/`). +- `POST /api/v2/token` β†’ `POST /api/v1/token` (this one is a same-shape rename, not a + behavior change). + +Not part of any CI gate (confirmed in `progress.md`'s Task 15 leftover note), so this can be +scheduled independently of any test-suite concern. + +## 5. `_next_version_number` max()+1 race β€” RESOLVED 2026-08-03 + +**Fixed** in `contexts/schema_versions._next_version_number`, which now takes a +`SELECT ... FOR UPDATE` on the dataset row before allocating, so publishes for one dataset +serialize for the rest of the transaction. On SQLite the dialect emits no `FOR UPDATE` clause +(its single-writer model already serializes), so this is a no-op there rather than an error. + +**This section originally understated the damage** (CodeRabbit, PR #236). It recorded the +consequence as a losing request that rolls back cleanly, leaving "an orphaned S3 object" β€” +annoying but harmless, hence "worth fixing only if concurrent publish is a real usage +pattern". That is wrong. `object_key_for(dataset_id, next_version)` is derived from the +version number, so two publishers that read the same max also `put_object` to the **same +key**. The write that lands second overwrites the first's body β€” and it can do so *after* the +first publisher's `SchemaVersion` row, carrying a checksum computed from its own body, has +already committed. The committed row then points at content that does not match its checksum, +breaking exactly the immutability the model exists to provide. The unique constraint does not +save us: it fires at `db.flush()`, long after the object was overwritten. + +Silent corruption of a committed row outranks a stray object, so this was fixed rather than +deferred. + +## 6. Remaining deferred minors carried over from the ledger + +Everything below is copied from `.superpowers/sdd/2026-07-26-fold-v2-into-v1/progress.md`'s +`minor (deferred)` lines, for the items not otherwise covered above or fixed in this wave +(T7 and T9(a) were fixed in this wave; not repeated here). + +- **Task 6 (b):** `schema_versions.py`'s `getattr(metadata, "version_id", None)` is needless β€” + `ObjectMetadata.version_id` always exists on the return type of `files_ctx.put_object`. +- **Task 6 (c):** `tests/unit/contexts/test_schema_versions.py`'s + `test_publish_creates_the_search_index` used `mock_search_engine.create_index.assert_awaited()` + without pinning the argument; `assert_awaited_with(dataset)` would be tighter. (Note: this + test's assertion style is unchanged by this fix wave; the new republish tests added alongside + it do pin arguments/call counts.) +- **Task 8 (a):** the `total` count query under `contexts/records.py`'s reference filter + (`:220-225`) is correct but untested directly β€” only `items` is asserted in the existing + suite. +- **Task 8 (b):** the full-dict-equality test-literal style used across `tests/unit/api/handlers/v1/` + (e.g. asserting an entire JSON response body as a literal dict) is a pre-existing blast-radius + problem β€” any schema field addition touches dozens of test files. A + `response_model_exclude_unset`-aware partial-assertion helper deserves its own cleanup ticket. +- **Task 9 (b):** `QuestionColumnBindingValidator` has no `min_length` on column-name strings, + so `columns: [""]` reaches the validator and is rejected only as "not declared" β€” harmless, + but asymmetric with field-name validation elsewhere in the codebase. +- **Task 11 (a):** `@pytest.mark.asyncio` markers are redundant under `asyncio_mode="auto"` + (`pyproject.toml:137`) in several test files touched by the fold; carried verbatim from the + brief rather than cleaned up. +- **Task 11 (b):** a test `get_async_db` override dropped the `IsolationLevel | None` + annotation present in the original `conftest.py` β€” cosmetic typing regression, no behavior + change. +- **Task 13 (a):** `ExtractionsGrid.client.vue:207` still emits the payload key `schemaId` + while it now carries a v1 dataset id β€” the value is correct, only the key name is stale + relative to the fold. +- **Task 13 (b):** the "discarded" status filter option (`pages/schemas/[id]/index.vue:32`) and + `V2RecordStatus.discarded` (`V2Record.ts:1`) will silently return zero rows against v1's + `RecordStatus` (`pending`/`completed` only β€” `enums.py:53-55`). Pre-existing, not a regression + from this fold, but a real dead UI affordance worth removing or wiring up. +- **Task 13 (c):** `extralit-frontend/package.json` still lists the now-unused + `openapi-typescript` devDependency. Removing it touches the lockfile, so it was left for a + dedicated dependency-cleanup pass. +- **Task 14 (a):** `v1/di/di.ts` imports 16 repositories via the barrel export but the 3 + merged-from-v2 ones by direct path β€” two import styles in one file. Add the 3 to + `v1/infrastructure/repositories/index.ts` for consistency. +- **Task 14 (b):** `ExtractionsStorage.ts:4`'s comment still says "v1/v2 useStoreFor" β€” no v2 + namespace remains anywhere in the frontend tree as of this fold. +- **Task 14 (c):** `composables/useV2Breadcrumbs.ts` still carries a `V2` prefix in its name, + but it predates this fold's base commit and was never under `v2/` β€” flagged only because it + reads as if it violates "nothing carries a V2 prefix" scope, when it's actually pre-existing + and out of this fold's scope. + +## 7. DECISION NEEDED β€” annotators lost read access to the schema/extraction record views + +**Severity: High.** Introduced by this fold (roborev job 295), and a real user-visible +regression rather than a code-hygiene issue. + +`SchemaRecordRepository` (`v1/infrastructure/repositories/SchemaRecordRepository.ts`) β€” which +backs the whole `pages/schemas/[id]/` view via `useSchemaRecordsViewModel` β€” calls: + +- `GET /v1/datasets/{id}/records`, authorized by `DatasetPolicy.list_records_with_all_responses` +- `POST /v1/datasets/{id}/records/search`, authorized by `DatasetPolicy.search_records_with_all_responses` + +Both are `actor.is_owner or (actor.is_admin and await actor.is_member(...))` β€” **admin/owner +only**. The deleted `SchemaPolicy.list_records` they replaced was +`actor.is_owner or await actor.is_member(...)`, i.e. any workspace member including annotators. +So an annotator who could browse and search `/schemas/{id}` under v2 now gets 403 on both calls +and the page is empty for them. + +Every current test is an axios mock asserting a URL the test itself supplies, so nothing catches +the role change. The sibling `ProjectionRepository` in the same commit *did* pick the `/me/` +variant, so the inconsistency is internal to this change. + +**Options:** +- **(a)** Point `searchRecords` at `/v1/me/datasets/{id}/records/search` + (`api/handlers/v1/datasets/records.py:341`, the annotator-scoped twin, already exists). There + is **no `/me/` equivalent for the list path**, so this alone does not restore the annotator β€” + the initial page load still 403s. +- **(b)** Relax the list route to `DatasetPolicy.list_records` (member-readable), restoring v2's + behaviour. Widens who can see all responses on a v1 route that annotation datasets also use. +- **(c)** Add a `/me/datasets/{id}/records` list twin scoped like the search one. +- **(d)** Declare `/schemas` admin-only by design and hide the nav entry for annotators. + +(c) is the cleanest and matches the existing `/me/` precedent; (b) is the smallest diff but +changes an existing v1 route's authorization for annotation datasets too. Needs a call before +implementation, plus a policy-level test pinning the intended role either way. + +## 8. RESOLVED 2026-08-01 β€” `publish_version` no longer touches `dataset.status` + +**Decided: option (c).** `PUT /datasets/{id}/publish` is the sole draft -> ready transition. +See Β§0b for the resulting lifecycle. The section below is the original finding. + +### Original finding β€” `publish_version` can create a `ready` dataset with zero questions + +**Severity: High** as filed (roborev jobs 286/287/290), **partially resolved in practice.** + +The original finding was that `publish_version` sets `dataset.status = DatasetStatus.ready`, +while `QuestionCreateValidator._validate_dataset_is_not_ready` (`validators/questions.py`) +rejects question creation on a `ready` dataset β€” making "publish the Pandera body, then bind a +question to a declared column" impossible. + +**What actually happened:** the e2e seed (commit `4256b7f`) established a working order β€” +**create questions before publish, then `PATCH /questions/{id}` the `columns` bindings after** β€” +because `QuestionUpdateValidator` does *not* call `_validate_dataset_is_not_ready`. So the flow +is reachable; it is just non-obvious. That ordering is now the de-facto contract and Β§4 below +depends on it. + +**What is still genuinely open:** `publish_version` bypasses `DatasetPublishValidator` +(`validators/datasets.py`), so it can put a dataset in `ready` with **zero questions** β€” a state +`PUT /datasets/{id}/publish` explicitly refuses to create, and from which no question can ever be +added (create is blocked by `ready`; there is nothing to PATCH). That dataset is permanently +unconfigurable. + +**Options:** (a) run the relevant `DatasetPublishValidator` checks in `publish_version` too; +(b) relax `_validate_dataset_is_not_ready` for `current_schema_version_id IS NOT NULL` so +schema-backed datasets stay configurable after publish; (c) don't flip `status` in +`publish_version` at all and leave `PUT /datasets/{id}/publish` as the sole ready transition. +(b) also removes the need for the create-before-publish dance entirely, which would simplify Β§4. +Whichever is chosen, document the intended lifecycle β€” it is currently only discoverable by +reading the e2e seed. + +## 9. Open roborev findings carried forward (jobs 283–301) + +Triaged 2026-07-28; the reviews are closed, so this is the surviving record. Grouped by area, +each with the job it came from. None block merge. + +### Server β€” correctness / semantics + +- **(287)** `test_schema_versions.py`'s `_mock_put_object` patches `files_ctx.put_object` but not + the `Depends(files_ctx.get_s3_client)` that resolves *before* the handler body β€” so every one + of those requests still constructs a `LocalFileClient(settings.home_path)`, creates + directories under the developer's `~/.extralit`, and caches it in the process-global + `shared_resources`. Register `files_ctx.get_s3_client` in `api_v1.dependency_overrides`, or + patch `helpers.create_s3_client`. +- **(288)** The `reference` predicate is written twice β€” inside `_build_list_records_query` and + hand-rolled onto `total_query` in `contexts/records.list_dataset_records`. The next filter + added to the builder will silently not apply to the count, reproducing the bug that fix closed. +- **(290)** `columns` is only on the `text` and `table` settings triples, but + `QuestionColumnBindingValidator`'s error message advertises every scalar type. With pydantic's + default `extra='ignore'`, `{"type": "label_selection", "columns": ["a"]}` returns **201** with + the binding silently discarded. Either add `columns` to the remaining bindable types or + restrict the validator to `QuestionType.text` and say so. +- **(290)** `QuestionUpdateValidator` dumps settings without `exclude_unset=True`, so it cannot + distinguish an omitted `columns` from an explicit `null`, while `contexts/questions.update_question` + dumps *with* it and therefore does persist the clear. Nothing breaks today (clearing needs no + validation) but the two disagree. +- **(292)** `contexts/projection.py`'s DuckDB staging layer still speaks v2 vocabulary + (`questions(question_id, schema_id, schema_name, …)`, `q.schema_id`) while the Python half is + renamed to `dataset_*`. `_INSERTS` binds **positionally** into all-`VARCHAR` columns, so + reordering either side silently produces wrong column names in the grid instead of raising. + Rename and switch to named inserts. +- **(292)** A `table` question whose `settings["columns"]` is absent or empty contributes no + manifest column and no cells β€” it vanishes from the grid with no error. Reachable: + `TableQuestionSettings.columns` defaults to `None` and the binding validator returns early on + `None`. The non-table branch emits a column unconditionally, so the two paths treat "no + binding" in opposite ways. Pick one. +- **(294)** `index/mapping.py`'s module docstring says the manifest is "the list of column dicts + derived from a schema version's `Field` rows", but every function reads `column["dtype"]` at + the *top level* β€” the flat `columns_cache` shape. A `Field` row nests `dtype` under `settings`, + so passing them raises `KeyError`. No adapter exists. Since `index/` has no production callers, + this docstring is the only spec of the contract β€” and it is wrong for the ENG-36 implementer. + +### Server β€” tests + +- **(286, 285, 284)** Three `pytest.raises(Exception)` broad catches remain in fold-touched files + (`tests/unit/contexts/test_schema_versions.py:150`, `tests/unit/validators/test_column_fields.py:68`); + narrow to the typed exception with `match=`. Several new assertions also lack `match=` + anchors (`test_field_settings.py`). +- ~~**(285)** The `ColumnFieldSettingsUpdate` dict-merge path is untested.~~ **Resolved + 2026-08-01 by deletion** β€” see Β§0b. Columns are not PATCHable at all now. +- **(291)** `test_patch_rejects_an_invalid_column_binding`'s non-persistence assertion is vacuous: + `TextQuestionFactory.settings` never sets `columns`, so `stored.settings.get("columns") is None` + holds whether or not the PATCH was rejected. Seed a valid binding first. +- **(291)** The column-binding PATCH tests do not pin the `selectinload(Dataset.fields)` eager + load they were written to protect β€” the handler shares the test's session and identity map, so + `dataset.fields` is served from session state. Add `db.expunge_all()` before the PATCH. +- **(293)** `test_extraction_response_side_effects.py` never asserts + `search_engine.partial_record_update` β€” the index write that actually carries the derived + `completed` status, and the exact junction the module's docstring names. Delete that line from + `distribution.py` and all four tests stay green. +- **(293)** No `discarded` case: `ResponseUpsert` is a three-member union and only submitted and + draft are covered. Discard is the branch this fold reasoned about explicitly when removing + `V2RecordStatus.discarded`. +- **(293)** Index assertions are argument-blind (`assert_awaited()` with no args pinned). +- **(293)** Third hand-rolled copy of the "redirect a context's own session back at the test + session" workaround; promote to a shared `tests/conftest.py` fixture. +- **(296)** The non-null `current_schema_version_id` case is pinned only on `GET /datasets/{id}`, + not the `GET /me/datasets` list the frontend filter actually uses. +- **(300)** `api_v1.dependency_overrides.pop(get_search_engine, None)` in a `finally` deletes the + key rather than restoring the `async_client` fixture's own override β€” harmless as written, but + it is the copy-paste template for the next one, and the failure it produces (a later test + reaching a real Elasticsearch) is slow to diagnose. +- **(301)** `_RealMappingSearchEngine` constructs a real `ElasticSearchEngine`, which constructs a + real `AsyncElasticsearch` client that is never closed β€” surfaces as an "Unclosed client + session" `ResourceWarning` at GC in an unrelated later test. `_configure_index_mappings` reads + nothing off `self`, so the client is pure overhead. +- **(283)** `tests/unit/api/test_api_mounts.py`'s comment claims a `base_url` fix that did not + happen β€” `_app.py`'s module-level `app` *is* `create_server_app()`'s return value, wrapper + included, so the assertion still fails under a non-`/` `EXTRALIT_BASE_URL`. +- **(283)** The alembic recovery instructions in + `13da2d87e660_add_schema_versions_and_record_reference.py` are dialect-wrong (the collision hits + SQLite too) and internally out of order (`schema_versions` cannot be dropped "BEFORE upgrading" + while five tables still FK into it). `extralit-server/CLAUDE.md` points readers there. + +### Server β€” API shape + +- **(287)** `list_schema_versions` returns a bare JSON array while every other v1 collection + endpoint returns an `{items: [...]}` envelope. Adding pagination later becomes breaking. +- **(288)** `reference` is filterable on `GET /datasets/{id}/records` but is not in the ES + document or mapping, so it cannot be filtered or sorted on the search endpoint β€” two listing + surfaces on one resource disagree about what is queryable, with nothing recording that the + omission is deliberate. +- **(294)** `settings.lancedb_uri`'s pydantic `description` embeds the Linear id "ENG-36" β€” a + user-facing string surfacing in operator config docs. Keep the ticket pointer in + `index/__init__.py`'s docstring instead. +- **(294)** `index/base.py` and `index/mapping.py` still take `schema_id: UUID` and build + `schema_`-prefixed table names, and `union_columns(caches:)` keeps the `columns_cache`-era + parameter name, though `Schema` no longer exists as a model. +- **(296)** The SDK's `DatasetModel` (`extralit/_models/_dataset.py`) omits + `current_schema_version_id`, and its default `extra="ignore"` drops it silently β€” so the + frontend can tell a schema-backed dataset from a plain one and the SDK cannot, from the same + endpoint. + +### Frontend + +- **(295)** `RecordsPage`'s comment claims `total` is "Authoritative: v1's Elasticsearch-backed + list/search endpoints return an exact count", but only the search path is ES-backed; the list + path is Postgres with `total: int | None = None` and a standing server TODO. When the + `?? 0` fallback fires the page renders a literal `0` above populated results. Relatedly, + `pages/schemas/[id]/index.vue` still gates "next" on a full page rather than + `currentOffset + pageSize < page.total`, so it can advance onto an empty page. +- **(297)** `v1/domain/entities/search/SearchCriteria.ts` sits beside the unrelated annotation + `SearchTextCriteria extends Criteria` with nothing in either file saying which family it + belongs to and no compiler-visible collision to force the issue. Add a docstring naming the + endpoint, or move it under `v1/domain/entities/schema/`. + +### e2e / demo + +- **(298)** `e2e/extraction/seed/seed_v2_e2e.py` still carries the v2 name in its filename and in + the values it writes β€” `SCHEMA_NAME = "e2e_v2_slice"`, `EMPTY_SCHEMA_NAME = "e2e_v2_empty"`, + `WORKSPACE_NAME = "e2e-v2"`, `REFERENCE = "10.1000/j.e2e-v2"`, output key `schemaId`. A freshly + seeded stack renders "e2e_v2_slice" in the UI the extraction specs assert against. +- **(298)** The `country` fixture comment justifies parking `"KE-control"` by citing + `index/mapping.py::record_to_row`, which has zero production callers. The live invariant is + `search_engine/commons.py::_build_text_query` scoping a fieldless `q` to `fields.*` β€” same + outcome, different reason. A change to that scoping would break `search-roundtrip.spec.ts` + while the comment kept vouching for the old reasoning. +- **(298)** `demo/seed_demo_workspace.py` β€” see Β§4. It fails fast rather than 404ing obscurely, + but is still unmigrated. diff --git a/docs/superpowers/plans/2026-07-26-fold-v2-into-v1.md b/docs/superpowers/plans/2026-07-26-fold-v2-into-v1.md index 90c817156..d105a0ef1 100644 --- a/docs/superpowers/plans/2026-07-26-fold-v2-into-v1.md +++ b/docs/superpowers/plans/2026-07-26-fold-v2-into-v1.md @@ -19,6 +19,17 @@ - Frontend commands run from `extralit-frontend/`: `npm run test`, `npm run lint`, `npx nuxi typecheck`. - Commit after every task. Never leave the tree with a failing `uv run ruff check` or a failing test suite between tasks. +### Why the deletions come first (Tasks 1–3), and how to read the deleted code + +`models/v2/schemas.py:44` already declares a `SchemaVersion` with `__tablename__ = "schema_versions"` on the same `DatabaseModel` base as every v1 model. Declaring the new v1 `SchemaVersion` (Task 4) while it exists raises `InvalidRequestError: Table 'schema_versions' is already defined for this MetaData instance` at import time β€” so the model foundation cannot be built until the v2 model is gone. `models/v2` is reachable from `api/v2`, `contexts/v2`, `cli/index`, and the v2 test tree, so removing it cascades to the whole parallel tree. Hence the fold runs **delete first, then build**, not the other way round. + +Two consequences of that ordering, both binding on every task: + +- **`/api/v2` is unreachable from Task 1 until Task 13 repoints the frontend.** This is deliberate and is not a regression to "fix" mid-plan. The v1 replacements land in Tasks 7 (schema versions) and 10 (projection); the frontend catches up in Task 13 and is verified live in Task 15. Do not add a compatibility shim. +- **The v2 sources every later task references are deleted before those tasks run.** Task 1 Step 1 writes a snapshot and a tag so they stay readable: + - **`$V2REF`** = `/.superpowers/sdd/2026-07-26-fold-v2-into-v1/v2-reference/` (git-ignored). Paths under it mirror the repo, e.g. `$V2REF/extralit-server/src/extralit_server/contexts/v2/projection.py`. Every "Reference:" line naming a `v2/` path in Tasks 4–11 means *this snapshot*, not a live file. + - **`git show v2-pre-fold:`** is the fallback if the snapshot is lost β€” the tag marks the last commit with the tree intact. + ### The server test tree is named backwards β€” read this before writing a test This trips everyone. The directory names do not mean what they say: @@ -28,10 +39,11 @@ This trips everyone. The directory names do not mean what they say: **Every new test in this plan goes under `tests/unit/`**, at the path mirroring the module it covers. Do not add anything to `tests/integration/` β€” by the end of this plan that tree contains only `index/` (kept for ENG-36) and `test_rq_groups_workflow.py`. -Two consequences the deletion tasks must handle, both easy to miss: +Three consequences the deletion tasks must handle, all easy to miss: -- `tests/integration/conftest.py` imports `api_v2`, so it breaks at collection the moment `api/v2/` is deleted. `tests/integration/test_rq_groups_workflow.py` is a genuine v1 test that depends on that conftest's `async_client` and `owner_auth_header`. β†’ Task 9 Step 4. -- `tests/integration/index/test_lancedb_engine.py:19` imports `V2RecordStatus`, which Task 1 deletes. The `index/` tests are otherwise untouched, but this one stub needs a two-line fix. β†’ Task 10 Step 4. +- `tests/integration/conftest.py` imports `api_v2`, so it breaks at collection the moment `api/v2/` is deleted. `tests/integration/test_rq_groups_workflow.py` is a genuine v1 test that depends on that conftest's `async_client` and `owner_auth_header`. β†’ Task 1 Step 6. +- `contexts/v2/{annotation,projection,records}.py` import from `api/schemas/v2/`, so those modules become unimportable the moment Task 1 deletes that package β€” even though their own deletion is Task 2. Nothing imports them at runtime after `api/v2` goes (`ruff` does not resolve cross-module imports, so lint stays clean), but **every test that imports them must be deleted in Task 1**, not Task 2: `tests/integration/contexts/v2/` and `tests/unit/test_annotation_no_index_import.py`. β†’ Task 1 Step 5. +- `tests/integration/index/test_lancedb_engine.py:19` imports `V2RecordStatus`, which Task 4 deletes from `enums.py`. The `index/` tests are otherwise untouched, but this one stub needs a two-line fix, done early so the enum deletion is unblocked. β†’ Task 2 Step 4. --- @@ -50,13 +62,13 @@ Of the 26 v2 endpoints, **8 are live**, 4 are orphaned (their UI was retired by These are fixed as a consequence of folding, and each has a regression test in this plan: -1. **Records can never reach `completed`.** `contexts/v2/annotation.py:136` `upsert_response` deliberately "never mutates `record.status`". No v2 code path calls `contexts/distribution.py`. So every v2 record sits at `pending` forever and `/extractions` coverage counts are wrong. v1's `contexts/datasets.py:552` `upsert_response` calls `distribution.update_record_status`. β†’ Task 8. -2. **v2 review data is unsearchable by construction.** `tests/unit/test_annotation_no_index_import.py` *enforces* that the annotation modules never reach the index engine. So a submitted response or a suggestion never reaches any index, and no search can filter by response status or suggestion agent β€” which v1 ES does natively via `update_record_response` / `update_record_suggestion`. β†’ Task 8. -3. **`PUT /schemas/{schema_id}` is an untested destructive path.** Zero consumers anywhere, zero tests, and `contexts/v2/schemas.py:69` passes `replace_dict=True` so a partial `settings` payload silently wipes stored keys. β†’ deleted in Task 12; `PATCH /datasets/{id}` (merge semantics, validated by `DatasetUpdateValidator`) replaces it. -4. **Deleting a schema version silently deletes its records.** `models/v2/records.py:29` FKs `schema_version_id` with `ondelete="CASCADE"`. β†’ the pin column is dropped entirely in Task 3. +1. **Records can never reach `completed`.** `contexts/v2/annotation.py:136` `upsert_response` deliberately "never mutates `record.status`". No v2 code path calls `contexts/distribution.py`. So every v2 record sits at `pending` forever and `/extractions` coverage counts are wrong. v1's `contexts/datasets.py:552` `upsert_response` calls `distribution.update_record_status`. β†’ Task 11. +2. **v2 review data is unsearchable by construction.** `tests/unit/test_annotation_no_index_import.py` *enforces* that the annotation modules never reach the index engine. So a submitted response or a suggestion never reaches any index, and no search can filter by response status or suggestion agent β€” which v1 ES does natively via `update_record_response` / `update_record_suggestion`. β†’ Task 11. +3. **`PUT /schemas/{schema_id}` is an untested destructive path.** Zero consumers anywhere, zero tests, and `contexts/v2/schemas.py:69` passes `replace_dict=True` so a partial `settings` payload silently wipes stored keys. β†’ deleted with `api/v2` in Task 1; `PATCH /datasets/{id}` (merge semantics, validated by `DatasetUpdateValidator`) replaces it. +4. **Deleting a schema version silently deletes its records.** `models/v2/records.py:29` FKs `schema_version_id` with `ondelete="CASCADE"`. β†’ the pin column goes with `models/v2` in Task 3 and is deliberately not carried onto the v1 `Record` in Task 4. 5. **Record search totals are wrong.** `RecordsPage` carries an "approximate total" because "stale Lance ids are skipped on hydration and FTS", and `index_sync`'s `sync_*` functions log-and-swallow every error, so Postgres and Lance diverge with no signal. β†’ Task 13 repoints search at v1 ES, which returns an authoritative total and has `cli/search_engine/reindex.py` as a documented repair path. -6. **`V2RecordStatus.discarded` is set by nothing.** Discard is a *response* status in v1 (`ResponseStatus.discarded`); record status is derived. The enum member and the `status` patch field on `RecordUpsert` both go. β†’ Task 3. -7. `_get_schema_or_404` is byte-identical in three files (`api/v2/schemas.py:30`, `api/v2/records.py:37`, `api/v2/questions.py:26`). All three go; v1's `Dataset.get_or_raise` replaces them. +6. **`V2RecordStatus.discarded` is set by nothing.** Discard is a *response* status in v1 (`ResponseStatus.discarded`); record status is derived. The enum member and the `status` patch field on `RecordUpsert` both go. β†’ `RecordUpsert` with `api/schemas/v2` in Task 1, `V2RecordStatus` with the `enums.py` sweep in Task 4. +7. `_get_schema_or_404` is byte-identical in three files (`api/v2/schemas.py:30`, `api/v2/records.py:37`, `api/v2/questions.py:26`). All three go with `api/v2` in Task 1; v1's `Dataset.get_or_raise` replaces them. --- @@ -149,2210 +161,2301 @@ DELETED ENUM TYPES: schema_status_enum, v2_record_status_enum, v2_question_type_ --- -### Task 1: v1 model + enum foundation for the folded model +### Task 1: Snapshot the v2 tree, then delete the v2 API surface -No behavior yet β€” just the schema surface every later task builds on. Doing this first means every subsequent task can be tested against a real database. +The first three tasks are removals, and they come first because the new `SchemaVersion` cannot be declared while `models/v2/schemas.py` owns the `schema_versions` table name β€” see "Why the deletions come first" in Global Constraints. Within the removals, order still matters: API first (nothing depends on it), then contexts, then models. **Files:** -- Modify: `extralit-server/src/extralit_server/enums.py` -- Modify: `extralit-server/src/extralit_server/models/database.py` -- Test: `extralit-server/tests/unit/test_enums.py` (create if absent), `extralit-server/tests/unit/models/test_schema_version_model.py` (create) +- Create: `/.superpowers/sdd/2026-07-26-fold-v2-into-v1/v2-reference/` β€” the snapshot Tasks 4–11 read +- Delete: `extralit-server/src/extralit_server/api/v2/` (entire directory: `__init__.py`, `annotation.py`, `projection.py`, `questions.py`, `records.py`, `schemas.py`) +- Delete: `extralit-server/src/extralit_server/api/schemas/v2/` (entire directory) +- Delete: `extralit-server/src/extralit_server/api/policies/v1/schema_policy.py`, `extralit-server/src/extralit_server/api/policies/v1/v2_annotation_policy.py` +- Delete: `extralit-server/tests/integration/api/v2/`, `extralit-server/tests/integration/api/schemas/v2/`, `extralit-server/tests/integration/contexts/v2/`, `extralit-server/tests/unit/test_annotation_no_index_import.py` +- Modify: `extralit-server/src/extralit_server/_app.py`, `extralit-server/src/extralit_server/api/policies/v1/__init__.py`, `extralit-server/src/extralit_server/cli/openapi_dump.py`, `extralit-server/tests/integration/conftest.py` **Interfaces:** -- Consumes: nothing. -- Produces: `FieldType.column`; `SchemaVersion` (table `schema_versions`, FK `dataset_id`, columns `version: int`, `object_key: str`, `object_version_id: str | None`, `etag: str`, `checksum: str`, `parent_version_id: UUID | None`, `created_by: UUID | None`); `Dataset.current_schema_version_id: UUID | None`; `Dataset.schema_versions: list[SchemaVersion]`; `Record.reference: str | None`; `Field.__upsertable_columns__`. +- Consumes: nothing. This task does **not** wait for the v1 replacements β€” they land in Tasks 7 (schema versions) and 10 (projection), and `/api/v2` is deliberately unreachable in between. The frontend is repointed in Task 13. +- Produces: `/api/v2` no longer exists; `$V2REF` snapshot and the `v2-pre-fold` tag exist for every later task's "Reference:" paths. -- [ ] **Step 1: Write the failing tests** +- [ ] **Step 1: Snapshot the v2 tree before anything is deleted** -Create `extralit-server/tests/unit/models/test_schema_version_model.py`: +Every later task that says "Reference: `.../v2/...`" reads this snapshot, because the live files will not survive Tasks 1–3. Write it once, now, from a clean tree: -```python -import pytest -from sqlalchemy.ext.asyncio import AsyncSession +```bash +cd /home/jonny/Projects/Extralit/extralit +REF=.superpowers/sdd/2026-07-26-fold-v2-into-v1/v2-reference +mkdir -p "$REF" +for path in \ + extralit-server/src/extralit_server/api/v2 \ + extralit-server/src/extralit_server/api/schemas/v2 \ + extralit-server/src/extralit_server/contexts/v2 \ + extralit-server/src/extralit_server/models/v2 \ + extralit-server/src/extralit_server/validators/v2 \ + extralit-server/src/extralit_server/cli/index \ + extralit-server/src/extralit_server/api/policies/v1/schema_policy.py \ + extralit-server/src/extralit_server/api/policies/v1/v2_annotation_policy.py \ + extralit-server/tests/integration/api/v2 \ + extralit-server/tests/integration/api/schemas/v2 \ + extralit-server/tests/integration/contexts/v2 \ + extralit-server/tests/integration/models/v2 \ + extralit-server/tests/unit/validators/v2 \ + extralit-server/tests/factories.py \ + extralit-server/tests/integration/conftest.py ; do + mkdir -p "$REF/$(dirname "$path")" && cp -r "$path" "$REF/$path" +done +git tag -f v2-pre-fold +find "$REF" -name '__pycache__' -type d -prune -exec rm -rf {} + +find "$REF" -name '*.py' | wc -l +``` -from extralit_server.enums import FieldType -from extralit_server.models.database import Dataset, Field, Record, SchemaVersion -from tests.factories import DatasetFactory, RecordFactory +Expected: a non-zero file count, and `git status` still clean (the workspace is git-ignored; the tag is not a commit). Confirm the tag resolves: +```bash +cd /home/jonny/Projects/Extralit/extralit && git show v2-pre-fold:extralit-server/src/extralit_server/contexts/v2/projection.py | head -3 +``` -@pytest.mark.asyncio -class TestSchemaVersionModel: - async def test_field_type_column_exists(self): - assert FieldType.column == "column" +- [ ] **Step 2: Write the failing test that pins the deletion** - async def test_schema_version_belongs_to_dataset(self, db: AsyncSession): - dataset = await DatasetFactory.create() - version = await SchemaVersion.create( - db, - dataset_id=dataset.id, - version=1, - object_key=f"schemas/{dataset.id}/v1.json", - etag="etag-1", - checksum="checksum-1", - ) - assert version.dataset_id == dataset.id - assert version.version == 1 - assert version.parent_version_id is None +Add to `extralit-server/tests/unit/api/test_api_mounts.py` (create if absent): - async def test_dataset_points_at_current_schema_version(self, db: AsyncSession): - dataset = await DatasetFactory.create() - version = await SchemaVersion.create( - db, dataset_id=dataset.id, version=1, object_key="k", etag="e", checksum="c" - ) - await dataset.update(db, current_schema_version_id=version.id) - await db.refresh(dataset, attribute_names=["schema_versions"]) - assert dataset.current_schema_version_id == version.id - assert [v.id for v in dataset.schema_versions] == [version.id] +```python +import pytest - async def test_schema_version_number_is_unique_per_dataset(self, db: AsyncSession): - dataset = await DatasetFactory.create() - await SchemaVersion.create(db, dataset_id=dataset.id, version=1, object_key="k", etag="e", checksum="c") - with pytest.raises(Exception): - await SchemaVersion.create(db, dataset_id=dataset.id, version=1, object_key="k2", etag="e", checksum="c") +from extralit_server._app import create_server_app - async def test_record_carries_a_reference(self, db: AsyncSession): - record = await RecordFactory.create(reference="10.1000/j.foo.2020.01") - assert record.reference == "10.1000/j.foo.2020.01" - async def test_record_reference_defaults_to_none(self, db: AsyncSession): - record = await RecordFactory.create() - assert record.reference is None +class TestApiMounts: + def test_only_v1_is_mounted(self): + app = create_server_app() + mounts = {route.path for route in app.routes if hasattr(route, "app")} + assert "/api/v1" in mounts + assert "/api/v2" not in mounts +``` - async def test_field_is_upsertable(self): - assert Field.__upsertable_columns__ == {"title", "required", "settings"} +Confirm the app-factory function name in `_app.py` and use the real one. - async def test_deleting_dataset_deletes_its_schema_versions(self, db: AsyncSession): - dataset = await DatasetFactory.create() - await SchemaVersion.create(db, dataset_id=dataset.id, version=1, object_key="k", etag="e", checksum="c") - await dataset.delete(db) - assert (await SchemaVersion.get_by(db, dataset_id=dataset.id)) is None +- [ ] **Step 3: Run it to verify it fails** + +```bash +cd extralit-server && uv run pytest tests/unit/api/test_api_mounts.py -v ``` -- [ ] **Step 2: Run the tests to verify they fail** +Expected: FAIL β€” `/api/v2` is still mounted. + +- [ ] **Step 4: Delete the API surface** ```bash -cd extralit-server && uv run pytest tests/unit/models/test_schema_version_model.py -v +cd extralit-server && rm -rf src/extralit_server/api/v2 src/extralit_server/api/schemas/v2 \ + src/extralit_server/api/policies/v1/schema_policy.py \ + src/extralit_server/api/policies/v1/v2_annotation_policy.py \ + tests/integration/api/v2 tests/integration/api/schemas/v2 ``` -Expected: collection error β€” `ImportError: cannot import name 'SchemaVersion' from 'extralit_server.models.database'`. - -- [ ] **Step 3: Add `FieldType.column` and delete the v2 enums** +- [ ] **Step 5: Delete the tests that die with `api/schemas/v2`** -In `enums.py`, add `column = "column"` to `FieldType` (after `table`). Then delete the `SchemaStatus` and `V2RecordStatus` classes at the bottom of the file entirely β€” nothing will reference them after Task 12, and leaving them would keep the parallel vocabulary alive. `ruff` will flag any remaining importer; that is the intent. +`contexts/v2/{annotation,projection,records}.py` import `api/schemas/v2/{annotation,questions,projection,records}`, which Step 4 just removed. Those three modules are now unimportable β€” that is fine, nothing imports them at runtime and Task 2 deletes them outright, but any test that *does* import them fails at collection. Two test paths do, so they go here rather than with their sources: -```python -class FieldType(StrEnum): - text = "text" - image = "image" - chat = "chat" - custom = "custom" - table = "table" - # A column declared by the dataset's Pandera schema version. Carries a dtype for the - # index mapping and is deliberately not value-validated: columns are extraction inputs, - # not annotator-editable answers. Editable columns get a Question bound to them instead. - column = "column" +```bash +cd extralit-server && rm -rf tests/integration/contexts/v2 tests/unit/test_annotation_no_index_import.py ``` -- [ ] **Step 4: Add the `SchemaVersion` model** +`tests/unit/test_annotation_no_index_import.py` goes for a second, better reason: the constraint it enforced β€” annotation must never reach the index β€” is the *cause* of bug 2. v1 syncs responses and suggestions to the index by design, so a guard forbidding that is now actively wrong. + +`tests/unit/validators/v2/` and `tests/integration/models/v2/` are **not** deleted here: `validators/v2` imports only `api/schemas/v1`, and `models/v2` imports nothing from `api/`, so both still collect and pass. They go with their sources in Tasks 2 and 3. + +- [ ] **Step 6: Rewrite the v2 conftest so the one genuine v1 test in that tree survives** -In `models/database.py`, add the class next to `Dataset` (they change together). Copy the column list from `models/v2/schemas.py:45-67` but **omit `columns_cache` and `review_widgets`** β€” the `fields` table replaces both β€” and rename `schema_id` to `dataset_id`. +`tests/integration/conftest.py` mounts `api_v2` and will fail at collection now. But `tests/integration/test_rq_groups_workflow.py` is a real v1 test (it hits `/api/v1/jobs/...`) that depends on this conftest's `async_client` and `owner_auth_header`. Do not delete the conftest β€” reduce it to what that one test needs, retargeted onto `api_v1`: ```python -class SchemaVersion(DatabaseModel): - """An immutable, object-store-backed Pandera schema body for a dataset. +"""Fixtures for the tests remaining in this tree. - The body itself lives in the workspace bucket at `object_key`; this row is the - pointer plus integrity metadata. The column manifest derived from the body is - materialized as `Field` rows on the dataset, so there is no cached copy here. - """ +This file used to wire the isolated `/api/v2` suite. That suite is gone; what remains +is `test_rq_groups_workflow.py` (a v1 jobs test) and `index/` (the LanceDB engine, +kept for ENG-36 and fixture-free). New tests belong under `tests/unit/` β€” see the +plan's "The server test tree is named backwards" note. +""" - __tablename__ = "schema_versions" +from collections.abc import AsyncGenerator - dataset_id: Mapped[UUID] = mapped_column(ForeignKey("datasets.id", ondelete="CASCADE"), index=True) - version: Mapped[int] = mapped_column(index=True) - object_key: Mapped[str] = mapped_column(Text) - object_version_id: Mapped[str | None] = mapped_column(Text, nullable=True) - etag: Mapped[str] = mapped_column(String) - checksum: Mapped[str] = mapped_column(String) - parent_version_id: Mapped[UUID | None] = mapped_column( - ForeignKey("schema_versions.id", ondelete="SET NULL"), nullable=True - ) - created_by: Mapped[UUID | None] = mapped_column(ForeignKey("users.id", ondelete="SET NULL"), nullable=True) +import pytest +import pytest_asyncio +from httpx import AsyncClient - dataset: Mapped["Dataset"] = relationship(back_populates="schema_versions", foreign_keys=[dataset_id]) +from extralit_server.constants import API_KEY_HEADER_NAME +from extralit_server.database import get_async_db +from extralit_server.models import User +from tests.database import TestSession +from tests.factories import OwnerFactory - __table_args__ = (UniqueConstraint("dataset_id", "version", name="schema_version_dataset_id_version_uq"),) - def __repr__(self) -> str: - return f"SchemaVersion(id={self.id!s}, dataset_id={self.dataset_id!s}, version={self.version!r})" -``` +@pytest_asyncio.fixture +async def owner() -> User: + return await OwnerFactory.create(first_name="Owner", username="owner", api_key="owner.apikey") -- [ ] **Step 5: Wire `Dataset`, `Record`, and `Field`** -On `Dataset` (`models/database.py:414`), add the pointer and the collection. `use_alter=True` is required: `datasets` and `schema_versions` reference each other, so Alembic must emit this FK as a separate `ALTER`. +@pytest.fixture +def owner_auth_header(owner: User) -> dict[str, str]: + return {API_KEY_HEADER_NAME: owner.api_key} -```python - current_schema_version_id: Mapped[UUID | None] = mapped_column( - ForeignKey("schema_versions.id", ondelete="SET NULL", use_alter=True), nullable=True - ) -``` -```python - schema_versions: Mapped[list["SchemaVersion"]] = relationship( - back_populates="dataset", - order_by="SchemaVersion.version", - cascade="all, delete-orphan", - foreign_keys="SchemaVersion.dataset_id", - ) -``` +@pytest_asyncio.fixture +async def async_client() -> AsyncGenerator[AsyncClient, None]: + from extralit_server import app + from extralit_server.api.routes import api_v1 -On `Record` (`models/database.py:219`), add `reference` beside `external_id`: + async def override_get_async_db(): + yield TestSession() -```python - # The source document identifier (DOI/PMID/filename) records were extracted from. - # Deliberately a plain indexed string, mirroring `Document.reference`: a reference may - # have no `documents` row yet, and the projection groups and paginates by this column. - reference: Mapped[str | None] = mapped_column(String, nullable=True, index=True) -``` + api_v1.dependency_overrides[get_async_db] = override_get_async_db -Add the composite index to `Record.__table_args__` (`models/database.py:256`), alongside the existing `UniqueConstraint`: + async with AsyncClient(app=app, base_url="http://testserver") as client: + yield client -```python - Index("ix_records_dataset_id_reference", "dataset_id", "reference"), + api_v1.dependency_overrides.clear() ``` -On `Field` (`models/database.py:65`), add the upsertable-columns declaration so schema publish can re-derive fields idempotently via `Field.upsert_many`, matching `Response.__upsertable_columns__` at `models/database.py:125`: +Note the override now lands on `api_v1` β€” previously it was registered on `api_v2`, so `test_rq_groups_workflow.py` was never actually getting the test session for its v1 route. Run that file before and after this change and compare: -```python - __upsertable_columns__ = {"title", "required", "settings"} +```bash +cd extralit-server && uv run pytest tests/integration/test_rq_groups_workflow.py -v ``` -- [ ] **Step 6: Add `reference` to `RecordFactory`** +If it was passing only by accident and now fails on the real session, fix the test β€” do not revert the override. Also delete the `annotator` / `annotator_auth_header` fixtures if nothing in the remaining tree uses them: -In `tests/factories.py`, find `RecordFactory` and add `reference = None` so the new column is explicit in every factory-built record. +```bash +cd extralit-server && grep -rn "annotator_auth_header\|annotator\b" tests/integration --include=*.py | grep -v __pycache__ +``` -- [ ] **Step 7: Generate the replacement migration** +- [ ] **Step 7: Unwire the mount and the exports** -Delete the four v2 migrations first so autogenerate does not see their tables: +In `_app.py`: delete `from extralit_server.api.v2 import api_v2` (`:27`) and `app.mount("/api/v2", api_v2)` (`:214`). -```bash -cd extralit-server && rm src/extralit_server/alembic/versions/9f3010c649c8_create_schema_and_schema_version_tables.py \ - src/extralit_server/alembic/versions/8136bc88ee3a_create_v2_records_table.py \ - src/extralit_server/alembic/versions/6393b1a01aa0_drop_schemas_kind.py \ - src/extralit_server/alembic/versions/c1510e93882a_create_v2_annotation_tables.py -``` +In `api/policies/v1/__init__.py`: delete the `SchemaPolicy` export (`:11`) and the `V2QuestionPolicy, V2ResponsePolicy, V2SuggestionPolicy` export (`:14`). -`c1510e93882a` was head and nothing revised it, so the chain tail is now `54d65879a68e`. Confirm: +In `cli/openapi_dump.py`: repoint `from extralit_server.api.v2 import api_v2` / `api_v2.openapi()` (`:18-20`) to `from extralit_server.api.routes import api_v1` / `api_v1.openapi()`, and update the docstring at `:16`. + +- [ ] **Step 8: Run the full suite and lint** ```bash -cd extralit-server && uv run alembic -c src/extralit_server/alembic.ini heads +cd extralit-server && uv run pytest tests -q --disable-warnings && uv run ruff check ``` -Expected: exactly one head, `54d65879a68e`. Then generate: +Expected: **green**. The mount test passes and roughly 54 v2 API tests plus the `contexts/v2` tests are gone from the collected count. `ruff` does not resolve cross-module imports, so the now-dangling `from extralit_server.api.schemas.v2 import ...` lines inside `contexts/v2/` do not trip it β€” leave them; Task 2 deletes those files. If the suite is *not* green, a test still imports something Step 4 deleted: delete that test here rather than deferring it. + +- [ ] **Step 9: Commit** ```bash -cd extralit-server && uv run alembic -c src/extralit_server/alembic.ini revision --autogenerate \ - -m "add schema_versions and record reference" +git add -A extralit-server/src/extralit_server/api extralit-server/src/extralit_server/_app.py \ + extralit-server/src/extralit_server/cli/openapi_dump.py extralit-server/tests +git commit -m "refactor(server)!: delete the /api/v2 surface + +Removes api/v2, api/schemas/v2, SchemaPolicy and the three V2*Policy classes +(they reproduced DatasetPolicy/QuestionPolicy/ResponsePolicy predicate for +predicate). openapi_dump now dumps v1." ``` -- [ ] **Step 8: Review the generated migration by hand** +--- -Autogenerate will not get the mutual FK right. Open the new file and verify it does exactly these five things, in this order, and nothing else β€” in particular it must **not** contain any `drop_table` for `schemas`/`v2_*` (those tables now only exist in databases created by the deleted migrations, and there is no production data): +### Task 2: Delete `contexts/v2`, `validators/v2`, `cli/index`, and the index-sync glue -1. `op.create_table("schema_versions", ...)` with `dataset_id` FK to `datasets` `ondelete="CASCADE"`, a self-FK `parent_version_id` `ondelete="SET NULL"`, `created_by` FK to `users` `ondelete="SET NULL"`, and `UniqueConstraint("dataset_id", "version", name="schema_version_dataset_id_version_uq")`. -2. `op.add_column("datasets", sa.Column("current_schema_version_id", sa.Uuid(), nullable=True))`. -3. `op.create_foreign_key("datasets_current_schema_version_id_fkey", "datasets", "schema_versions", ["current_schema_version_id"], ["id"], ondelete="SET NULL")` as a **separate** statement. -4. `op.add_column("records", sa.Column("reference", sa.String(), nullable=True))` + `op.create_index("ix_records_reference", "records", ["reference"])` + `op.create_index("ix_records_dataset_id_reference", "records", ["dataset_id", "reference"])`. -5. A `downgrade()` that reverses 1–4 in inverse order. +**Files:** +- Delete: `extralit-server/src/extralit_server/contexts/v2/` (entire directory) +- Delete: `extralit-server/src/extralit_server/validators/v2/` (entire directory) +- Delete: `extralit-server/src/extralit_server/cli/index/` (entire directory) +- Delete: `extralit-server/tests/unit/validators/v2/`, `extralit-server/tests/integration/cli/test_index_reindex.py` +- Modify: `extralit-server/src/extralit_server/cli/__init__.py`, `extralit-server/tests/integration/index/test_lancedb_engine.py` +- Keep untouched: `extralit-server/src/extralit_server/index/**` and `extralit-server/tests/{unit,integration}/index/**` -Set `down_revision = "54d65879a68e"`. +**Interfaces:** +- Consumes: Task 1 β€” `api/v2` is gone, so nothing imports these modules any more. Their survivors are re-homed later (Tasks 6, 9, 10); read them from `$V2REF` when you get there. +- Produces: nothing. `contexts/v2`, `validators/v2`, `cli/index` no longer exist. `tests/integration/index/test_lancedb_engine.py` no longer imports `V2RecordStatus`, which unblocks the `enums.py` sweep in Task 4. -- [ ] **Step 9: Apply the migration and run the tests** +- [ ] **Step 1: Confirm nothing outside these trees still imports them** ```bash -cd extralit-server && uv run alembic -c src/extralit_server/alembic.ini upgrade head \ - && uv run pytest tests/unit/models/test_schema_version_model.py -v +cd extralit-server && grep -rn "contexts\.v2\|contexts import v2\|validators\.v2\|validators import v2\|cli\.index\|index_sync" src tests --include=*.py \ + | grep -v "^src/extralit_server/contexts/v2/" \ + | grep -v "^src/extralit_server/validators/v2/" \ + | grep -v "^src/extralit_server/cli/index/" \ + | grep -v "^tests/integration/contexts/v2/" \ + | grep -v "^tests/unit/validators/v2/" ``` -Expected: 8 passed. +Expected remaining hits, all of which this task removes: `src/extralit_server/cli/__init__.py:4,13`. (`tests/integration/contexts/v2/` and `tests/unit/test_annotation_no_index_import.py` were already deleted in Task 1 Step 5.) If anything else appears, stop and fold that caller onto its v1 equivalent before deleting. -- [ ] **Step 10: Verify the migration round-trips** +- [ ] **Step 2: Delete** ```bash -cd extralit-server && uv run alembic -c src/extralit_server/alembic.ini downgrade -1 \ - && uv run alembic -c src/extralit_server/alembic.ini upgrade head +cd extralit-server && rm -rf src/extralit_server/contexts/v2 src/extralit_server/validators/v2 \ + src/extralit_server/cli/index tests/unit/validators/v2 \ + tests/integration/cli/test_index_reindex.py ``` -Expected: both succeed with no error. +- [ ] **Step 3: Unregister the index CLI** -- [ ] **Step 11: Commit** - -```bash -git add extralit-server/src/extralit_server/enums.py \ - extralit-server/src/extralit_server/models/database.py \ - extralit-server/src/extralit_server/alembic/versions/ \ - extralit-server/tests/factories.py \ - extralit-server/tests/unit/models/test_schema_version_model.py -git commit -m "feat(server): fold v2 schema/record columns into v1 models - -Adds SchemaVersion (FK datasets), Dataset.current_schema_version_id, -Record.reference, FieldType.column, and Field.__upsertable_columns__. -Replaces the four v2 migrations with one; drops columns_cache and -review_widgets, which the fields table supersedes." -``` - ---- - -### Task 2: `ColumnFieldSettings` and the deliberately-empty column validator +In `cli/__init__.py`, delete the `index_app` import and `app.add_typer(index_app, name="index")` (`:13`). Leave `cli/search_engine/` alone β€” that is v1's mature reindexer and stays. -The Pandera schema declares *all* columns; the editable subset gets Questions bound to it. So a column field must carry a dtype for the index mapping while validating no values. This task adds that type end-to-end. +- [ ] **Step 4: Fix the one index test that referenced a deleted enum** -**Files:** -- Modify: `extralit-server/src/extralit_server/api/schemas/v1/fields.py` -- Modify: `extralit-server/src/extralit_server/models/database.py` (one property) -- Modify: `extralit-server/src/extralit_server/validators/records.py` (comment only β€” see Step 4) -- Modify: `extralit-server/src/extralit_server/search_engine/commons.py` -- Test: `extralit-server/tests/unit/api/schemas/v1/test_field_settings.py` (create), `extralit-server/tests/unit/validators/test_column_fields.py` (create), `extralit-server/tests/unit/search_engine/test_column_field_mapping.py` (create) +`index/`'s *source* is model-agnostic, but one of its tests is not. `tests/integration/index/test_lancedb_engine.py:14-25` defines a local `_Rec` test double that imports `V2RecordStatus` and sets `schema_version_id`. Both go away in Task 4 (the `enums.py` sweep) and Task 4's `Record` model respectively, and this stub is the last thing holding `V2RecordStatus` alive β€” fix it here so Task 4 can delete the enum without tripping over it. It is a plain stub, so this is a two-line change: -**Interfaces:** -- Consumes: `FieldType.column` from Task 1. -- Produces: `ColumnFieldSettings`, `ColumnFieldSettingsCreate`, `ColumnFieldSettingsUpdate` in `api/schemas/v1/fields.py`, each with `type: Literal[FieldType.column]`, `dtype: str`, `nullable: bool = True`, `review: dict[str, Any] | None = None`; all three added to the `FieldSettings` / `FieldSettingsCreate` / `FieldSettingsUpdate` unions. `Field.is_column -> bool`. `es_mapping_for_field` handles `FieldType.column`. +```python +class _Rec: + def __init__(self, title, year, reference="pmid:1", external_id=None): + from extralit_server.enums import RecordStatus -**Note on `validators/records.py`:** it needs **no dispatch change**. `_validate_fields` (`validators/records.py:39`) calls one collector per type, and each collector selects its fields with `filter(lambda field: field.is_text, dataset.fields)` β€” so a `column` field is picked up by no collector and is never value-validated, which is exactly the required behavior. `_validate_extra_fields` still accepts it (it is in `dataset.fields`) and `_validate_required_fields` ignores it (`required=False`). The only change is a comment recording that the omission is deliberate, so a later reader does not "fix" it by adding a collector. + self.id = uuid4() + self.reference = reference + self.status = RecordStatus.pending + self.external_id = external_id + self.fields = {"title": title, "year": year} +``` -- [ ] **Step 1: Write the failing tests** +If dropping `schema_version_id` makes `index/mapping.py:record_to_row` fail, that is because `index/mapping.py:17` `SYSTEM_FIELDS` still lists it. Remove it there too and note in ENG-36 that the Lance row layout no longer pins a schema version β€” that pin is gone from `records` deliberately (bug 4). -Create `extralit-server/tests/unit/api/schemas/v1/test_field_settings.py`: +- [ ] **Step 5: Verify the index engine still stands alone** -```python -import pytest -from pydantic import TypeAdapter, ValidationError +```bash +cd extralit-server && uv run pytest tests/unit/index tests/integration/index -v +``` -from extralit_server.api.schemas.v1.fields import FieldSettings, FieldSettingsCreate +Expected: 24 passed. Any *other* failure means the engine had a hidden dependency on `models/v2` β€” record it in ENG-36 and fix the test, not by resurrecting `index_sync`. +- [ ] **Step 6: Verify the CLI still starts** -class TestColumnFieldSettings: - def test_column_settings_parse_from_the_discriminated_union(self): - settings = TypeAdapter(FieldSettings).validate_python( - {"type": "column", "dtype": "int64", "nullable": False} - ) - assert settings.type == "column" - assert settings.dtype == "int64" - assert settings.nullable is False - assert settings.review is None +```bash +cd extralit-server && uv run python -m extralit_server --help +``` - def test_column_settings_default_to_nullable_with_no_review_overlay(self): - settings = TypeAdapter(FieldSettingsCreate).validate_python({"type": "column", "dtype": "str"}) - assert settings.nullable is True - assert settings.review is None +Expected: help text with no `index` subcommand and with `search_engine` still present. - def test_column_settings_carry_an_opaque_review_overlay(self): - settings = TypeAdapter(FieldSettings).validate_python( - {"type": "column", "dtype": "str", "review": {"widget": "textarea", "rows": 4}} - ) - assert settings.review == {"widget": "textarea", "rows": 4} +- [ ] **Step 7: Full suite, lint, and commit** - def test_column_settings_require_a_dtype(self): - with pytest.raises(ValidationError): - TypeAdapter(FieldSettings).validate_python({"type": "column"}) +```bash +cd extralit-server && uv run pytest tests -q --disable-warnings && uv run ruff check ``` -Create `extralit-server/tests/unit/validators/test_column_fields.py`: +Expected: green. -```python -import pytest +```bash +git add -A extralit-server/src/extralit_server extralit-server/tests +git commit -m "refactor(server)!: delete contexts/v2, validators/v2, and cli/index -from extralit_server.api.schemas.v1.records import RecordCreate -from extralit_server.validators.records import RecordCreateValidator -from tests.factories import DatasetFactory, FieldFactory +The LanceDB engine in index/ is kept untouched; only its v2 glue goes. +Registering it as a SearchEngine implementation is ENG-36. Drops the +no-index-import guard, which is what made v2 review data unsearchable." +``` +--- -@pytest.mark.asyncio -class TestColumnFieldValidation: - async def _dataset_with_column_fields(self): - dataset = await DatasetFactory.create() - await FieldFactory.create( - dataset=dataset, name="population", settings={"type": "column", "dtype": "str", "nullable": True} - ) - await FieldFactory.create( - dataset=dataset, name="n_arms", settings={"type": "column", "dtype": "int64", "nullable": True} - ) - return await DatasetFactory.refresh_with_relationships(dataset) +### Task 3: Delete `models/v2` and the v2 test factories - async def test_column_fields_accept_any_json_scalar(self): - dataset = await self._dataset_with_column_fields() - # An int in a column field must NOT be rejected the way a text field would be: - # extraction inputs are typed by the Pandera schema, not validated here. - RecordCreateValidator.validate( - RecordCreate(fields={"population": "Kenya", "n_arms": 2}), dataset - ) +**Files:** +- Delete: `extralit-server/src/extralit_server/models/v2/` (entire directory) +- Delete: `extralit-server/tests/integration/models/v2/`, `extralit-server/tests/integration/test_enums_v2.py` +- Modify: `extralit-server/src/extralit_server/models/__init__.py`, `extralit-server/tests/factories.py` - async def test_column_fields_accept_null(self): - dataset = await self._dataset_with_column_fields() - RecordCreateValidator.validate(RecordCreate(fields={"population": None, "n_arms": None}), dataset) +**Interfaces:** +- Consumes: Tasks 1 and 2 β€” every importer is gone. +- Produces: `models/v2` no longer exists, so the `schema_versions` table name is free for Task 4's v1 `SchemaVersion`. The v2 factories β€” **including `SchemaVersionFactory`** β€” are deleted outright; Task 4 re-adds a `SchemaVersionFactory` against the new dataset-scoped model. - async def test_column_fields_accept_nested_json(self): - dataset = await self._dataset_with_column_fields() - RecordCreateValidator.validate( - RecordCreate(fields={"population": {"country": "Kenya"}, "n_arms": [1, 2]}), dataset - ) +- [ ] **Step 1: Confirm no importers remain** - async def test_undeclared_columns_are_still_rejected(self): - dataset = await self._dataset_with_column_fields() - with pytest.raises(Exception) as excinfo: - RecordCreateValidator.validate(RecordCreate(fields={"not_a_column": "x"}), dataset) - assert "not_a_column" in str(excinfo.value) +```bash +cd extralit-server && grep -rn "models\.v2\|models import v2\|V2Record\|V2Question\|V2Response\|V2Suggestion" src tests --include=*.py \ + | grep -v "^src/extralit_server/models/v2/" ``` -`DatasetFactory.refresh_with_relationships` may not exist. Check `tests/factories.py` first; if it does not, reload the dataset with the standard four `selectinload`s used at `api/handlers/v1/datasets/records_bulk.py:36-42` and inline that in the helper. +Expected hits only in `src/extralit_server/models/__init__.py:8-9`, `tests/factories.py:655-750`, `tests/integration/models/v2/`, and `tests/integration/test_enums_v2.py`. -- [ ] **Step 2: Run the tests to verify they fail** +- [ ] **Step 2: Delete** ```bash -cd extralit-server && uv run pytest tests/unit/api/schemas/v1/test_field_settings.py \ - tests/unit/validators/test_column_fields.py -v +cd extralit-server && rm -rf src/extralit_server/models/v2 tests/integration/models/v2 \ + tests/integration/test_enums_v2.py ``` -Expected: the unit tests fail on the discriminated union rejecting `type: "column"`; the integration tests fail in `_validate_extra_fields` or on a missing `column` branch. - -- [ ] **Step 3: Add the settings triple** +- [ ] **Step 3: Unwire the metadata registration** -In `api/schemas/v1/fields.py`, add the three classes after `TableFieldSettingsUpdate` (`fields.py:105`), following the exact shape of the neighbouring triples: +In `models/__init__.py`, delete lines 8–9 (`from .v2 import Schema, SchemaVersion` and `from .v2 import Record as V2Record`). Nothing replaces them: Task 4's `SchemaVersion` lives in `models/database.py` and is picked up by the `from .database import *` star-export already above these lines. -```python -class ColumnFieldSettings(BaseModel): - type: Literal[FieldType.column] - dtype: str - nullable: bool = True - # Opaque per-column review widget overlay, carried through to the client verbatim. - # Replaces the former SchemaVersion.review_widgets column. - review: dict[str, Any] | None = None +- [ ] **Step 4: Delete the v2 factories** +In `tests/factories.py`, delete `SchemaFactory` (`:655`), `SchemaVersionFactory` (`:666`), `V2RecordFactory` (`:679`), `V2QuestionFactory` (`:703`), `V2SuggestionFactory` (`:726`), and `V2ResponseFactory` (`:746`), plus the now-unused `from extralit_server.models.v2 import ...` imports at the top of the file. -class ColumnFieldSettingsCreate(BaseModel): - type: Literal[FieldType.column] - dtype: str - nullable: bool = True - review: dict[str, Any] | None = None +`SchemaVersionFactory` is **deleted, not retargeted** β€” the model it builds no longer exists and its replacement is not declared until Task 4. Task 4 Step 6 re-adds a dataset-scoped `SchemaVersionFactory` alongside `ColumnFieldFactory`. Do not leave a stub behind. +- [ ] **Step 5: Confirm the table name is free** -class ColumnFieldSettingsUpdate(UpdateSchema): - type: Literal[FieldType.column] - dtype: str | None = None - nullable: bool | None = None - review: dict[str, Any] | None = None +This is the whole point of Tasks 1–3 β€” verify it before moving on: - __non_nullable_fields__ = {"dtype"} +```bash +cd extralit-server && uv run python -c " +import extralit_server.models # importing the package is what registers every table +from extralit_server.models.base import DatabaseModel +names = set(DatabaseModel.metadata.tables) +assert 'schema_versions' not in names, 'schema_versions is still registered' +assert not {n for n in names if n.startswith('v2_') or n == 'schemas'}, sorted(names) +print('table registry is clean') +" ``` -Then add `ColumnFieldSettings` to the `FieldSettings` union (`fields.py:110`), `ColumnFieldSettingsCreate` to `FieldSettingsCreate` (`:119`), and `ColumnFieldSettingsUpdate` to `FieldSettingsUpdate` (`:128`). Import `Any` from `typing` if it is not already imported. - -- [ ] **Step 4: Add `Field.is_column` and record why no validator collector exists** +Expected: `table registry is clean`. -In `models/database.py`, add the property to `Field` next to `is_table` (`models/database.py:93`), matching the surrounding style: +- [ ] **Step 6: Run the full suite** -```python - @property - def is_column(self) -> bool: - return self.settings.get("type") == FieldType.column +```bash +cd extralit-server && uv run pytest tests -q --disable-warnings ``` -In `validators/records.py`, add this comment to `_validate_fields` (`records.py:39`) directly after the `_validate_custom_fields` call. **Add no collector** β€” the absence is the feature: - -```python - # No `_validate_column_fields` collector, deliberately. A column field is an - # extraction input declared by the dataset's Pandera schema version, not an - # annotator-editable answer: `Field.settings["dtype"]` exists to type the search - # index, not to gate ingestion. Because every collector above selects its fields - # with `filter(lambda field: field.is_, dataset.fields)`, column fields fall - # through all of them and are never value-validated β€” while - # `_validate_extra_fields` still requires them to be declared, and editable - # columns are validated on the Question/Response path by ResponseValueValidator. - # Do not "fix" this by adding a collector. -``` +Expected: all pass. Tasks 1–3 delete about 160 v2 tests (β‰ˆ54 API + the `contexts/v2` tests in Task 1, β‰ˆ92 across Tasks 1–2, β‰ˆ16 in Task 3), so the collected count should be down by roughly that much from the pre-plan baseline. Record the baseline before Task 1 and the count here β€” the Verification section's gate 5 needs both. Any *failure* here means a v1 module still depended on the v2 tree; fix it in the v1 code, not by restoring a v2 module. -- [ ] **Step 5: Run the tests to verify they pass** +- [ ] **Step 7: Lint and commit** ```bash -cd extralit-server && uv run pytest tests/unit/api/schemas/v1/test_field_settings.py \ - tests/unit/validators/test_column_fields.py -v +cd extralit-server && uv run ruff check ``` -Expected: 8 passed. - -- [ ] **Step 6: Write the failing search-mapping test** - -`search_engine/commons.py:152` `es_mapping_for_field` branches on `field.is_text` / `is_chat` / `is_custom` / `is_table`. A `column` field matches none, so it must get its own branch or `create_index` produces a `dynamic: "strict"` mapping with no property for the column β€” and every extraction record would then be rejected at index time. +```bash +git add -A extralit-server/src/extralit_server/models extralit-server/tests +git commit -m "refactor(server)!: delete models/v2 -Create `extralit-server/tests/unit/search_engine/test_column_field_mapping.py`: +Schema folds into Dataset; V2Record/V2Question/V2Response/V2Suggestion fold +into records/questions/responses/suggestions. Frees the schema_versions +table name for the v1 SchemaVersion that lands next." +``` -```python -import pytest +--- -from extralit_server.search_engine.commons import es_mapping_for_field -from tests.factories import FieldFactory +### Task 4: v1 model + enum foundation for the folded model +The first *building* task. No behavior yet β€” just the schema surface every later task builds on, so every subsequent task can be tested against a real database. It runs after the deletions because `models/v2/schemas.py` owned the `schema_versions` table name until Task 3 removed it. -def _field(dtype: str): - return FieldFactory.build(name="col", settings={"type": "column", "dtype": dtype, "nullable": True}) +**Files:** +- Modify: `extralit-server/src/extralit_server/enums.py` +- Modify: `extralit-server/src/extralit_server/models/database.py` +- Test: `extralit-server/tests/unit/test_enums.py` (create if absent), `extralit-server/tests/unit/models/test_schema_version_model.py` (create) +**Interfaces:** +- Consumes: Tasks 1–3 β€” the v2 tree is gone, so `schema_versions` is an unclaimed table name and `SchemaStatus`/`V2RecordStatus` have no importers left. +- Produces: `FieldType.column`; `SchemaVersion` (table `schema_versions`, FK `dataset_id`, columns `version: int`, `object_key: str`, `object_version_id: str | None`, `etag: str`, `checksum: str`, `parent_version_id: UUID | None`, `created_by: UUID | None`); `Dataset.current_schema_version_id: UUID | None`; `Dataset.schema_versions: list[SchemaVersion]`; `Record.reference: str | None`; `Field.__upsertable_columns__`. -class TestColumnFieldMapping: - @pytest.mark.parametrize( - ("dtype", "expected"), - [ - ("int64", "long"), - ("int32", "long"), - ("float64", "double"), - ("float32", "double"), - ("bool", "boolean"), - ("datetime64[ns]", "date_nanos"), - ], - ) - def test_numeric_and_temporal_dtypes_map_to_typed_es_fields(self, dtype, expected): - mapping = es_mapping_for_field(_field(dtype)) - assert next(iter(mapping.values()))["type"] == expected +- [ ] **Step 1: Write the failing tests** - def test_string_dtypes_map_to_text_with_a_keyword_subfield(self): - mapping = es_mapping_for_field(_field("str")) - es_field = next(iter(mapping.values())) - assert es_field["type"] == "text" - # A keyword sub-field is what makes terms filters and sorting on a column work. - assert es_field["fields"]["keyword"]["type"] == "keyword" +Create `extralit-server/tests/unit/models/test_schema_version_model.py`: - def test_an_unrecognized_dtype_falls_back_to_text(self): - mapping = es_mapping_for_field(_field("some_extension_dtype")) - assert next(iter(mapping.values()))["type"] == "text" +```python +import pytest +from sqlalchemy.ext.asyncio import AsyncSession - def test_the_mapping_is_keyed_under_the_record_field_namespace(self): - mapping = es_mapping_for_field(_field("str")) - assert list(mapping.keys()) == ["fields.col"] -``` +from extralit_server.enums import FieldType +from extralit_server.models.database import Dataset, Field, Record, SchemaVersion +from tests.factories import DatasetFactory, RecordFactory -Confirm the expected key by reading `es_field_for_record_field` (`search_engine/commons.py:144`) β€” if it namespaces differently than `fields.col`, use whatever it actually produces. -- [ ] **Step 7: Run it to verify it fails** +@pytest.mark.asyncio +class TestSchemaVersionModel: + async def test_field_type_column_exists(self): + assert FieldType.column == "column" -```bash -cd extralit-server && uv run pytest tests/unit/search_engine/test_column_field_mapping.py -v -``` + async def test_schema_version_belongs_to_dataset(self, db: AsyncSession): + dataset = await DatasetFactory.create() + version = await SchemaVersion.create( + db, + dataset_id=dataset.id, + version=1, + object_key=f"schemas/{dataset.id}/v1.json", + etag="etag-1", + checksum="checksum-1", + ) + assert version.dataset_id == dataset.id + assert version.version == 1 + assert version.parent_version_id is None -Expected: FAIL β€” `es_mapping_for_field` returns nothing (or raises) for a `column` field. + async def test_dataset_points_at_current_schema_version(self, db: AsyncSession): + dataset = await DatasetFactory.create() + version = await SchemaVersion.create( + db, dataset_id=dataset.id, version=1, object_key="k", etag="e", checksum="c" + ) + await dataset.update(db, current_schema_version_id=version.id) + await db.refresh(dataset, attribute_names=["schema_versions"]) + assert dataset.current_schema_version_id == version.id + assert [v.id for v in dataset.schema_versions] == [version.id] -- [ ] **Step 8: Add the `column` branch to the ES mapper** + async def test_schema_version_number_is_unique_per_dataset(self, db: AsyncSession): + dataset = await DatasetFactory.create() + await SchemaVersion.create(db, dataset_id=dataset.id, version=1, object_key="k", etag="e", checksum="c") + with pytest.raises(Exception): + await SchemaVersion.create(db, dataset_id=dataset.id, version=1, object_key="k2", etag="e", checksum="c") -In `search_engine/commons.py`, add a module-level table above `es_mapping_for_field` (`:152`) and a branch inside it, placed after the `is_table` branch: + async def test_record_carries_a_reference(self, db: AsyncSession): + record = await RecordFactory.create(reference="10.1000/j.foo.2020.01") + assert record.reference == "10.1000/j.foo.2020.01" -```python -# Pandera dtype -> Elasticsearch field type for FieldType.column. Anything unlisted -# indexes as text: a column's dtype is advisory for the index, and an unknown dtype -# must not make the dataset unindexable. -_ES_TYPE_BY_COLUMN_DTYPE = { - "int8": "long", - "int16": "long", - "int32": "long", - "int64": "long", - "float32": "double", - "float64": "double", - "bool": "boolean", - "datetime64[ns]": "date_nanos", -} -``` + async def test_record_reference_defaults_to_none(self, db: AsyncSession): + record = await RecordFactory.create() + assert record.reference is None -```python - elif field.is_column: - dtype = field.settings.get("dtype", "") - es_type = _ES_TYPE_BY_COLUMN_DTYPE.get(dtype) - if es_type is None: - # Keyword sub-field so terms filters and sorting work on the column. - return { - es_field_for_record_field(field.name): { - "type": "text", - "fields": {"keyword": {"type": "keyword", "ignore_above": 256}}, - } - } - return {es_field_for_record_field(field.name): {"type": es_type}} + async def test_field_is_upsertable(self): + assert Field.__upsertable_columns__ == {"title", "required", "settings"} + + async def test_deleting_dataset_deletes_its_schema_versions(self, db: AsyncSession): + dataset = await DatasetFactory.create() + await SchemaVersion.create(db, dataset_id=dataset.id, version=1, object_key="k", etag="e", checksum="c") + await dataset.delete(db) + assert (await SchemaVersion.get_by(db, dataset_id=dataset.id)) is None ``` -- [ ] **Step 9: Run the mapping test and check nothing else regressed** +- [ ] **Step 2: Run the tests to verify they fail** ```bash -cd extralit-server && uv run pytest tests/unit/search_engine/test_column_field_mapping.py -v \ - && uv run pytest tests/unit/validators tests/unit/api/schemas tests/unit/search_engine -q \ - && uv run ruff check +cd extralit-server && uv run pytest tests/unit/models/test_schema_version_model.py -v ``` -Expected: all pass, no lint errors. +Expected: collection error β€” `ImportError: cannot import name 'SchemaVersion' from 'extralit_server.models.database'`. -- [ ] **Step 10: Commit** +- [ ] **Step 3: Add `FieldType.column` and delete the v2 enums** + +In `enums.py`, add `column = "column"` to `FieldType` (after `table`). Then delete the `SchemaStatus` and `V2RecordStatus` classes at the bottom of the file entirely β€” Tasks 1–3 removed every importer (`models/v2/schemas.py` held the last `SchemaStatus` import; Task 2 Step 4 removed the last `V2RecordStatus` one), and leaving them would keep the parallel vocabulary alive. Verify before deleting: ```bash -git add extralit-server/src/extralit_server/api/schemas/v1/fields.py \ - extralit-server/src/extralit_server/models/database.py \ - extralit-server/src/extralit_server/validators/records.py \ - extralit-server/src/extralit_server/search_engine/commons.py \ - extralit-server/tests/unit/api/schemas/v1/test_field_settings.py \ - extralit-server/tests/unit/validators/test_column_fields.py \ - extralit-server/tests/unit/search_engine/test_column_field_mapping.py -git commit -m "feat(server): add FieldType.column β€” indexed, deliberately unvalidated +cd extralit-server && grep -rn "SchemaStatus\|V2RecordStatus" src tests --include=*.py +``` -Column fields declare a Pandera dtype that types the ES mapping without gating -ingestion; no validator collector selects them. Editable columns are reviewed -via a Question bound to them." +Expected: hits only in `enums.py` itself. If anything else appears, fold that caller onto its v1 equivalent first. + +```python +class FieldType(StrEnum): + text = "text" + image = "image" + chat = "chat" + custom = "custom" + table = "table" + # A column declared by the dataset's Pandera schema version. Carries a dtype for the + # index mapping and is deliberately not value-validated: columns are extraction inputs, + # not annotator-editable answers. Editable columns get a Question bound to them instead. + column = "column" ``` ---- +- [ ] **Step 4: Add the `SchemaVersion` model** -### Task 3: `contexts/schema_versions.py` β€” publish a version, derive the fields +In `models/database.py`, add the class next to `Dataset` (they change together). Copy the column list from the snapshot at `$V2REF/extralit-server/src/extralit_server/models/v2/schemas.py:44-67` but **omit `columns_cache` and `review_widgets`** β€” the `fields` table replaces both β€” and rename `schema_id` to `dataset_id`. -This is the heart of the fold: the one genuinely new capability, rewritten to write v1 `Field` rows instead of a `columns_cache` blob. +```python +class SchemaVersion(DatabaseModel): + """An immutable, object-store-backed Pandera schema body for a dataset. -**Files:** -- Create: `extralit-server/src/extralit_server/contexts/schema_versions.py` -- Test: `extralit-server/tests/unit/contexts/test_schema_versions.py` (create) -- Reference (do not modify): `extralit-server/src/extralit_server/contexts/v2/schemas.py`, `extralit-server/src/extralit_server/contexts/v2/schema_bodies.py`, `extralit-server/tests/integration/contexts/v2/test_schema_bodies.py` + The body itself lives in the workspace bucket at `object_key`; this row is the + pointer plus integrity metadata. The column manifest derived from the body is + materialized as `Field` rows on the dataset, so there is no cached copy here. + """ -**Interfaces:** -- Consumes: `SchemaVersion`, `Dataset.current_schema_version_id`, `Field.__upsertable_columns__` (Task 1); `ColumnFieldSettings` (Task 2). -- Produces: - - `object_key_for(dataset_id: UUID, version: int) -> str` - - `derive_column_fields(body_json: str, review_widgets: dict[str, dict] | None = None) -> list[dict]` β†’ `[{"name": str, "title": str, "required": bool, "settings": {"type": "column", "dtype": str, "nullable": bool, "review": dict | None}}]` - - `publish_version(db, search_engine, s3_client, dataset, *, body: str, bucket: str, review_widgets: dict | None = None, created_by: UUID | None = None) -> SchemaVersion` - - `list_versions(db, dataset) -> list[SchemaVersion]` - - `get_version_by_number(db, dataset_id: UUID, version: int) -> SchemaVersion | None` + __tablename__ = "schema_versions" -- [ ] **Step 1: Discover the real dtype strings before writing assertions** + dataset_id: Mapped[UUID] = mapped_column(ForeignKey("datasets.id", ondelete="CASCADE"), index=True) + version: Mapped[int] = mapped_column(index=True) + object_key: Mapped[str] = mapped_column(Text) + object_version_id: Mapped[str | None] = mapped_column(Text, nullable=True) + etag: Mapped[str] = mapped_column(String) + checksum: Mapped[str] = mapped_column(String) + parent_version_id: Mapped[UUID | None] = mapped_column( + ForeignKey("schema_versions.id", ondelete="SET NULL"), nullable=True + ) + created_by: Mapped[UUID | None] = mapped_column(ForeignKey("users.id", ondelete="SET NULL"), nullable=True) -`derive_column_fields` stores `str(column.dtype)`, and the exact strings Pandera produces are what the ES mapper's `_ES_TYPE_BY_COLUMN_DTYPE` table (Task 2) and the tests below must key on. Do not guess them: + dataset: Mapped["Dataset"] = relationship(back_populates="schema_versions", foreign_keys=[dataset_id]) -```bash -cd extralit-server && uv run python -c " -import pandera as pa -s = pa.DataFrameSchema({'a': pa.Column(str), 'b': pa.Column(pa.Int64), 'c': pa.Column(float), 'd': pa.Column(bool)}) -r = pa.DataFrameSchema.from_json(s.to_json()) -print({n: str(c.dtype) for n, c in r.columns.items()}) -" + __table_args__ = (UniqueConstraint("dataset_id", "version", name="schema_version_dataset_id_version_uq"),) + + def __repr__(self) -> str: + return f"SchemaVersion(id={self.id!s}, dataset_id={self.dataset_id!s}, version={self.version!r})" ``` -Cross-check the output against `index/mapping.py:21` `_ARROW_BY_DTYPE` and `:35` `_STRING_DTYPES` β€” those tables were built from the same round-trip, so they are the existing authority on which strings actually occur. Use the real strings in the tests below and in Task 2's mapping table; if they differ from `"str"` / `"int64"` as written here, the strings from this command win. +- [ ] **Step 5: Wire `Dataset`, `Record`, and `Field`** -- [ ] **Step 2: Write the failing tests** +On `Dataset` (`models/database.py:414`), add the pointer and the collection. `use_alter=True` is required: `datasets` and `schema_versions` reference each other, so Alembic must emit this FK as a separate `ALTER`. -Create `extralit-server/tests/unit/contexts/test_schema_versions.py`. The Pandera body fixture must match what `pa.DataFrameSchema.to_json()` emits β€” copy the fixture from `tests/integration/contexts/v2/test_schema_bodies.py` rather than hand-writing JSON. +```python + current_schema_version_id: Mapped[UUID | None] = mapped_column( + ForeignKey("schema_versions.id", ondelete="SET NULL", use_alter=True), nullable=True + ) +``` ```python -import json -from unittest.mock import AsyncMock + schema_versions: Mapped[list["SchemaVersion"]] = relationship( + back_populates="dataset", + order_by="SchemaVersion.version", + cascade="all, delete-orphan", + foreign_keys="SchemaVersion.dataset_id", + ) +``` -import pandera as pa -import pytest +On `Record` (`models/database.py:219`), add `reference` beside `external_id`: -from extralit_server.contexts import schema_versions -from extralit_server.enums import DatasetStatus, FieldType -from extralit_server.models.database import Field -from tests.factories import DatasetFactory +```python + # The source document identifier (DOI/PMID/filename) records were extracted from. + # Deliberately a plain indexed string, mirroring `Document.reference`: a reference may + # have no `documents` row yet, and the projection groups and paginates by this column. + reference: Mapped[str | None] = mapped_column(String, nullable=True, index=True) +``` +Add the composite index to `Record.__table_args__` (`models/database.py:256`), alongside the existing `UniqueConstraint`: -def _body() -> str: - return pa.DataFrameSchema( - { - "population": pa.Column(str, nullable=True), - "n_arms": pa.Column(pa.Int64, nullable=False), - } - ).to_json() +```python + Index("ix_records_dataset_id_reference", "dataset_id", "reference"), +``` +On `Field` (`models/database.py:65`), add the upsertable-columns declaration so schema publish can re-derive fields idempotently via `Field.upsert_many`, matching `Response.__upsertable_columns__` at `models/database.py:125`: -class TestDeriveColumnFields: - def test_one_field_per_pandera_column(self): - fields = schema_versions.derive_column_fields(_body()) - assert {f["name"] for f in fields} == {"population", "n_arms"} +```python + __upsertable_columns__ = {"title", "required", "settings"} +``` - def test_dtype_and_nullability_come_from_the_body(self): - by_name = {f["name"]: f for f in schema_versions.derive_column_fields(_body())} - assert by_name["n_arms"]["settings"]["dtype"] == "int64" - assert by_name["n_arms"]["settings"]["nullable"] is False - assert by_name["population"]["settings"]["nullable"] is True +- [ ] **Step 6: Restore the factories against the new model** - def test_every_derived_field_is_a_column_field(self): - for field in schema_versions.derive_column_fields(_body()): - assert field["settings"]["type"] == FieldType.column +In `tests/factories.py`: - def test_review_widgets_land_on_the_matching_field(self): - overlay = {"population": {"widget": "textarea"}} - by_name = {f["name"]: f for f in schema_versions.derive_column_fields(_body(), overlay)} - assert by_name["population"]["settings"]["review"] == {"widget": "textarea"} - assert by_name["n_arms"]["settings"]["review"] is None +1. Find `RecordFactory` and add `reference = None` so the new column is explicit in every factory-built record. +2. Re-add `SchemaVersionFactory` β€” Task 3 deleted the v2 one, and this is its dataset-scoped replacement. Import `SchemaVersion` from `extralit_server.models.database` alongside the other model imports: - def test_column_fields_are_never_required(self): - # `required` gates annotator input; a column is an ingestion input, never required. - for field in schema_versions.derive_column_fields(_body()): - assert field["required"] is False +```python +class SchemaVersionFactory(BaseFactory): + class Meta: + model = SchemaVersion + dataset = SubFactory(DatasetFactory) + version = 1 + object_key = LazyAttribute(lambda v: f"schemas/{v.dataset.id}/v{v.version}.json") + etag = "etag" + checksum = "checksum" +``` -@pytest.mark.asyncio -class TestPublishVersion: - async def test_publish_creates_version_one_and_marks_the_dataset_ready(self, db, mock_search_engine): - dataset = await DatasetFactory.create(status=DatasetStatus.draft) - version = await schema_versions.publish_version( - db, mock_search_engine, AsyncMock(), dataset, body=_body(), bucket="ws" - ) - assert version.version == 1 - assert version.dataset_id == dataset.id - assert dataset.current_schema_version_id == version.id - assert dataset.status == DatasetStatus.ready +3. Add a `ColumnFieldFactory` next to the existing `TextFieldFactory`, so Tasks 5, 9 and 10 have a one-liner for a declared column: - async def test_publish_materializes_column_fields(self, db, mock_search_engine): - dataset = await DatasetFactory.create(status=DatasetStatus.draft) - await schema_versions.publish_version( - db, mock_search_engine, AsyncMock(), dataset, body=_body(), bucket="ws" - ) - fields = await Field.list_by(db, dataset_id=dataset.id) - assert {f.name for f in fields} == {"population", "n_arms"} - assert all(f.settings["type"] == FieldType.column for f in fields) +```python +class ColumnFieldFactory(FieldFactory): + settings = {"type": "column", "dtype": "string", "nullable": True} +``` - async def test_republishing_is_idempotent_for_unchanged_columns(self, db, mock_search_engine): - dataset = await DatasetFactory.create(status=DatasetStatus.draft) - await schema_versions.publish_version( - db, mock_search_engine, AsyncMock(), dataset, body=_body(), bucket="ws" - ) - v2 = await schema_versions.publish_version( - db, mock_search_engine, AsyncMock(), dataset, body=_body(), bucket="ws" - ) - assert v2.version == 2 - fields = await Field.list_by(db, dataset_id=dataset.id) - assert len(fields) == 2 # upserted, not duplicated - - async def test_republishing_adds_newly_declared_columns(self, db, mock_search_engine): - dataset = await DatasetFactory.create(status=DatasetStatus.draft) - await schema_versions.publish_version( - db, mock_search_engine, AsyncMock(), dataset, body=_body(), bucket="ws" - ) - wider = pa.DataFrameSchema( - { - "population": pa.Column(str, nullable=True), - "n_arms": pa.Column(pa.Int64, nullable=False), - "outcome": pa.Column(str, nullable=True), - } - ).to_json() - await schema_versions.publish_version( - db, mock_search_engine, AsyncMock(), dataset, body=wider, bucket="ws" - ) - fields = await Field.list_by(db, dataset_id=dataset.id) - assert {f.name for f in fields} == {"population", "n_arms", "outcome"} +Match the surrounding factory style β€” check whether the file uses `factory.SubFactory` or a bare imported `SubFactory` and follow it. - async def test_second_version_links_the_first_as_parent(self, db, mock_search_engine): - dataset = await DatasetFactory.create(status=DatasetStatus.draft) - v1 = await schema_versions.publish_version( - db, mock_search_engine, AsyncMock(), dataset, body=_body(), bucket="ws" - ) - v2 = await schema_versions.publish_version( - db, mock_search_engine, AsyncMock(), dataset, body=_body(), bucket="ws" - ) - assert v2.parent_version_id == v1.id +- [ ] **Step 7: Generate the replacement migration** - async def test_publish_creates_the_search_index(self, db, mock_search_engine): - dataset = await DatasetFactory.create(status=DatasetStatus.draft) - await schema_versions.publish_version( - db, mock_search_engine, AsyncMock(), dataset, body=_body(), bucket="ws" - ) - mock_search_engine.create_index.assert_awaited() +Delete the four v2 migrations first so autogenerate does not see their tables: - async def test_publish_uploads_the_body_under_a_versioned_key(self, db, mock_search_engine): - dataset = await DatasetFactory.create(status=DatasetStatus.draft) - s3 = AsyncMock() - version = await schema_versions.publish_version( - db, mock_search_engine, s3, dataset, body=_body(), bucket="ws" - ) - assert version.object_key == f"schemas/{dataset.id}/v1.json" +```bash +cd extralit-server && rm src/extralit_server/alembic/versions/9f3010c649c8_create_schema_and_schema_version_tables.py \ + src/extralit_server/alembic/versions/8136bc88ee3a_create_v2_records_table.py \ + src/extralit_server/alembic/versions/6393b1a01aa0_drop_schemas_kind.py \ + src/extralit_server/alembic/versions/c1510e93882a_create_v2_annotation_tables.py +``` - async def test_invalid_body_is_rejected_before_anything_is_written(self, db, mock_search_engine): - dataset = await DatasetFactory.create(status=DatasetStatus.draft) - with pytest.raises(Exception): - await schema_versions.publish_version( - db, mock_search_engine, AsyncMock(), dataset, body="{not pandera}", bucket="ws" - ) - assert dataset.current_schema_version_id is None - assert await Field.list_by(db, dataset_id=dataset.id) == [] +`c1510e93882a` was head and nothing revised it, so the chain tail is now `54d65879a68e`. Confirm: +```bash +cd extralit-server && uv run alembic -c src/extralit_server/alembic.ini heads +``` -@pytest.mark.asyncio -class TestReadVersions: - async def test_list_versions_is_ordered_by_version_number(self, db, mock_search_engine): - dataset = await DatasetFactory.create(status=DatasetStatus.draft) - for _ in range(3): - await schema_versions.publish_version( - db, mock_search_engine, AsyncMock(), dataset, body=_body(), bucket="ws" - ) - assert [v.version for v in await schema_versions.list_versions(db, dataset)] == [1, 2, 3] +Expected: exactly one head, `54d65879a68e`. Then generate: - async def test_get_version_by_number(self, db, mock_search_engine): - dataset = await DatasetFactory.create(status=DatasetStatus.draft) - await schema_versions.publish_version( - db, mock_search_engine, AsyncMock(), dataset, body=_body(), bucket="ws" - ) - assert (await schema_versions.get_version_by_number(db, dataset.id, 1)).version == 1 - assert await schema_versions.get_version_by_number(db, dataset.id, 99) is None +```bash +cd extralit-server && uv run alembic -c src/extralit_server/alembic.ini revision --autogenerate \ + -m "add schema_versions and record reference" ``` -`mock_search_engine` and `db` are existing fixtures β€” confirm their names in `tests/conftest.py` and adjust if they differ. `Field.list_by` comes from `CRUDMixin`; if it is not present, use a plain `select(Field).where(Field.dataset_id == dataset.id)`. - -- [ ] **Step 3: Run the tests to verify they fail** +**If either command fails with `Can't locate revision identified by 'c1510e93882a'`**, the local database's `alembic_version` row points at a revision file you just deleted. `tests/conftest.py` migrates to `head` on session start and back to `base` on teardown, so this only happens after an interrupted run or on a hand-migrated dev database. There is no data to preserve (see Global Constraints) β€” drop and recreate the database, then re-run: ```bash -cd extralit-server && uv run pytest tests/unit/contexts/test_schema_versions.py -v +cd extralit-server && dropdb --if-exists extralit && createdb extralit ``` -Expected: collection error β€” `ModuleNotFoundError: extralit_server.contexts.schema_versions`. +- [ ] **Step 8: Review the generated migration by hand** -- [ ] **Step 4: Write the context** +Autogenerate will not get the mutual FK right, and β€” because Task 3 already removed `models/v2` from the metadata β€” it will also propose dropping every v2 table it still sees in the local database. **Delete those `drop_table` / `drop_index` statements by hand.** They must not ship: the migration chain no longer *creates* `schemas`, `v2_records`, `v2_questions`, `v2_responses` or `v2_suggestions`, so a from-scratch `upgrade head` (Task 12 Step 2) would fail trying to drop tables that were never made. Those tables only exist in databases built by the four migrations Step 7 deleted, and there is no production data (see Global Constraints) β€” dropping the database is the correct cleanup, not a migration. -Create `extralit-server/src/extralit_server/contexts/schema_versions.py`. Reuse `contexts/files.py:291` `put_object` and `contexts/files.py:73` `compute_hash` for storage; take the `flush()`-then-point ordering from `contexts/v2/schemas.py:117-122` (it exists to break the `datasets`↔`schema_versions` FK cycle) and the dtype/nullable extraction from `contexts/v2/schema_bodies.py:39` `derive_columns_cache`. Note that `derive_column_fields` is a pure function with no DB or S3 access β€” same boundary `schema_bodies.py` had. +Open the new file and verify it does exactly these five things, in this order, and nothing else: -```python -"""Versioned, object-store-backed Pandera schema bodies for a dataset. +1. `op.create_table("schema_versions", ...)` with `dataset_id` FK to `datasets` `ondelete="CASCADE"`, a self-FK `parent_version_id` `ondelete="SET NULL"`, `created_by` FK to `users` `ondelete="SET NULL"`, and `UniqueConstraint("dataset_id", "version", name="schema_version_dataset_id_version_uq")`. +2. `op.add_column("datasets", sa.Column("current_schema_version_id", sa.Uuid(), nullable=True))`. +3. `op.create_foreign_key("datasets_current_schema_version_id_fkey", "datasets", "schema_versions", ["current_schema_version_id"], ["id"], ondelete="SET NULL")` as a **separate** statement. +4. `op.add_column("records", sa.Column("reference", sa.String(), nullable=True))` + `op.create_index("ix_records_reference", "records", ["reference"])` + `op.create_index("ix_records_dataset_id_reference", "records", ["dataset_id", "reference"])`. +5. A `downgrade()` that reverses 1–4 in inverse order. -A dataset's record shape is declared by a Pandera schema whose body lives in the -workspace bucket. Publishing a version uploads the body, registers a `SchemaVersion` -pointer, and projects every declared column into a `Field` row β€” so the `fields` -table is the queryable column manifest and there is no cached copy of it. -""" +Set `down_revision = "54d65879a68e"`. -from typing import TYPE_CHECKING, Any -from uuid import UUID +- [ ] **Step 9: Apply the migration and run the tests** -import pandera as pa -from sqlalchemy import select -from sqlalchemy.ext.asyncio import AsyncSession +```bash +cd extralit-server && uv run alembic -c src/extralit_server/alembic.ini upgrade head \ + && uv run pytest tests/unit/models/test_schema_version_model.py -v +``` -from extralit_server.contexts import files as files_ctx -from extralit_server.enums import DatasetStatus, FieldType -from extralit_server.errors.future.base_errors import UnprocessableEntityError -from extralit_server.models.database import Dataset, Field, SchemaVersion -from extralit_server.search_engine import SearchEngine +Expected: 8 passed. -if TYPE_CHECKING: - from types_aiobotocore_s3.client import S3Client +- [ ] **Step 10: Verify the migration round-trips** +```bash +cd extralit-server && uv run alembic -c src/extralit_server/alembic.ini downgrade -1 \ + && uv run alembic -c src/extralit_server/alembic.ini upgrade head +``` -def object_key_for(dataset_id: UUID, version: int) -> str: - return f"schemas/{dataset_id}/v{version}.json" +Expected: both succeed with no error. +- [ ] **Step 11: Commit** -def derive_column_fields( - body_json: str, review_widgets: dict[str, dict[str, Any]] | None = None -) -> list[dict[str, Any]]: - """Project a Pandera body into `Field` row payloads, one per declared column. +```bash +git add extralit-server/src/extralit_server/enums.py \ + extralit-server/src/extralit_server/models/database.py \ + extralit-server/src/extralit_server/alembic/versions/ \ + extralit-server/tests/factories.py \ + extralit-server/tests/unit/models/test_schema_version_model.py +git commit -m "feat(server): fold v2 schema/record columns into v1 models - `review_widgets` is the out-of-band per-column widget overlay: Pandera's `to_json` - drops `Column.metadata`, so widget config cannot ride inside the body itself. - """ - review_widgets = review_widgets or {} - try: - schema = pa.DataFrameSchema.from_json(body_json) - except Exception as ex: - raise UnprocessableEntityError(f"schema body is not a valid Pandera DataFrameSchema: {ex}") from ex +Adds SchemaVersion (FK datasets), Dataset.current_schema_version_id, +Record.reference, FieldType.column, and Field.__upsertable_columns__. +Replaces the four v2 migrations with one; drops columns_cache and +review_widgets, which the fields table supersedes." +``` - return [ - { - "name": name, - "title": name, - # A column is an ingestion input, never annotator-required. - "required": False, - "settings": { - "type": FieldType.column, - "dtype": str(column.dtype), - "nullable": bool(column.nullable), - "review": review_widgets.get(name), - }, - } - for name, column in schema.columns.items() - ] +--- +### Task 5: `ColumnFieldSettings` and the deliberately-empty column validator -async def _next_version_number(db: AsyncSession, dataset_id: UUID) -> int: - stmt = select(SchemaVersion.version).where(SchemaVersion.dataset_id == dataset_id) - return max((await db.execute(stmt)).scalars().all(), default=0) + 1 +The Pandera schema declares *all* columns; the editable subset gets Questions bound to it. So a column field must carry a dtype for the index mapping while validating no values. This task adds that type end-to-end. +**Files:** +- Modify: `extralit-server/src/extralit_server/api/schemas/v1/fields.py` +- Modify: `extralit-server/src/extralit_server/models/database.py` (one property) +- Modify: `extralit-server/src/extralit_server/validators/records.py` (comment only β€” see Step 4) +- Modify: `extralit-server/src/extralit_server/search_engine/commons.py` +- Test: `extralit-server/tests/unit/api/schemas/v1/test_field_settings.py` (create), `extralit-server/tests/unit/validators/test_column_fields.py` (create), `extralit-server/tests/unit/search_engine/test_column_field_mapping.py` (create) -async def publish_version( - db: AsyncSession, - search_engine: SearchEngine, - s3_client: "S3Client", - dataset: Dataset, - *, - body: str, - bucket: str, - review_widgets: dict[str, dict[str, Any]] | None = None, - created_by: UUID | None = None, -) -> SchemaVersion: - """Upload a body, register the version, materialize its column fields, publish the dataset.""" - # Parse before any write so an invalid body leaves no version row and no S3 object. - field_payloads = derive_column_fields(body, review_widgets) +**Interfaces:** +- Consumes: `FieldType.column` from Task 4. +- Produces: `ColumnFieldSettings`, `ColumnFieldSettingsCreate`, `ColumnFieldSettingsUpdate` in `api/schemas/v1/fields.py`, each with `type: Literal[FieldType.column]`, `dtype: str`, `nullable: bool = True`, `review: dict[str, Any] | None = None`; all three added to the `FieldSettings` / `FieldSettingsCreate` / `FieldSettingsUpdate` unions. `Field.is_column -> bool`. `es_mapping_for_field` handles `FieldType.column`. - next_version = await _next_version_number(db, dataset.id) - key = object_key_for(dataset.id, next_version) - metadata = await files_ctx.put_object(s3_client, bucket, key, body, content_type="application/json") +**Note on `validators/records.py`:** it needs **no dispatch change**. `_validate_fields` (`validators/records.py:39`) calls one collector per type, and each collector selects its fields with `filter(lambda field: field.is_text, dataset.fields)` β€” so a `column` field is picked up by no collector and is never value-validated, which is exactly the required behavior. `_validate_extra_fields` still accepts it (it is in `dataset.fields`) and `_validate_required_fields` ignores it (`required=False`). The only change is a comment recording that the omission is deliberate, so a later reader does not "fix" it by adding a collector. - parent_id = dataset.current_schema_version_id +- [ ] **Step 1: Write the failing tests** - version = await SchemaVersion.create( - db, - dataset_id=dataset.id, - version=next_version, - object_key=key, - object_version_id=getattr(metadata, "version_id", None), - etag=metadata.etag, - checksum=files_ctx.compute_hash(body.encode("utf-8")), - parent_version_id=parent_id, - created_by=created_by, - autocommit=False, - ) - # Flush so `version.id` (a flush-time default) exists before `datasets` points at it. - # Doing both in one flush would form a datasets<->schema_versions FK cycle. - await db.flush() +Create `extralit-server/tests/unit/api/schemas/v1/test_field_settings.py`: - await Field.upsert_many( - db, - objects=[{**payload, "dataset_id": dataset.id} for payload in field_payloads], - constraints=[Field.name, Field.dataset_id], - autocommit=False, - ) +```python +import pytest +from pydantic import TypeAdapter, ValidationError - await dataset.update( - db, current_schema_version_id=version.id, status=DatasetStatus.ready, autocommit=False - ) - await db.commit() +from extralit_server.api.schemas.v1.fields import FieldSettings, FieldSettingsCreate - # Post-commit, outside the transaction β€” the repo-wide convention for index side effects. - await search_engine.create_index(dataset) - return version +class TestColumnFieldSettings: + def test_column_settings_parse_from_the_discriminated_union(self): + settings = TypeAdapter(FieldSettings).validate_python( + {"type": "column", "dtype": "int64", "nullable": False} + ) + assert settings.type == "column" + assert settings.dtype == "int64" + assert settings.nullable is False + assert settings.review is None + def test_column_settings_default_to_nullable_with_no_review_overlay(self): + settings = TypeAdapter(FieldSettingsCreate).validate_python({"type": "column", "dtype": "string"}) + assert settings.nullable is True + assert settings.review is None -async def list_versions(db: AsyncSession, dataset: Dataset) -> list[SchemaVersion]: - stmt = ( - select(SchemaVersion) - .where(SchemaVersion.dataset_id == dataset.id) - .order_by(SchemaVersion.version) - ) - return list((await db.execute(stmt)).scalars().all()) - + def test_column_settings_carry_an_opaque_review_overlay(self): + settings = TypeAdapter(FieldSettings).validate_python( + {"type": "column", "dtype": "string", "review": {"widget": "textarea", "rows": 4}} + ) + assert settings.review == {"widget": "textarea", "rows": 4} -async def get_version_by_number(db: AsyncSession, dataset_id: UUID, version: int) -> SchemaVersion | None: - stmt = select(SchemaVersion).where( - SchemaVersion.dataset_id == dataset_id, SchemaVersion.version == version - ) - return (await db.execute(stmt)).scalar_one_or_none() + def test_column_settings_require_a_dtype(self): + with pytest.raises(ValidationError): + TypeAdapter(FieldSettings).validate_python({"type": "column"}) ``` -- [ ] **Step 5: Run the tests to verify they pass** - -```bash -cd extralit-server && uv run pytest tests/unit/contexts/test_schema_versions.py -v -``` +Create `extralit-server/tests/unit/validators/test_column_fields.py`: -Expected: 15 passed. If `Field.upsert_many` raises about `objects` vs `schema`, read `models/mixins.py:125` and match its actual signature β€” `upsert_many(db, objects, constraints, autocommit)` per the mixin, but confirm whether it takes model instances or dicts and adapt. +```python +import pytest -- [ ] **Step 6: Verify a published dataset actually indexes against a real engine** +from extralit_server.api.schemas.v1.records import RecordCreate +from extralit_server.validators.records import RecordCreateValidator +from tests.factories import DatasetFactory, FieldFactory -Task 2 added the `column` branch to `es_mapping_for_field`, so this should pass β€” but that was a unit test against the mapper in isolation. This checks the whole `create_index` path with `dynamic: "strict"` and a real record: -```bash -cd extralit-server && uv run pytest tests/unit/contexts/test_schema_versions.py -v \ - && uv run pytest tests/unit/search_engine -q -``` +@pytest.mark.asyncio +class TestColumnFieldValidation: + async def _dataset_with_column_fields(self): + dataset = await DatasetFactory.create() + await FieldFactory.create( + dataset=dataset, name="population", settings={"type": "column", "dtype": "string", "nullable": True} + ) + await FieldFactory.create( + dataset=dataset, name="n_arms", settings={"type": "column", "dtype": "int64", "nullable": True} + ) + return await DatasetFactory.refresh_with_relationships(dataset) -Expected: all pass. If a record is rejected at index time with a `strict_dynamic_mapping_exception`, the derived field name and the ES property name disagree β€” compare `derive_column_fields`'s `name` against `es_field_for_record_field` (`search_engine/commons.py:144`) and fix the mapper, not the derivation. + async def test_column_fields_accept_any_json_scalar(self): + dataset = await self._dataset_with_column_fields() + # An int in a column field must NOT be rejected the way a text field would be: + # extraction inputs are typed by the Pandera schema, not validated here. + RecordCreateValidator.validate( + RecordCreate(fields={"population": "Kenya", "n_arms": 2}), dataset + ) -- [ ] **Step 7: Commit** + async def test_column_fields_accept_null(self): + dataset = await self._dataset_with_column_fields() + RecordCreateValidator.validate(RecordCreate(fields={"population": None, "n_arms": None}), dataset) -```bash -git add extralit-server/src/extralit_server/contexts/schema_versions.py \ - extralit-server/tests/unit/contexts/test_schema_versions.py -git commit -m "feat(server): contexts/schema_versions β€” publish a version, derive column fields + async def test_column_fields_accept_nested_json(self): + dataset = await self._dataset_with_column_fields() + RecordCreateValidator.validate( + RecordCreate(fields={"population": {"country": "Kenya"}, "n_arms": [1, 2]}), dataset + ) -Replaces contexts/v2/schemas.publish_version. columns_cache and review_widgets -are gone: the body's columns become Field rows, the widget overlay rides in -Field.settings['review']." + async def test_undeclared_columns_are_still_rejected(self): + dataset = await self._dataset_with_column_fields() + with pytest.raises(Exception) as excinfo: + RecordCreateValidator.validate(RecordCreate(fields={"not_a_column": "x"}), dataset) + assert "not_a_column" in str(excinfo.value) ``` ---- +`DatasetFactory.refresh_with_relationships` may not exist. Check `tests/factories.py` first; if it does not, reload the dataset with the standard four `selectinload`s used at `api/handlers/v1/datasets/records_bulk.py:36-42` and inline that in the helper. -### Task 4: Schema-version endpoints on `/api/v1` +- [ ] **Step 2: Run the tests to verify they fail** -**Files:** -- Create: `extralit-server/src/extralit_server/api/schemas/v1/schema_versions.py` -- Create: `extralit-server/src/extralit_server/api/handlers/v1/datasets/schema_versions.py` -- Modify: `extralit-server/src/extralit_server/api/handlers/v1/datasets/__init__.py` -- Test: `extralit-server/tests/unit/api/handlers/v1/datasets/test_schema_versions.py` (create) -- Reference: `extralit-server/src/extralit_server/api/v2/schemas.py:103-160`, `extralit-server/src/extralit_server/api/handlers/v1/datasets/questions.py` +```bash +cd extralit-server && uv run pytest tests/unit/api/schemas/v1/test_field_settings.py \ + tests/unit/validators/test_column_fields.py -v +``` -**Interfaces:** -- Consumes: `contexts/schema_versions.py` (Task 3). -- Produces: `POST /datasets/{dataset_id}/schema-versions` β†’ 201 `SchemaVersionRead`; `GET /datasets/{dataset_id}/schema-versions` β†’ `list[SchemaVersionRead]`; `GET /datasets/{dataset_id}/schema-versions/{version}` β†’ `SchemaVersionRead`. `SchemaVersionCreate{body: str, review_widgets: dict[str, dict] = {}}`, `SchemaVersionRead{id, dataset_id, version, object_key, object_version_id, etag, checksum, parent_version_id, created_by, inserted_at, updated_at}`. +Expected: the unit tests fail on the discriminated union rejecting `type: "column"`; the integration tests fail in `_validate_extra_fields` or on a missing `column` branch. -- [ ] **Step 1: Write the failing tests** +- [ ] **Step 3: Add the settings triple** -Create `extralit-server/tests/unit/api/handlers/v1/datasets/test_schema_versions.py`. Copy the auth and client conventions from a neighbouring v1 handler test β€” `tests/unit/api/handlers/v1/test_datasets.py` or `tests/unit/api/handlers/v1/test_fields.py` β€” and use `tests/unit/conftest.py`'s fixtures (`async_client`, `owner_auth_header`, `mock_search_engine`). These differ from the v2 suite's isolated `tests/integration/conftest.py`, which mounted `api_v2` and had no OpenSearch fixture. +In `api/schemas/v1/fields.py`, add the three classes after `TableFieldSettingsUpdate` (`fields.py:105`), following the exact shape of the neighbouring triples: ```python -import pandera as pa -import pytest - -from extralit_server.enums import DatasetStatus -from tests.factories import AdminFactory, AnnotatorFactory, DatasetFactory, WorkspaceFactory - - -def _body() -> str: - return pa.DataFrameSchema({"population": pa.Column(str, nullable=True)}).to_json() - +class ColumnFieldSettings(BaseModel): + type: Literal[FieldType.column] + dtype: str + nullable: bool = True + # Opaque per-column review widget overlay, carried through to the client verbatim. + # Replaces the former SchemaVersion.review_widgets column. + review: dict[str, Any] | None = None -@pytest.mark.asyncio -class TestPublishSchemaVersion: - async def test_owner_publishes_a_version(self, async_client, owner_auth_header, mock_search_engine): - dataset = await DatasetFactory.create(status=DatasetStatus.draft) - response = await async_client.post( - f"/api/v1/datasets/{dataset.id}/schema-versions", - headers=owner_auth_header, - json={"body": _body()}, - ) - assert response.status_code == 201, response.json() - assert response.json()["version"] == 1 - assert response.json()["dataset_id"] == str(dataset.id) - async def test_publish_returns_422_for_an_invalid_body(self, async_client, owner_auth_header): - dataset = await DatasetFactory.create(status=DatasetStatus.draft) - response = await async_client.post( - f"/api/v1/datasets/{dataset.id}/schema-versions", - headers=owner_auth_header, - json={"body": "{not pandera}"}, - ) - assert response.status_code == 422 +class ColumnFieldSettingsCreate(BaseModel): + type: Literal[FieldType.column] + dtype: str + nullable: bool = True + review: dict[str, Any] | None = None - async def test_publish_returns_404_for_an_unknown_dataset(self, async_client, owner_auth_header): - response = await async_client.post( - "/api/v1/datasets/00000000-0000-0000-0000-000000000000/schema-versions", - headers=owner_auth_header, - json={"body": _body()}, - ) - assert response.status_code == 404 - async def test_annotator_cannot_publish(self, async_client, mock_search_engine): - workspace = await WorkspaceFactory.create() - dataset = await DatasetFactory.create(workspace=workspace, status=DatasetStatus.draft) - annotator = await AnnotatorFactory.create(workspaces=[workspace]) - response = await async_client.post( - f"/api/v1/datasets/{dataset.id}/schema-versions", - headers={"X-Extralit-Api-Key": annotator.api_key}, - json={"body": _body()}, - ) - assert response.status_code == 403 +class ColumnFieldSettingsUpdate(UpdateSchema): + type: Literal[FieldType.column] + dtype: str | None = None + nullable: bool | None = None + review: dict[str, Any] | None = None - async def test_published_columns_are_readable_as_dataset_fields( - self, async_client, owner_auth_header, mock_search_engine - ): - dataset = await DatasetFactory.create(status=DatasetStatus.draft) - await async_client.post( - f"/api/v1/datasets/{dataset.id}/schema-versions", - headers=owner_auth_header, - json={"body": _body()}, - ) - # The former GET /schemas/{id}/columns is now the existing v1 fields endpoint. - fields = await async_client.get(f"/api/v1/datasets/{dataset.id}/fields", headers=owner_auth_header) - assert fields.status_code == 200 - assert [f["name"] for f in fields.json()["items"]] == ["population"] - assert fields.json()["items"][0]["settings"]["dtype"] == "str" + __non_nullable_fields__ = {"dtype"} +``` +Then add `ColumnFieldSettings` to the `FieldSettings` union (`fields.py:110`), `ColumnFieldSettingsCreate` to `FieldSettingsCreate` (`:119`), and `ColumnFieldSettingsUpdate` to `FieldSettingsUpdate` (`:128`). Import `Any` from `typing` if it is not already imported. -@pytest.mark.asyncio -class TestReadSchemaVersions: - async def test_list_versions(self, async_client, owner_auth_header, mock_search_engine): - dataset = await DatasetFactory.create(status=DatasetStatus.draft) - for _ in range(2): - await async_client.post( - f"/api/v1/datasets/{dataset.id}/schema-versions", - headers=owner_auth_header, - json={"body": _body()}, - ) - response = await async_client.get( - f"/api/v1/datasets/{dataset.id}/schema-versions", headers=owner_auth_header - ) - assert response.status_code == 200 - assert [v["version"] for v in response.json()] == [1, 2] +- [ ] **Step 4: Add `Field.is_column` and record why no validator collector exists** - async def test_list_versions_is_empty_for_an_unpublished_dataset(self, async_client, owner_auth_header): - dataset = await DatasetFactory.create(status=DatasetStatus.draft) - response = await async_client.get( - f"/api/v1/datasets/{dataset.id}/schema-versions", headers=owner_auth_header - ) - assert response.status_code == 200 - assert response.json() == [] +In `models/database.py`, add the property to `Field` next to `is_table` (`models/database.py:93`), matching the surrounding style: - async def test_get_version_by_number(self, async_client, owner_auth_header, mock_search_engine): - dataset = await DatasetFactory.create(status=DatasetStatus.draft) - await async_client.post( - f"/api/v1/datasets/{dataset.id}/schema-versions", - headers=owner_auth_header, - json={"body": _body()}, - ) - response = await async_client.get( - f"/api/v1/datasets/{dataset.id}/schema-versions/1", headers=owner_auth_header - ) - assert response.status_code == 200 - assert response.json()["version"] == 1 +```python + @property + def is_column(self) -> bool: + return self.settings.get("type") == FieldType.column +``` - async def test_get_unknown_version_returns_404(self, async_client, owner_auth_header): - dataset = await DatasetFactory.create(status=DatasetStatus.draft) - response = await async_client.get( - f"/api/v1/datasets/{dataset.id}/schema-versions/99", headers=owner_auth_header - ) - assert response.status_code == 404 +In `validators/records.py`, add this comment to `_validate_fields` (`records.py:39`) directly after the `_validate_custom_fields` call. **Add no collector** β€” the absence is the feature: - async def test_annotator_in_the_workspace_can_read_versions(self, async_client): - workspace = await WorkspaceFactory.create() - dataset = await DatasetFactory.create(workspace=workspace) - annotator = await AnnotatorFactory.create(workspaces=[workspace]) - response = await async_client.get( - f"/api/v1/datasets/{dataset.id}/schema-versions", - headers={"X-Extralit-Api-Key": annotator.api_key}, - ) - assert response.status_code == 200 +```python + # No `_validate_column_fields` collector, deliberately. A column field is an + # extraction input declared by the dataset's Pandera schema version, not an + # annotator-editable answer: `Field.settings["dtype"]` exists to type the search + # index, not to gate ingestion. Because every collector above selects its fields + # with `filter(lambda field: field.is_, dataset.fields)`, column fields fall + # through all of them and are never value-validated β€” while + # `_validate_extra_fields` still requires them to be declared, and editable + # columns are validated on the Question/Response path by ResponseValueValidator. + # Do not "fix" this by adding a collector. ``` -- [ ] **Step 2: Run the tests to verify they fail** +- [ ] **Step 5: Run the tests to verify they pass** ```bash -cd extralit-server && uv run pytest tests/unit/api/handlers/v1/datasets/test_schema_versions.py -v +cd extralit-server && uv run pytest tests/unit/api/schemas/v1/test_field_settings.py \ + tests/unit/validators/test_column_fields.py -v ``` -Expected: all 404 β€” the routes do not exist. +Expected: 8 passed. -- [ ] **Step 3: Write the pydantic contracts** +- [ ] **Step 6: Write the failing search-mapping test** + +`search_engine/commons.py:152` `es_mapping_for_field` branches on `field.is_text` / `is_chat` / `is_custom` / `is_table`. A `column` field matches none, so it must get its own branch or `create_index` produces a `dynamic: "strict"` mapping with no property for the column β€” and every extraction record would then be rejected at index time. -Create `api/schemas/v1/schema_versions.py`. Take the field list from `api/schemas/v2/schemas.py:23-58`, minus `columns_cache` and `review_widgets`, and rename `schema_id` β†’ `dataset_id`. Follow the v1 read-model convention: bare noun with `model_config = ConfigDict(from_attributes=True)`. +Create `extralit-server/tests/unit/search_engine/test_column_field_mapping.py`: ```python -from typing import Any -from uuid import UUID -from datetime import datetime +import pytest -from pydantic import BaseModel, ConfigDict, Field +from extralit_server.search_engine.commons import es_mapping_for_field +from tests.factories import FieldFactory -class SchemaVersionCreate(BaseModel): - """A new schema version. `body` is a Pandera `DataFrameSchema.to_json()` payload.""" +def _field(dtype: str): + return FieldFactory.build(name="col", settings={"type": "column", "dtype": dtype, "nullable": True}) - body: str - # Per-column widget overlay; Pandera's to_json drops Column.metadata, so this rides - # alongside and lands in each derived Field's settings["review"]. - review_widgets: dict[str, dict[str, Any]] = Field(default_factory=dict) +class TestColumnFieldMapping: + @pytest.mark.parametrize( + ("dtype", "expected"), + [ + ("int64", "long"), + ("int32", "long"), + ("float64", "double"), + ("float32", "double"), + ("bool", "boolean"), + ("datetime64[ns]", "date_nanos"), + ], + ) + def test_numeric_and_temporal_dtypes_map_to_typed_es_fields(self, dtype, expected): + mapping = es_mapping_for_field(_field(dtype)) + assert next(iter(mapping.values()))["type"] == expected -class SchemaVersionRead(BaseModel): - id: UUID - dataset_id: UUID - version: int - object_key: str - object_version_id: str | None - etag: str - checksum: str - parent_version_id: UUID | None - created_by: UUID | None - inserted_at: datetime - updated_at: datetime + def test_string_dtypes_map_to_text_with_a_keyword_subfield(self): + mapping = es_mapping_for_field(_field("string")) + es_field = next(iter(mapping.values())) + assert es_field["type"] == "text" + # A keyword sub-field is what makes terms filters and sorting on a column work. + assert es_field["fields"]["keyword"]["type"] == "keyword" - model_config = ConfigDict(from_attributes=True) + def test_an_unrecognized_dtype_falls_back_to_text(self): + mapping = es_mapping_for_field(_field("some_extension_dtype")) + assert next(iter(mapping.values()))["type"] == "text" + + def test_the_mapping_is_keyed_under_the_record_field_namespace(self): + mapping = es_mapping_for_field(_field("string")) + assert list(mapping.keys()) == ["fields.col"] ``` -- [ ] **Step 4: Write the handler** +Confirm the expected key by reading `es_field_for_record_field` (`search_engine/commons.py:144`) β€” if it namespaces differently than `fields.col`, use whatever it actually produces. -Create `api/handlers/v1/datasets/schema_versions.py`. Reuse `DatasetPolicy.publish` / `DatasetPolicy.get` β€” do **not** create a new policy class. Take the bucket resolution and `s3_client` dependency from `api/v2/schemas.py:103-133`; take the router shape (bare `APIRouter()`, literal paths, no prefix) from `api/handlers/v1/datasets/questions.py`. +- [ ] **Step 7: Run it to verify it fails** -```python -from typing import Annotated -from uuid import UUID +```bash +cd extralit-server && uv run pytest tests/unit/search_engine/test_column_field_mapping.py -v +``` -from fastapi import APIRouter, Depends, Security, status -from sqlalchemy.ext.asyncio import AsyncSession +Expected: FAIL β€” `es_mapping_for_field` returns nothing (or raises) for a `column` field. -from extralit_server.api.policies.v1 import DatasetPolicy, authorize -from extralit_server.api.schemas.v1.schema_versions import SchemaVersionCreate, SchemaVersionRead -from extralit_server.contexts import files as files_ctx -from extralit_server.contexts import schema_versions -from extralit_server.database import get_async_db -from extralit_server.errors.future import NotFoundError -from extralit_server.models.database import Dataset, User -from extralit_server.search_engine import SearchEngine, get_search_engine -from extralit_server.security import auth +- [ ] **Step 8: Add the `column` branch to the ES mapper** -router = APIRouter() +In `search_engine/commons.py`, add a module-level table above `es_mapping_for_field` (`:152`) and a branch inside it, placed after the `is_table` branch: +```python +# Pandera dtype -> Elasticsearch field type for FieldType.column. Anything unlisted +# indexes as text: a column's dtype is advisory for the index, and an unknown dtype +# must not make the dataset unindexable. +_ES_TYPE_BY_COLUMN_DTYPE = { + "int8": "long", + "int16": "long", + "int32": "long", + "int64": "long", + "float32": "double", + "float64": "double", + "bool": "boolean", + "datetime64[ns]": "date_nanos", +} +``` -@router.post( - "/datasets/{dataset_id}/schema-versions", - status_code=status.HTTP_201_CREATED, - response_model=SchemaVersionRead, -) -async def publish_schema_version( - *, - dataset_id: UUID, - version_create: SchemaVersionCreate, - db: Annotated[AsyncSession, Depends(get_async_db)], - search_engine: Annotated[SearchEngine, Depends(get_search_engine)], - s3_client=Depends(files_ctx.get_s3_client), - current_user: Annotated[User, Security(auth.get_current_user)], -): - dataset = await Dataset.get_or_raise(db, dataset_id, options=[selectinload(Dataset.workspace)]) - await authorize(current_user, DatasetPolicy.publish(dataset)) +```python + elif field.is_column: + dtype = field.settings.get("dtype", "") + es_type = _ES_TYPE_BY_COLUMN_DTYPE.get(dtype) + if es_type is None: + # Keyword sub-field so terms filters and sorting work on the column. + return { + es_field_for_record_field(field.name): { + "type": "text", + "fields": {"keyword": {"type": "keyword", "ignore_above": 256}}, + } + } + return {es_field_for_record_field(field.name): {"type": es_type}} +``` - return await schema_versions.publish_version( - db, - search_engine, - s3_client, - dataset, - body=version_create.body, - # One bucket per workspace, named exactly Workspace.name β€” contexts/files.py:381. - bucket=dataset.workspace.name, - review_widgets=version_create.review_widgets, - created_by=current_user.id, - ) +- [ ] **Step 9: Run the mapping test and check nothing else regressed** +```bash +cd extralit-server && uv run pytest tests/unit/search_engine/test_column_field_mapping.py -v \ + && uv run pytest tests/unit/validators tests/unit/api/schemas tests/unit/search_engine -q \ + && uv run ruff check +``` -@router.get("/datasets/{dataset_id}/schema-versions", response_model=list[SchemaVersionRead]) -async def list_schema_versions( - *, - dataset_id: UUID, - db: Annotated[AsyncSession, Depends(get_async_db)], - current_user: Annotated[User, Security(auth.get_current_user)], -): - dataset = await Dataset.get_or_raise(db, dataset_id) - await authorize(current_user, DatasetPolicy.get(dataset)) +Expected: all pass, no lint errors. - return await schema_versions.list_versions(db, dataset) +- [ ] **Step 10: Commit** +```bash +git add extralit-server/src/extralit_server/api/schemas/v1/fields.py \ + extralit-server/src/extralit_server/models/database.py \ + extralit-server/src/extralit_server/validators/records.py \ + extralit-server/src/extralit_server/search_engine/commons.py \ + extralit-server/tests/unit/api/schemas/v1/test_field_settings.py \ + extralit-server/tests/unit/validators/test_column_fields.py \ + extralit-server/tests/unit/search_engine/test_column_field_mapping.py +git commit -m "feat(server): add FieldType.column β€” indexed, deliberately unvalidated -@router.get("/datasets/{dataset_id}/schema-versions/{version}", response_model=SchemaVersionRead) -async def get_schema_version( - *, - dataset_id: UUID, - version: int, - db: Annotated[AsyncSession, Depends(get_async_db)], - current_user: Annotated[User, Security(auth.get_current_user)], -): - dataset = await Dataset.get_or_raise(db, dataset_id) - await authorize(current_user, DatasetPolicy.get(dataset)) +Column fields declare a Pandera dtype that types the ES mapping without gating +ingestion; no validator collector selects them. Editable columns are reviewed +via a Question bound to them." +``` - schema_version = await schema_versions.get_version_by_number(db, dataset.id, version) - if schema_version is None: - raise NotFoundError(f"SchemaVersion {version} not found for dataset {dataset_id}") +--- - return schema_version -``` +### Task 6: `contexts/schema_versions.py` β€” publish a version, derive the fields -Import `selectinload` from `sqlalchemy.orm`. Check the actual name and import path of the not-found error class used elsewhere in `api/handlers/v1/` and match it. +This is the heart of the fold: the one genuinely new capability, rewritten to write v1 `Field` rows instead of a `columns_cache` blob. -- [ ] **Step 5: Register the router** +**Files:** +- Create: `extralit-server/src/extralit_server/contexts/schema_versions.py` +- Test: `extralit-server/tests/unit/contexts/test_schema_versions.py` (create) +- Reference (snapshot, read-only): `$V2REF/extralit-server/src/extralit_server/contexts/v2/schemas.py`, `$V2REF/extralit-server/src/extralit_server/contexts/v2/schema_bodies.py`, `$V2REF/extralit-server/tests/integration/contexts/v2/test_schema_bodies.py` -In `api/handlers/v1/datasets/__init__.py`, import the new router and add `router.include_router(schema_versions_router)` alongside the existing four includes (`__init__.py:8-13`). +**Interfaces:** +- Consumes: `SchemaVersion`, `Dataset.current_schema_version_id`, `Field.__upsertable_columns__` (Task 4); `ColumnFieldSettings` (Task 5). +- Produces: + - `object_key_for(dataset_id: UUID, version: int) -> str` + - `derive_column_fields(body_json: str, review_widgets: dict[str, dict] | None = None) -> list[dict]` β€” **guard the empty case**: `models/mixins.py:132` raises `ValueError("Cannot upsert empty list of objects")`, so a valid but column-less body (`pa.DataFrameSchema({})`) makes `Field.upsert_many` 500. Either skip the call when `field_payloads` is empty or reject a column-less body with `UnprocessableEntityError`; add a test asserting whichever you choose. β†’ `[{"name": str, "title": str, "required": bool, "settings": {"type": "column", "dtype": str, "nullable": bool, "review": dict | None}}]` + - `publish_version(db, search_engine, s3_client, dataset, *, body: str, bucket: str, review_widgets: dict | None = None, created_by: UUID | None = None) -> SchemaVersion` + - `list_versions(db, dataset) -> list[SchemaVersion]` + - `get_version_by_number(db, dataset_id: UUID, version: int) -> SchemaVersion | None` -- [ ] **Step 6: Run the tests to verify they pass** +- [ ] **Step 1: Discover the real dtype strings before writing assertions** + +`derive_column_fields` stores `str(column.dtype)`, and the exact strings Pandera produces are what the ES mapper's `_ES_TYPE_BY_COLUMN_DTYPE` table (Task 5) and the tests below must key on. Do not guess them. + +**The repo already answers this and the plan was written wrong.** `index/mapping.py:21` `_ARROW_BY_DTYPE` and `:35` `_STRING_DTYPES` record the observed pandera/pandas round-trip values as `"string[pyarrow]"`, `"string"`, `"object"` (plus `"int64"`, `"int32"`, `"float64"`, `"float32"`, `"bool"`) β€” **never `"str"`**. Every `"str"` written as a dtype in Tasks 5–10 of this plan is wrong. `ColumnFieldFactory` was already corrected to `"string"` in the roborev fix pass; when you reach a `"str"` dtype literal in a later task, substitute a real value rather than copying it. Confirm with: ```bash -cd extralit-server && uv run pytest tests/unit/api/handlers/v1/datasets/test_schema_versions.py -v +cd extralit-server && uv run python -c " +import pandera as pa +s = pa.DataFrameSchema({'a': pa.Column(str), 'b': pa.Column(pa.Int64), 'c': pa.Column(float), 'd': pa.Column(bool)}) +r = pa.DataFrameSchema.from_json(s.to_json()) +print({n: str(c.dtype) for n, c in r.columns.items()}) +" ``` -Expected: 11 passed. +Cross-check the output against `index/mapping.py:21` `_ARROW_BY_DTYPE` and `:35` `_STRING_DTYPES` β€” those tables were built from the same round-trip, so they are the existing authority on which strings actually occur. Use the real strings in the tests below, and correct Task 5's mapping table in place if it is wrong; if they differ from `"str"` / `"int64"` as written here, the strings from this command win. -- [ ] **Step 7: Commit** +- [ ] **Step 2: Write the failing tests** -```bash -git add extralit-server/src/extralit_server/api/schemas/v1/schema_versions.py \ - extralit-server/src/extralit_server/api/handlers/v1/datasets/schema_versions.py \ - extralit-server/src/extralit_server/api/handlers/v1/datasets/__init__.py \ - extralit-server/tests/unit/api/handlers/v1/datasets/test_schema_versions.py -git commit -m "feat(server): schema-version endpoints on /api/v1 +Create `extralit-server/tests/unit/contexts/test_schema_versions.py`. The Pandera body fixture must match what `pa.DataFrameSchema.to_json()` emits β€” copy the fixture from `$V2REF/extralit-server/tests/integration/contexts/v2/test_schema_bodies.py` rather than hand-writing JSON. -Replaces POST/GET /api/v2/schemas/{id}/versions. GET /schemas/{id}/columns is -dropped: the derived columns are readable from GET /datasets/{id}/fields." -``` +```python +import json +from unittest.mock import AsyncMock ---- +import pandera as pa +import pytest -### Task 5: Carry `reference` through record create/upsert +from extralit_server.contexts import schema_versions +from extralit_server.enums import DatasetStatus, FieldType +from extralit_server.models.database import Field +from tests.factories import DatasetFactory -**Files:** -- Modify: `extralit-server/src/extralit_server/api/schemas/v1/records.py` -- Modify: `extralit-server/src/extralit_server/contexts/records_bulk.py` -- Modify: `extralit-server/src/extralit_server/contexts/records.py` -- Test: `extralit-server/tests/unit/api/handlers/v1/datasets/test_records_reference.py` (create) -- Reference: `extralit-server/src/extralit_server/contexts/v2/records.py:28-120` -**Interfaces:** -- Consumes: `Record.reference` (Task 1). -- Produces: `reference: str | None` on `Record`, `RecordCreate`, `RecordUpdate`, and `RecordUpsert` (inherited); `reference` persisted by `CreateRecordsBulk.create_records_bulk` and patched by `UpsertRecordsBulk.upsert_records_bulk` under `is_set("reference")` semantics. +def _body() -> str: + return pa.DataFrameSchema( + { + "population": pa.Column(str, nullable=True), + "n_arms": pa.Column(pa.Int64, nullable=False), + } + ).to_json() -- [ ] **Step 1: Write the failing tests** -Create `extralit-server/tests/unit/api/handlers/v1/datasets/test_records_reference.py`: +class TestDeriveColumnFields: + def test_one_field_per_pandera_column(self): + fields = schema_versions.derive_column_fields(_body()) + assert {f["name"] for f in fields} == {"population", "n_arms"} -```python -import pytest + def test_dtype_and_nullability_come_from_the_body(self): + by_name = {f["name"]: f for f in schema_versions.derive_column_fields(_body())} + assert by_name["n_arms"]["settings"]["dtype"] == "int64" + assert by_name["n_arms"]["settings"]["nullable"] is False + assert by_name["population"]["settings"]["nullable"] is True -from tests.factories import DatasetFactory, FieldFactory, RecordFactory, TextFieldFactory + def test_every_derived_field_is_a_column_field(self): + for field in schema_versions.derive_column_fields(_body()): + assert field["settings"]["type"] == FieldType.column + + def test_review_widgets_land_on_the_matching_field(self): + overlay = {"population": {"widget": "textarea"}} + by_name = {f["name"]: f for f in schema_versions.derive_column_fields(_body(), overlay)} + assert by_name["population"]["settings"]["review"] == {"widget": "textarea"} + assert by_name["n_arms"]["settings"]["review"] is None + + def test_column_fields_are_never_required(self): + # `required` gates annotator input; a column is an ingestion input, never required. + for field in schema_versions.derive_column_fields(_body()): + assert field["required"] is False @pytest.mark.asyncio -class TestRecordReference: - async def _ready_dataset(self): - dataset = await DatasetFactory.create(status="ready") - await TextFieldFactory.create(dataset=dataset, name="text") - return dataset +class TestPublishVersion: + async def test_publish_creates_version_one_and_marks_the_dataset_ready(self, db, mock_search_engine): + dataset = await DatasetFactory.create(status=DatasetStatus.draft) + version = await schema_versions.publish_version( + db, mock_search_engine, AsyncMock(), dataset, body=_body(), bucket="ws" + ) + assert version.version == 1 + assert version.dataset_id == dataset.id + assert dataset.current_schema_version_id == version.id + assert dataset.status == DatasetStatus.ready - async def test_bulk_create_persists_reference(self, async_client, owner_auth_header, mock_search_engine, db): - dataset = await self._ready_dataset() - response = await async_client.post( - f"/api/v1/datasets/{dataset.id}/records/bulk", - headers=owner_auth_header, - json={"items": [{"fields": {"text": "a"}, "reference": "10.1000/j.foo.2020.01"}]}, + async def test_publish_materializes_column_fields(self, db, mock_search_engine): + dataset = await DatasetFactory.create(status=DatasetStatus.draft) + await schema_versions.publish_version( + db, mock_search_engine, AsyncMock(), dataset, body=_body(), bucket="ws" ) - assert response.status_code == 201, response.json() - assert response.json()["items"][0]["reference"] == "10.1000/j.foo.2020.01" + fields = await Field.list_by(db, dataset_id=dataset.id) + assert {f.name for f in fields} == {"population", "n_arms"} + assert all(f.settings["type"] == FieldType.column for f in fields) - async def test_reference_is_optional(self, async_client, owner_auth_header, mock_search_engine): - dataset = await self._ready_dataset() - response = await async_client.post( - f"/api/v1/datasets/{dataset.id}/records/bulk", - headers=owner_auth_header, - json={"items": [{"fields": {"text": "a"}}]}, + async def test_republishing_is_idempotent_for_unchanged_columns(self, db, mock_search_engine): + dataset = await DatasetFactory.create(status=DatasetStatus.draft) + await schema_versions.publish_version( + db, mock_search_engine, AsyncMock(), dataset, body=_body(), bucket="ws" ) - assert response.status_code == 201 - assert response.json()["items"][0]["reference"] is None + v2 = await schema_versions.publish_version( + db, mock_search_engine, AsyncMock(), dataset, body=_body(), bucket="ws" + ) + assert v2.version == 2 + fields = await Field.list_by(db, dataset_id=dataset.id) + assert len(fields) == 2 # upserted, not duplicated - async def test_bulk_upsert_updates_reference(self, async_client, owner_auth_header, mock_search_engine, db): - dataset = await self._ready_dataset() - record = await RecordFactory.create(dataset=dataset, external_id="x1", reference="old") - response = await async_client.put( - f"/api/v1/datasets/{dataset.id}/records/bulk", - headers=owner_auth_header, - json={"items": [{"external_id": "x1", "reference": "new"}]}, + async def test_republishing_adds_newly_declared_columns(self, db, mock_search_engine): + dataset = await DatasetFactory.create(status=DatasetStatus.draft) + await schema_versions.publish_version( + db, mock_search_engine, AsyncMock(), dataset, body=_body(), bucket="ws" ) - assert response.status_code == 200, response.json() - await db.refresh(record) - assert record.reference == "new" + wider = pa.DataFrameSchema( + { + "population": pa.Column(str, nullable=True), + "n_arms": pa.Column(pa.Int64, nullable=False), + "outcome": pa.Column(str, nullable=True), + } + ).to_json() + await schema_versions.publish_version( + db, mock_search_engine, AsyncMock(), dataset, body=wider, bucket="ws" + ) + fields = await Field.list_by(db, dataset_id=dataset.id) + assert {f.name for f in fields} == {"population", "n_arms", "outcome"} - async def test_bulk_upsert_leaves_reference_alone_when_omitted( - self, async_client, owner_auth_header, mock_search_engine, db - ): - dataset = await self._ready_dataset() - record = await RecordFactory.create(dataset=dataset, external_id="x1", reference="keep") - await async_client.put( - f"/api/v1/datasets/{dataset.id}/records/bulk", - headers=owner_auth_header, - json={"items": [{"external_id": "x1", "metadata": {"a": 1}}]}, + async def test_second_version_links_the_first_as_parent(self, db, mock_search_engine): + dataset = await DatasetFactory.create(status=DatasetStatus.draft) + v1 = await schema_versions.publish_version( + db, mock_search_engine, AsyncMock(), dataset, body=_body(), bucket="ws" ) - await db.refresh(record) - assert record.reference == "keep" + v2 = await schema_versions.publish_version( + db, mock_search_engine, AsyncMock(), dataset, body=_body(), bucket="ws" + ) + assert v2.parent_version_id == v1.id - async def test_list_records_filters_by_reference(self, async_client, owner_auth_header): - dataset = await self._ready_dataset() - await RecordFactory.create(dataset=dataset, reference="doi-a") - await RecordFactory.create(dataset=dataset, reference="doi-b") - response = await async_client.get( - f"/api/v1/datasets/{dataset.id}/records?reference=doi-a", headers=owner_auth_header + async def test_publish_creates_the_search_index(self, db, mock_search_engine): + dataset = await DatasetFactory.create(status=DatasetStatus.draft) + await schema_versions.publish_version( + db, mock_search_engine, AsyncMock(), dataset, body=_body(), bucket="ws" ) - assert response.status_code == 200 - assert [r["reference"] for r in response.json()["items"]] == ["doi-a"] + mock_search_engine.create_index.assert_awaited() - async def test_a_reference_may_contain_slashes(self, async_client, owner_auth_header, mock_search_engine): - dataset = await self._ready_dataset() - await async_client.post( - f"/api/v1/datasets/{dataset.id}/records/bulk", - headers=owner_auth_header, - json={"items": [{"fields": {"text": "a"}, "reference": "10.1000/j.foo.2020.01"}]}, + async def test_publish_uploads_the_body_under_a_versioned_key(self, db, mock_search_engine): + dataset = await DatasetFactory.create(status=DatasetStatus.draft) + s3 = AsyncMock() + version = await schema_versions.publish_version( + db, mock_search_engine, s3, dataset, body=_body(), bucket="ws" ) - response = await async_client.get( - f"/api/v1/datasets/{dataset.id}/records", - headers=owner_auth_header, - params={"reference": "10.1000/j.foo.2020.01"}, + assert version.object_key == f"schemas/{dataset.id}/v1.json" + + async def test_invalid_body_is_rejected_before_anything_is_written(self, db, mock_search_engine): + dataset = await DatasetFactory.create(status=DatasetStatus.draft) + with pytest.raises(Exception): + await schema_versions.publish_version( + db, mock_search_engine, AsyncMock(), dataset, body="{not pandera}", bucket="ws" + ) + assert dataset.current_schema_version_id is None + assert await Field.list_by(db, dataset_id=dataset.id) == [] + + +@pytest.mark.asyncio +class TestReadVersions: + async def test_list_versions_is_ordered_by_version_number(self, db, mock_search_engine): + dataset = await DatasetFactory.create(status=DatasetStatus.draft) + for _ in range(3): + await schema_versions.publish_version( + db, mock_search_engine, AsyncMock(), dataset, body=_body(), bucket="ws" + ) + assert [v.version for v in await schema_versions.list_versions(db, dataset)] == [1, 2, 3] + + async def test_get_version_by_number(self, db, mock_search_engine): + dataset = await DatasetFactory.create(status=DatasetStatus.draft) + await schema_versions.publish_version( + db, mock_search_engine, AsyncMock(), dataset, body=_body(), bucket="ws" ) - assert len(response.json()["items"]) == 1 + assert (await schema_versions.get_version_by_number(db, dataset.id, 1)).version == 1 + assert await schema_versions.get_version_by_number(db, dataset.id, 99) is None +``` + +`mock_search_engine` and `db` are existing fixtures β€” confirm their names in `tests/conftest.py` and adjust if they differ. `Field.list_by` comes from `CRUDMixin`; if it is not present, use a plain `select(Field).where(Field.dataset_id == dataset.id)`. + +- [ ] **Step 3: Run the tests to verify they fail** + +```bash +cd extralit-server && uv run pytest tests/unit/contexts/test_schema_versions.py -v ``` -- [ ] **Step 2: Run the tests to verify they fail** +Expected: collection error β€” `ModuleNotFoundError: extralit_server.contexts.schema_versions`. + +- [ ] **Step 4: Write the context** + +Create `extralit-server/src/extralit_server/contexts/schema_versions.py`. Reuse `contexts/files.py:291` `put_object` and `contexts/files.py:73` `compute_hash` for storage; take the `flush()`-then-point ordering from `$V2REF/extralit-server/src/extralit_server/contexts/v2/schemas.py:117-122` (it exists to break the `datasets`↔`schema_versions` FK cycle) and the dtype/nullable extraction from `$V2REF/extralit-server/src/extralit_server/contexts/v2/schema_bodies.py:39` `derive_columns_cache`. Note that `derive_column_fields` is a pure function with no DB or S3 access β€” same boundary `schema_bodies.py` had. + +```python +"""Versioned, object-store-backed Pandera schema bodies for a dataset. + +A dataset's record shape is declared by a Pandera schema whose body lives in the +workspace bucket. Publishing a version uploads the body, registers a `SchemaVersion` +pointer, and projects every declared column into a `Field` row β€” so the `fields` +table is the queryable column manifest and there is no cached copy of it. +""" + +from typing import TYPE_CHECKING, Any +from uuid import UUID + +import pandera as pa +from sqlalchemy import select +from sqlalchemy.ext.asyncio import AsyncSession + +from extralit_server.contexts import files as files_ctx +from extralit_server.enums import DatasetStatus, FieldType +from extralit_server.errors.future.base_errors import UnprocessableEntityError +from extralit_server.models.database import Dataset, Field, SchemaVersion +from extralit_server.search_engine import SearchEngine + +if TYPE_CHECKING: + from types_aiobotocore_s3.client import S3Client + + +def object_key_for(dataset_id: UUID, version: int) -> str: + return f"schemas/{dataset_id}/v{version}.json" + + +def derive_column_fields( + body_json: str, review_widgets: dict[str, dict[str, Any]] | None = None +) -> list[dict[str, Any]]: + """Project a Pandera body into `Field` row payloads, one per declared column. + + `review_widgets` is the out-of-band per-column widget overlay: Pandera's `to_json` + drops `Column.metadata`, so widget config cannot ride inside the body itself. + """ + review_widgets = review_widgets or {} + try: + schema = pa.DataFrameSchema.from_json(body_json) + except Exception as ex: + raise UnprocessableEntityError(f"schema body is not a valid Pandera DataFrameSchema: {ex}") from ex + + return [ + { + "name": name, + "title": name, + # A column is an ingestion input, never annotator-required. + "required": False, + "settings": { + "type": FieldType.column, + "dtype": str(column.dtype), + "nullable": bool(column.nullable), + "review": review_widgets.get(name), + }, + } + for name, column in schema.columns.items() + ] + + +async def _next_version_number(db: AsyncSession, dataset_id: UUID) -> int: + stmt = select(SchemaVersion.version).where(SchemaVersion.dataset_id == dataset_id) + return max((await db.execute(stmt)).scalars().all(), default=0) + 1 + + +async def publish_version( + db: AsyncSession, + search_engine: SearchEngine, + s3_client: "S3Client", + dataset: Dataset, + *, + body: str, + bucket: str, + review_widgets: dict[str, dict[str, Any]] | None = None, + created_by: UUID | None = None, +) -> SchemaVersion: + """Upload a body, register the version, materialize its column fields, publish the dataset.""" + # Parse before any write so an invalid body leaves no version row and no S3 object. + field_payloads = derive_column_fields(body, review_widgets) + + next_version = await _next_version_number(db, dataset.id) + key = object_key_for(dataset.id, next_version) + metadata = await files_ctx.put_object(s3_client, bucket, key, body, content_type="application/json") -```bash -cd extralit-server && uv run pytest tests/unit/api/handlers/v1/datasets/test_records_reference.py -v -``` + parent_id = dataset.current_schema_version_id -Expected: failures on the unknown `reference` key and on the unsupported `reference` query param. + version = await SchemaVersion.create( + db, + dataset_id=dataset.id, + version=next_version, + object_key=key, + object_version_id=getattr(metadata, "version_id", None), + etag=metadata.etag, + checksum=files_ctx.compute_hash(body.encode("utf-8")), + parent_version_id=parent_id, + created_by=created_by, + autocommit=False, + ) + # Flush so `version.id` (a flush-time default) exists before `datasets` points at it. + # Doing both in one flush would form a datasets<->schema_versions FK cycle. + await db.flush() -- [ ] **Step 3: Add `reference` to the record schemas** + await Field.upsert_many( + db, + objects=[{**payload, "dataset_id": dataset.id} for payload in field_payloads], + constraints=[Field.name, Field.dataset_id], + autocommit=False, + ) -In `api/schemas/v1/records.py`, add `reference: str | None = None` to `Record` (`:64`), `RecordCreate` (`:106`), and `RecordUpdate` (`:169`). `RecordUpsert` (`:194`) inherits from `RecordCreate` so it gets it for free. Reuse the existing constraint from `api/schemas/v2/records.py:15` β€” `Reference = Annotated[constr(min_length=1, max_length=500), ...]` β€” and use it rather than a bare `str`. + await dataset.update( + db, current_schema_version_id=version.id, status=DatasetStatus.ready, autocommit=False + ) + await db.commit() -- [ ] **Step 4: Persist it in the bulk contexts** + # Post-commit, outside the transaction β€” the repo-wide convention for index side effects. + await search_engine.create_index(dataset) -In `contexts/records_bulk.py:38` `create_records_bulk`, add `reference=record_create.reference` to the `Record(...)` construction alongside `external_id`. + return version -In `contexts/records_bulk.py:145` `upsert_records_bulk`, add a `reference` branch to the partial-update block that already handles `is_set("metadata")` / `is_set("fields")`: -```python - if record_upsert.is_set("reference"): - record.reference = record_upsert.reference -``` +async def list_versions(db: AsyncSession, dataset: Dataset) -> list[SchemaVersion]: + stmt = ( + select(SchemaVersion) + .where(SchemaVersion.dataset_id == dataset.id) + .order_by(SchemaVersion.version) + ) + return list((await db.execute(stmt)).scalars().all()) -- [ ] **Step 5: Add the `reference` list filter** -In `contexts/records.py:85` `_build_list_records_query`, add a `reference: str | None = None` parameter and `if reference is not None: query = query.filter(Record.reference == reference)`. Thread it through `list_dataset_records` (`:24`) and add the query param to `list_dataset_records` in `api/handlers/v1/datasets/records.py:269`, following the existing `metadata`/`sort_by` param style. +async def get_version_by_number(db: AsyncSession, dataset_id: UUID, version: int) -> SchemaVersion | None: + stmt = select(SchemaVersion).where( + SchemaVersion.dataset_id == dataset_id, SchemaVersion.version == version + ) + return (await db.execute(stmt)).scalar_one_or_none() +``` -- [ ] **Step 6: Run the tests to verify they pass** +- [ ] **Step 5: Run the tests to verify they pass** ```bash -cd extralit-server && uv run pytest tests/unit/api/handlers/v1/datasets/test_records_reference.py -v +cd extralit-server && uv run pytest tests/unit/contexts/test_schema_versions.py -v ``` -Expected: 6 passed. +Expected: 15 passed. If `Field.upsert_many` raises about `objects` vs `schema`, read `models/mixins.py:125` and match its actual signature β€” `upsert_many(db, objects, constraints, autocommit)` per the mixin, but confirm whether it takes model instances or dicts and adapt. -- [ ] **Step 7: Verify no existing record test regressed** +- [ ] **Step 6: Verify a published dataset actually indexes against a real engine** + +Task 5 added the `column` branch to `es_mapping_for_field`, so this should pass β€” but that was a unit test against the mapper in isolation. This checks the whole `create_index` path with `dynamic: "strict"` and a real record: ```bash -cd extralit-server && uv run pytest tests/unit/api/handlers/v1/datasets tests/unit/contexts -q +cd extralit-server && uv run pytest tests/unit/contexts/test_schema_versions.py -v \ + && uv run pytest tests/unit/search_engine -q ``` -Expected: all pass. +Expected: all pass. If a record is rejected at index time with a `strict_dynamic_mapping_exception`, the derived field name and the ES property name disagree β€” compare `derive_column_fields`'s `name` against `es_field_for_record_field` (`search_engine/commons.py:144`) and fix the mapper, not the derivation. -- [ ] **Step 8: Commit** +- [ ] **Step 7: Commit** ```bash -git add extralit-server/src/extralit_server/api/schemas/v1/records.py \ - extralit-server/src/extralit_server/contexts/records_bulk.py \ - extralit-server/src/extralit_server/contexts/records.py \ - extralit-server/src/extralit_server/api/handlers/v1/datasets/records.py \ - extralit-server/tests/unit/api/handlers/v1/datasets/test_records_reference.py -git commit -m "feat(server): carry record reference through v1 bulk create/upsert and list +git add extralit-server/src/extralit_server/contexts/schema_versions.py \ + extralit-server/tests/unit/contexts/test_schema_versions.py +git commit -m "feat(server): contexts/schema_versions β€” publish a version, derive column fields -Replaces V2Record.reference. Drops the schema_version_id pin (its CASCADE -silently deleted records) and status=discarded (record status is derived from -response distribution; discard is a response status)." +Replaces contexts/v2/schemas.publish_version. columns_cache and review_widgets +are gone: the body's columns become Field rows, the widget overlay rides in +Field.settings['review']." ``` --- -### Task 6: Question column bindings on v1 questions +### Task 7: Schema-version endpoints on `/api/v1` **Files:** -- Modify: `extralit-server/src/extralit_server/api/schemas/v1/questions.py` -- Modify: `extralit-server/src/extralit_server/validators/questions.py` -- Test: `extralit-server/tests/unit/api/handlers/v1/datasets/questions/test_column_binding.py` (create) -- Reference: `extralit-server/src/extralit_server/validators/v2/questions.py`, `extralit-server/tests/unit/validators/v2/test_question_binding.py` +- Create: `extralit-server/src/extralit_server/api/schemas/v1/schema_versions.py` +- Create: `extralit-server/src/extralit_server/api/handlers/v1/datasets/schema_versions.py` +- Modify: `extralit-server/src/extralit_server/api/handlers/v1/datasets/__init__.py` +- Test: `extralit-server/tests/unit/api/handlers/v1/datasets/test_schema_versions.py` (create) +- Reference: `$V2REF/extralit-server/src/extralit_server/api/v2/schemas.py:103-160` (snapshot), `extralit-server/src/extralit_server/api/handlers/v1/datasets/questions.py` (live) **Interfaces:** -- Consumes: `FieldType.column` (Task 2), derived `Field` rows (Task 3). -- Produces: `columns: list[str] | None` on every `QuestionSettings*` variant that can bind (`TextQuestionSettings*`, `TableQuestionSettings*`); `QuestionColumnBindingValidator.validate(settings: dict, dataset: Dataset) -> None` in `validators/questions.py`, called from `QuestionCreateValidator.validate` and `QuestionUpdateValidator.validate`. It takes the raw settings dict rather than the pydantic model because the two callers hold different types (`QuestionCreate.settings` vs a partial `QuestionUpdate.settings`) and both can `model_dump()` into it. +- Consumes: `contexts/schema_versions.py` (Task 6). +- Produces: `POST /datasets/{dataset_id}/schema-versions` β†’ 201 `SchemaVersionRead`; `GET /datasets/{dataset_id}/schema-versions` β†’ `list[SchemaVersionRead]`; `GET /datasets/{dataset_id}/schema-versions/{version}` β†’ `SchemaVersionRead`. `SchemaVersionCreate{body: str, review_widgets: dict[str, dict] = {}}`, `SchemaVersionRead{id, dataset_id, version, object_key, object_version_id, etag, checksum, parent_version_id, created_by, inserted_at, updated_at}`. - [ ] **Step 1: Write the failing tests** -Create `extralit-server/tests/unit/api/handlers/v1/datasets/questions/test_column_binding.py`: +**First, re-home the `get_s3_client` override.** Task 1 Step 6 deleted `tests/integration/conftest.py`'s `override_get_s3_client`, which was the *only* place in the test tree stubbing `files_ctx.get_s3_client`; `tests/unit/conftest.py` overrides only `get_async_db` and `get_search_engine`. This handler declares `s3_client=Depends(files_ctx.get_s3_client)` and `publish_version` calls the real `files_ctx.put_object`, so without a stub these tests hit `create_s3_client()` and real object storage instead of returning 201. Either add `files_ctx.get_s3_client: ` to `tests/unit/conftest.py`'s `api_v1.dependency_overrides` dict (and pop it in the same teardown loop), or monkeypatch `contexts.schema_versions.files_ctx.put_object` in this test module. Pick one and do it before writing the tests below. + +Create `extralit-server/tests/unit/api/handlers/v1/datasets/test_schema_versions.py`. Copy the auth and client conventions from a neighbouring v1 handler test β€” `tests/unit/api/handlers/v1/test_datasets.py` or `tests/unit/api/handlers/v1/test_fields.py` β€” and use `tests/unit/conftest.py`'s fixtures (`async_client`, `owner_auth_header`, `mock_search_engine`). These differ from the v2 suite's isolated `tests/integration/conftest.py`, which mounted `api_v2` and had no OpenSearch fixture. ```python +import pandera as pa import pytest -from tests.factories import DatasetFactory, FieldFactory +from extralit_server.enums import DatasetStatus +from tests.factories import AdminFactory, AnnotatorFactory, DatasetFactory, WorkspaceFactory -@pytest.mark.asyncio -class TestQuestionColumnBinding: - async def _dataset_with_columns(self, *names): - dataset = await DatasetFactory.create() - for name in names: - await FieldFactory.create( - dataset=dataset, name=name, settings={"type": "column", "dtype": "str", "nullable": True} - ) - return dataset +def _body() -> str: + return pa.DataFrameSchema({"population": pa.Column(str, nullable=True)}).to_json() - async def test_question_binds_to_a_declared_column(self, async_client, owner_auth_header): - dataset = await self._dataset_with_columns("population") + +@pytest.mark.asyncio +class TestPublishSchemaVersion: + async def test_owner_publishes_a_version(self, async_client, owner_auth_header, mock_search_engine): + dataset = await DatasetFactory.create(status=DatasetStatus.draft) response = await async_client.post( - f"/api/v1/datasets/{dataset.id}/questions", + f"/api/v1/datasets/{dataset.id}/schema-versions", headers=owner_auth_header, - json={ - "name": "population_review", - "title": "Population", - "settings": {"type": "text", "use_markdown": False, "columns": ["population"]}, - }, + json={"body": _body()}, ) assert response.status_code == 201, response.json() - assert response.json()["settings"]["columns"] == ["population"] + assert response.json()["version"] == 1 + assert response.json()["dataset_id"] == str(dataset.id) - async def test_binding_to_an_undeclared_column_is_rejected(self, async_client, owner_auth_header): - dataset = await self._dataset_with_columns("population") + async def test_publish_returns_422_for_an_invalid_body(self, async_client, owner_auth_header): + dataset = await DatasetFactory.create(status=DatasetStatus.draft) response = await async_client.post( - f"/api/v1/datasets/{dataset.id}/questions", + f"/api/v1/datasets/{dataset.id}/schema-versions", headers=owner_auth_header, - json={ - "name": "q", - "title": "Q", - "settings": {"type": "text", "use_markdown": False, "columns": ["nope"]}, - }, + json={"body": "{not pandera}"}, ) assert response.status_code == 422 - assert "nope" in response.text - async def test_a_scalar_question_binds_to_exactly_one_column(self, async_client, owner_auth_header): - dataset = await self._dataset_with_columns("a", "b") + async def test_publish_returns_404_for_an_unknown_dataset(self, async_client, owner_auth_header): response = await async_client.post( - f"/api/v1/datasets/{dataset.id}/questions", + "/api/v1/datasets/00000000-0000-0000-0000-000000000000/schema-versions", headers=owner_auth_header, - json={ - "name": "q", - "title": "Q", - "settings": {"type": "text", "use_markdown": False, "columns": ["a", "b"]}, - }, + json={"body": _body()}, ) - assert response.status_code == 422 + assert response.status_code == 404 - async def test_a_table_question_binds_to_many_columns(self, async_client, owner_auth_header): - dataset = await self._dataset_with_columns("a", "b") + async def test_annotator_cannot_publish(self, async_client, mock_search_engine): + workspace = await WorkspaceFactory.create() + dataset = await DatasetFactory.create(workspace=workspace, status=DatasetStatus.draft) + annotator = await AnnotatorFactory.create(workspaces=[workspace]) response = await async_client.post( - f"/api/v1/datasets/{dataset.id}/questions", - headers=owner_auth_header, - json={ - "name": "t", - "title": "T", - "settings": {"type": "table", "columns": ["a", "b"]}, - }, + f"/api/v1/datasets/{dataset.id}/schema-versions", + headers={"X-Extralit-Api-Key": annotator.api_key}, + json={"body": _body()}, ) - assert response.status_code == 201, response.json() - assert response.json()["settings"]["columns"] == ["a", "b"] + assert response.status_code == 403 - async def test_an_empty_binding_is_rejected(self, async_client, owner_auth_header): - dataset = await self._dataset_with_columns("a") - response = await async_client.post( - f"/api/v1/datasets/{dataset.id}/questions", + async def test_published_columns_are_readable_as_dataset_fields( + self, async_client, owner_auth_header, mock_search_engine + ): + dataset = await DatasetFactory.create(status=DatasetStatus.draft) + await async_client.post( + f"/api/v1/datasets/{dataset.id}/schema-versions", headers=owner_auth_header, - json={"name": "q", "title": "Q", "settings": {"type": "table", "columns": []}}, + json={"body": _body()}, ) - assert response.status_code == 422 + # The former GET /schemas/{id}/columns is now the existing v1 fields endpoint. + fields = await async_client.get(f"/api/v1/datasets/{dataset.id}/fields", headers=owner_auth_header) + assert fields.status_code == 200 + assert [f["name"] for f in fields.json()["items"]] == ["population"] + # NOT "str" β€” see Task 6 Step 1; pandera emits "string"/"string[pyarrow]"/"object". + assert fields.json()["items"][0]["settings"]["dtype"] in {"string", "string[pyarrow]", "object"} - async def test_questions_without_a_binding_are_still_valid(self, async_client, owner_auth_header): - # A plain annotation dataset has no column fields and no bindings β€” unchanged v1 behavior. - dataset = await DatasetFactory.create() - response = await async_client.post( - f"/api/v1/datasets/{dataset.id}/questions", + +@pytest.mark.asyncio +class TestReadSchemaVersions: + async def test_list_versions(self, async_client, owner_auth_header, mock_search_engine): + dataset = await DatasetFactory.create(status=DatasetStatus.draft) + for _ in range(2): + await async_client.post( + f"/api/v1/datasets/{dataset.id}/schema-versions", + headers=owner_auth_header, + json={"body": _body()}, + ) + response = await async_client.get( + f"/api/v1/datasets/{dataset.id}/schema-versions", headers=owner_auth_header + ) + assert response.status_code == 200 + assert [v["version"] for v in response.json()] == [1, 2] + + async def test_list_versions_is_empty_for_an_unpublished_dataset(self, async_client, owner_auth_header): + dataset = await DatasetFactory.create(status=DatasetStatus.draft) + response = await async_client.get( + f"/api/v1/datasets/{dataset.id}/schema-versions", headers=owner_auth_header + ) + assert response.status_code == 200 + assert response.json() == [] + + async def test_get_version_by_number(self, async_client, owner_auth_header, mock_search_engine): + dataset = await DatasetFactory.create(status=DatasetStatus.draft) + await async_client.post( + f"/api/v1/datasets/{dataset.id}/schema-versions", headers=owner_auth_header, - json={"name": "q", "title": "Q", "settings": {"type": "text", "use_markdown": False}}, + json={"body": _body()}, + ) + response = await async_client.get( + f"/api/v1/datasets/{dataset.id}/schema-versions/1", headers=owner_auth_header + ) + assert response.status_code == 200 + assert response.json()["version"] == 1 + + async def test_get_unknown_version_returns_404(self, async_client, owner_auth_header): + dataset = await DatasetFactory.create(status=DatasetStatus.draft) + response = await async_client.get( + f"/api/v1/datasets/{dataset.id}/schema-versions/99", headers=owner_auth_header + ) + assert response.status_code == 404 + + async def test_annotator_in_the_workspace_can_read_versions(self, async_client): + workspace = await WorkspaceFactory.create() + dataset = await DatasetFactory.create(workspace=workspace) + annotator = await AnnotatorFactory.create(workspaces=[workspace]) + response = await async_client.get( + f"/api/v1/datasets/{dataset.id}/schema-versions", + headers={"X-Extralit-Api-Key": annotator.api_key}, ) - assert response.status_code == 201, response.json() + assert response.status_code == 200 ``` - [ ] **Step 2: Run the tests to verify they fail** ```bash -cd extralit-server && uv run pytest tests/unit/api/handlers/v1/datasets/questions/test_column_binding.py -v +cd extralit-server && uv run pytest tests/unit/api/handlers/v1/datasets/test_schema_versions.py -v ``` -Expected: the `columns` key is rejected as an extra field by the settings models. +Expected: all 404 β€” the routes do not exist. -- [ ] **Step 3: Add `columns` to the bindable settings variants** +- [ ] **Step 3: Write the pydantic contracts** -In `api/schemas/v1/questions.py`, add `columns: list[str] | None = None` to `TextQuestionSettings`, `TextQuestionSettingsCreate`, `TextQuestionSettingsUpdate`, `TableQuestionSettings`, `TableQuestionSettingsCreate`, and `TableQuestionSettingsUpdate`. Read the existing classes to place it consistently. Do **not** add it to `span` β€” v2 deferred span (`validators/v2/questions.py:8`) and nothing binds it. +Create `api/schemas/v1/schema_versions.py`. Take the field list from `$V2REF/extralit-server/src/extralit_server/api/schemas/v2/schemas.py:23-58`, minus `columns_cache` and `review_widgets`, and rename `schema_id` β†’ `dataset_id`. Follow the v1 read-model convention: bare noun with `model_config = ConfigDict(from_attributes=True)`. -- [ ] **Step 4: Port the binding validator** +```python +from typing import Any +from uuid import UUID +from datetime import datetime -Add to `validators/questions.py`, following the file's existing one-class-per-operation classmethod convention. Port the rules from `validators/v2/questions.py:23-47`, replacing `columns_cache` with the dataset's column fields: +from pydantic import BaseModel, ConfigDict, Field -```python -class QuestionColumnBindingValidator: - """Validate a question's `settings["columns"]` against the dataset's declared columns. - Column fields are materialized from the dataset's Pandera schema version at publish - time (contexts/schema_versions.derive_column_fields), so `dataset.fields` is the - authoritative manifest. Requires `dataset.fields` to be eagerly loaded β€” every - question handler already preloads it. - """ +class SchemaVersionCreate(BaseModel): + """A new schema version. `body` is a Pandera `DataFrameSchema.to_json()` payload.""" - @classmethod - def validate(cls, settings: dict, dataset: Dataset) -> None: - columns = settings.get("columns") - if columns is None: - return + body: str + # Per-column widget overlay; Pandera's to_json drops Column.metadata, so this rides + # alongside and lands in each derived Field's settings["review"]. + review_widgets: dict[str, dict[str, Any]] = Field(default_factory=dict) - if not columns: - raise UnprocessableEntityError("question column binding cannot be empty") - declared = {field.name for field in dataset.fields if field.settings.get("type") == FieldType.column} - unknown = [column for column in columns if column not in declared] - if unknown: - raise UnprocessableEntityError( - f"question binds to columns not declared by the dataset schema: {', '.join(sorted(unknown))}" - ) +class SchemaVersionRead(BaseModel): + id: UUID + dataset_id: UUID + version: int + object_key: str + object_version_id: str | None + etag: str + checksum: str + parent_version_id: UUID | None + created_by: UUID | None + inserted_at: datetime + updated_at: datetime - if settings.get("type") != QuestionType.table and len(columns) != 1: - raise UnprocessableEntityError( - f"a {settings.get('type')} question must bind to exactly one column, got {len(columns)}" - ) + model_config = ConfigDict(from_attributes=True) ``` -Call it from `QuestionCreateValidator.validate` and `QuestionUpdateValidator.validate` in the same file. Confirm `contexts/questions.py:16` `create_question` passes a dataset with `fields` loaded; the handler at `api/handlers/v1/datasets/questions.py:33` may need `selectinload(Dataset.fields)` added to its dataset fetch. +- [ ] **Step 4: Write the handler** -- [ ] **Step 5: Run the tests to verify they pass** +Create `api/handlers/v1/datasets/schema_versions.py`. Reuse `DatasetPolicy.publish` / `DatasetPolicy.get` β€” do **not** create a new policy class. Take the bucket resolution and `s3_client` dependency from `$V2REF/extralit-server/src/extralit_server/api/v2/schemas.py:103-133`; take the router shape (bare `APIRouter()`, literal paths, no prefix) from `api/handlers/v1/datasets/questions.py`. -```bash -cd extralit-server && uv run pytest tests/unit/api/handlers/v1/datasets/questions/test_column_binding.py -v +```python +from typing import Annotated +from uuid import UUID + +from fastapi import APIRouter, Depends, Security, status +from sqlalchemy.ext.asyncio import AsyncSession + +from extralit_server.api.policies.v1 import DatasetPolicy, authorize +from extralit_server.api.schemas.v1.schema_versions import SchemaVersionCreate, SchemaVersionRead +from extralit_server.contexts import files as files_ctx +from extralit_server.contexts import schema_versions +from extralit_server.database import get_async_db +from extralit_server.errors.future import NotFoundError +from extralit_server.models.database import Dataset, User +from extralit_server.search_engine import SearchEngine, get_search_engine +from extralit_server.security import auth + +router = APIRouter() + + +@router.post( + "/datasets/{dataset_id}/schema-versions", + status_code=status.HTTP_201_CREATED, + response_model=SchemaVersionRead, +) +async def publish_schema_version( + *, + dataset_id: UUID, + version_create: SchemaVersionCreate, + db: Annotated[AsyncSession, Depends(get_async_db)], + search_engine: Annotated[SearchEngine, Depends(get_search_engine)], + s3_client=Depends(files_ctx.get_s3_client), + current_user: Annotated[User, Security(auth.get_current_user)], +): + dataset = await Dataset.get_or_raise(db, dataset_id, options=[selectinload(Dataset.workspace)]) + await authorize(current_user, DatasetPolicy.publish(dataset)) + + return await schema_versions.publish_version( + db, + search_engine, + s3_client, + dataset, + body=version_create.body, + # One bucket per workspace, named exactly Workspace.name β€” contexts/files.py:381. + bucket=dataset.workspace.name, + review_widgets=version_create.review_widgets, + created_by=current_user.id, + ) + + +@router.get("/datasets/{dataset_id}/schema-versions", response_model=list[SchemaVersionRead]) +async def list_schema_versions( + *, + dataset_id: UUID, + db: Annotated[AsyncSession, Depends(get_async_db)], + current_user: Annotated[User, Security(auth.get_current_user)], +): + dataset = await Dataset.get_or_raise(db, dataset_id) + await authorize(current_user, DatasetPolicy.get(dataset)) + + return await schema_versions.list_versions(db, dataset) + + +@router.get("/datasets/{dataset_id}/schema-versions/{version}", response_model=SchemaVersionRead) +async def get_schema_version( + *, + dataset_id: UUID, + version: int, + db: Annotated[AsyncSession, Depends(get_async_db)], + current_user: Annotated[User, Security(auth.get_current_user)], +): + dataset = await Dataset.get_or_raise(db, dataset_id) + await authorize(current_user, DatasetPolicy.get(dataset)) + + schema_version = await schema_versions.get_version_by_number(db, dataset.id, version) + if schema_version is None: + raise NotFoundError(f"SchemaVersion {version} not found for dataset {dataset_id}") + + return schema_version ``` -Expected: 6 passed. +Import `selectinload` from `sqlalchemy.orm`. Check the actual name and import path of the not-found error class used elsewhere in `api/handlers/v1/` and match it. -- [ ] **Step 6: Verify existing question tests still pass** +- [ ] **Step 5: Register the router** + +In `api/handlers/v1/datasets/__init__.py`, import the new router and add `router.include_router(schema_versions_router)` alongside the existing four includes (`__init__.py:8-13`). + +- [ ] **Step 6: Run the tests to verify they pass** ```bash -cd extralit-server && uv run pytest tests/unit/api/handlers/v1 -k question -q && uv run ruff check +cd extralit-server && uv run pytest tests/unit/api/handlers/v1/datasets/test_schema_versions.py -v ``` -Expected: all pass. +Expected: 11 passed. - [ ] **Step 7: Commit** ```bash -git add extralit-server/src/extralit_server/api/schemas/v1/questions.py \ - extralit-server/src/extralit_server/validators/questions.py \ - extralit-server/src/extralit_server/api/handlers/v1/datasets/questions.py \ - extralit-server/tests/unit/api/handlers/v1/datasets/questions/test_column_binding.py -git commit -m "feat(server): bind v1 questions to schema columns via settings['columns'] +git add extralit-server/src/extralit_server/api/schemas/v1/schema_versions.py \ + extralit-server/src/extralit_server/api/handlers/v1/datasets/schema_versions.py \ + extralit-server/src/extralit_server/api/handlers/v1/datasets/__init__.py \ + extralit-server/tests/unit/api/handlers/v1/datasets/test_schema_versions.py +git commit -m "feat(server): schema-version endpoints on /api/v1 -Replaces V2Question.columns and validators/v2/questions.QuestionBindingValidator, -retargeted from SchemaVersion.columns_cache to the dataset's column fields." +Replaces POST/GET /api/v2/schemas/{id}/versions. GET /schemas/{id}/columns is +dropped: the derived columns are readable from GET /datasets/{id}/fields." ``` --- -### Task 7: Retarget the workspace projection onto v1 tables - -The `/extractions` grid is the live product surface and has no v1 counterpart. This is a move-and-retarget, not a rewrite: the DuckDB denormalization SQL is the value and must survive byte-for-byte apart from column names. +### Task 8: Carry `reference` through record create/upsert **Files:** -- Create: `extralit-server/src/extralit_server/contexts/projection.py` -- Create: `extralit-server/src/extralit_server/api/schemas/v1/projection.py` -- Create: `extralit-server/src/extralit_server/api/handlers/v1/projection.py` -- Modify: `extralit-server/src/extralit_server/api/routes.py` -- Test: `extralit-server/tests/unit/contexts/test_projection.py` (create), `extralit-server/tests/unit/api/handlers/v1/test_projection.py` (create) -- Reference: `extralit-server/src/extralit_server/contexts/v2/projection.py`, `extralit-server/tests/integration/contexts/v2/test_workspace_projection.py` (15 tests β€” port all of them) +- Modify: `extralit-server/src/extralit_server/api/schemas/v1/records.py` +- Modify: `extralit-server/src/extralit_server/contexts/records_bulk.py` +- Modify: `extralit-server/src/extralit_server/contexts/records.py` +- Test: `extralit-server/tests/unit/api/handlers/v1/datasets/test_records_reference.py` (create) +- Reference (snapshot): `$V2REF/extralit-server/src/extralit_server/contexts/v2/records.py:28-120` **Interfaces:** -- Consumes: `Record.reference` (Task 1), `Question.settings["columns"]` (Task 6), `Dataset.current_schema_version_id` (Task 1). -- Produces: `build_workspace_view(db, *, workspace_id: UUID, offset: int, limit: int) -> WorkspaceProjection`; `GET /me/datasets/projection?workspace_id=&offset=&limit=` β†’ `WorkspaceProjection`. `WorkspaceProjection`, `WorkspaceProjectionColumn`, `WorkspaceProjectionRow`, `WorkspaceProjectionCell` move verbatim from `api/schemas/v2/projection.py` (the `ProjectionCell`/`ProjectionRecord`/`ProjectionView` trio does **not** move β€” `build_reference_view` is deleted, see Task 12). - -- [ ] **Step 1: Port the schema models** - -Create `api/schemas/v1/projection.py` containing `WorkspaceProjectionColumn`, `WorkspaceProjectionCell`, `WorkspaceProjectionRow`, and `WorkspaceProjection`, copied verbatim from `api/schemas/v2/projection.py:30-56`. Rename `schema_id` β†’ `dataset_id` and `schema_name` β†’ `dataset_name` on `WorkspaceProjectionColumn`; the flat column `name` format stays `"{dataset_name}.{question_name}[.{sub_column}]"`. +- Consumes: `Record.reference` (Task 4). +- Produces: `reference: str | None` on `Record`, `RecordCreate`, `RecordUpdate`, and `RecordUpsert` (inherited); `reference` persisted by `CreateRecordsBulk.create_records_bulk` and patched by `UpsertRecordsBulk.upsert_records_bulk` under `is_set("reference")` semantics. -- [ ] **Step 2: Write the failing context tests** +- [ ] **Step 1: Write the failing tests** -Create `extralit-server/tests/unit/contexts/test_projection.py` by porting every test from `tests/integration/contexts/v2/test_workspace_projection.py`, substituting factories: `SchemaFactory` β†’ `DatasetFactory`, `V2RecordFactory` β†’ `RecordFactory` (with `reference=`), `V2QuestionFactory` β†’ `QuestionFactory` (with `settings={"type": ..., "columns": [...]}`), `V2SuggestionFactory` β†’ `SuggestionFactory`, `V2ResponseFactory` β†’ `ResponseFactory`. Add these two tests, which the v2 version could not express: +Create `extralit-server/tests/unit/api/handlers/v1/datasets/test_records_reference.py`: ```python - async def test_only_schema_backed_datasets_appear_in_the_projection(self, db): - """A plain annotation dataset in the same workspace must not leak into the grid.""" - workspace = await WorkspaceFactory.create() - plain = await DatasetFactory.create(workspace=workspace) - await QuestionFactory.create(dataset=plain, name="sentiment") - await RecordFactory.create(dataset=plain, reference="ref-1") - - projection = await projection_ctx.build_workspace_view( - db, workspace_id=workspace.id, offset=0, limit=10 - ) - assert projection.columns == [] - assert projection.rows == [] - assert projection.total_references == 0 +import pytest - async def test_datasets_are_ordered_by_name(self, db): - workspace = await WorkspaceFactory.create() - for name in ("zeta", "alpha"): - dataset = await schema_backed_dataset(workspace, name=name) - await QuestionFactory.create(dataset=dataset, name="q", settings={"type": "text", "columns": ["c"]}) - projection = await projection_ctx.build_workspace_view( - db, workspace_id=workspace.id, offset=0, limit=10 - ) - assert [c.dataset_name for c in projection.columns] == ["alpha", "zeta"] -``` +from tests.factories import DatasetFactory, FieldFactory, RecordFactory, TextFieldFactory -Write a `schema_backed_dataset(workspace, name)` helper in the test module that creates a `Dataset`, a `SchemaVersion`, sets `current_schema_version_id`, and creates one `column` `Field` β€” the discriminator that makes a dataset appear in the projection. -- [ ] **Step 3: Run the tests to verify they fail** +@pytest.mark.asyncio +class TestRecordReference: + async def _ready_dataset(self): + dataset = await DatasetFactory.create(status="ready") + await TextFieldFactory.create(dataset=dataset, name="text") + return dataset -```bash -cd extralit-server && uv run pytest tests/unit/contexts/test_projection.py -v -``` + async def test_bulk_create_persists_reference(self, async_client, owner_auth_header, mock_search_engine, db): + dataset = await self._ready_dataset() + response = await async_client.post( + f"/api/v1/datasets/{dataset.id}/records/bulk", + headers=owner_auth_header, + json={"items": [{"fields": {"text": "a"}, "reference": "10.1000/j.foo.2020.01"}]}, + ) + assert response.status_code == 201, response.json() + assert response.json()["items"][0]["reference"] == "10.1000/j.foo.2020.01" -Expected: `ModuleNotFoundError: extralit_server.contexts.projection`. + async def test_reference_is_optional(self, async_client, owner_auth_header, mock_search_engine): + dataset = await self._ready_dataset() + response = await async_client.post( + f"/api/v1/datasets/{dataset.id}/records/bulk", + headers=owner_auth_header, + json={"items": [{"fields": {"text": "a"}}]}, + ) + assert response.status_code == 201 + assert response.json()["items"][0]["reference"] is None -- [ ] **Step 4: Move and retarget the context** + async def test_bulk_upsert_updates_reference(self, async_client, owner_auth_header, mock_search_engine, db): + dataset = await self._ready_dataset() + record = await RecordFactory.create(dataset=dataset, external_id="x1", reference="old") + response = await async_client.put( + f"/api/v1/datasets/{dataset.id}/records/bulk", + headers=owner_auth_header, + json={"items": [{"external_id": "x1", "reference": "new"}]}, + ) + assert response.status_code == 200, response.json() + await db.refresh(record) + assert record.reference == "new" -Copy `contexts/v2/projection.py` to `contexts/projection.py`, then make exactly these changes. **Do not touch `_INPUT_TABLES_DDL`, `_INSERTS`, `_DENORMALIZE_SQL`, or `_run_denormalization`** β€” the DuckDB staging tables are named independently of the Postgres tables, so the ~130-line denormalization SQL is unaffected. + async def test_bulk_upsert_leaves_reference_alone_when_omitted( + self, async_client, owner_auth_header, mock_search_engine, db + ): + dataset = await self._ready_dataset() + record = await RecordFactory.create(dataset=dataset, external_id="x1", reference="keep") + await async_client.put( + f"/api/v1/datasets/{dataset.id}/records/bulk", + headers=owner_auth_header, + json={"items": [{"external_id": "x1", "metadata": {"a": 1}}]}, + ) + await db.refresh(record) + assert record.reference == "keep" -1. Delete `build_reference_view` and `_build_columns`'s `ProjectionCell`/`ProjectionRecord`/`ProjectionView` imports; delete the `contexts.v2.records` import. -2. Imports become `from extralit_server.models.database import Dataset, Question, Record, Response, Suggestion` and `from extralit_server.api.schemas.v1.projection import (...)`. -3. In `build_workspace_view`, replace the schema query with a dataset query filtered to schema-backed datasets β€” this is the new discriminator and the reason the projection cannot simply select every dataset in the workspace: + async def test_list_records_filters_by_reference(self, async_client, owner_auth_header): + dataset = await self._ready_dataset() + await RecordFactory.create(dataset=dataset, reference="doi-a") + await RecordFactory.create(dataset=dataset, reference="doi-b") + response = await async_client.get( + f"/api/v1/datasets/{dataset.id}/records?reference=doi-a", headers=owner_auth_header + ) + assert response.status_code == 200 + assert [r["reference"] for r in response.json()["items"]] == ["doi-a"] -```python - datasets = ( - ( - await db.execute( - select(Dataset) - .where( - Dataset.workspace_id == workspace_id, - # Only schema-backed datasets are extraction projects; a plain - # annotation dataset in the same workspace has no column manifest - # and must not contribute columns or rows to the grid. - Dataset.current_schema_version_id.is_not(None), - ) - .order_by(Dataset.name) - ) + async def test_a_reference_may_contain_slashes(self, async_client, owner_auth_header, mock_search_engine): + dataset = await self._ready_dataset() + await async_client.post( + f"/api/v1/datasets/{dataset.id}/records/bulk", + headers=owner_auth_header, + json={"items": [{"fields": {"text": "a"}, "reference": "10.1000/j.foo.2020.01"}]}, ) - .scalars() - .all() - ) + response = await async_client.get( + f"/api/v1/datasets/{dataset.id}/records", + headers=owner_auth_header, + params={"reference": "10.1000/j.foo.2020.01"}, + ) + assert len(response.json()["items"]) == 1 ``` -4. Substitute throughout: `V2Question` β†’ `Question`, `V2Record` β†’ `Record`, `V2Response` β†’ `Response`, `V2Suggestion` β†’ `Suggestion`, `Schema` β†’ `Dataset`, `.schema_id` β†’ `.dataset_id`, `schema_names` β†’ `dataset_names`. -5. `question.columns` becomes `question.settings.get("columns") or []` β€” there are two sites: `_build_columns`'s table branch and the `question_columns` input tuple. -6. `question.type` still works unchanged: v1 `Question.type` (`models/database.py:322`) is a property reading `settings["type"]`. -7. `Response.status == ResponseStatus.submitted` is unchanged β€” v1's enum has the same member. - -- [ ] **Step 5: Run the context tests** +- [ ] **Step 2: Run the tests to verify they fail** ```bash -cd extralit-server && uv run pytest tests/unit/contexts/test_projection.py -v +cd extralit-server && uv run pytest tests/unit/api/handlers/v1/datasets/test_records_reference.py -v ``` -Expected: all 17 pass. - -- [ ] **Step 6: Write the failing handler tests** - -Create `extralit-server/tests/unit/api/handlers/v1/test_projection.py` by porting the workspace-projection half of `tests/integration/api/v2/test_projection.py` (drop the `/projection/references/{ref}` tests β€” that endpoint is deleted). Keep the pagination test, the `limit=101` rejection, the missing-`workspace_id` 422, and the annotator authorization test. +Expected: failures on the unknown `reference` key and on the unsupported `reference` query param. -- [ ] **Step 7: Write the handler and register it** +- [ ] **Step 3: Add `reference` to the record schemas** -Create `api/handlers/v1/projection.py`. Authorize with `DatasetPolicy.list(workspace_id)` β€” the same predicate `GET /me/datasets` uses (`api/handlers/v1/datasets/datasets.py:74`) β€” rather than a new policy class. The `/me/` prefix follows the v1 convention for user-scoped reads. +In `api/schemas/v1/records.py`, add `reference: str | None = None` to `Record` (`:64`), `RecordCreate` (`:106`), and `RecordUpdate` (`:169`). `RecordUpsert` (`:194`) inherits from `RecordCreate` so it gets it for free. Reuse the existing constraint from `$V2REF/extralit-server/src/extralit_server/api/schemas/v2/records.py:15` β€” `Reference = Annotated[constr(min_length=1, max_length=500), ...]` β€” and use it rather than a bare `str`. -```python -from typing import Annotated -from uuid import UUID +- [ ] **Step 4: Persist it in the bulk contexts** -from fastapi import APIRouter, Depends, Query, Security -from sqlalchemy.ext.asyncio import AsyncSession +In `contexts/records_bulk.py:38` `create_records_bulk`, add `reference=record_create.reference` to the `Record(...)` construction alongside `external_id`. -from extralit_server.api.policies.v1 import DatasetPolicy, authorize -from extralit_server.api.schemas.v1.projection import WorkspaceProjection -from extralit_server.contexts import projection -from extralit_server.database import get_async_db -from extralit_server.models.database import User -from extralit_server.security import auth +In `contexts/records_bulk.py:145` `upsert_records_bulk`, add a `reference` branch to the partial-update block that already handles `is_set("metadata")` / `is_set("fields")`: -router = APIRouter(tags=["projection"]) +```python + if record_upsert.is_set("reference"): + record.reference = record_upsert.reference +``` -LIST_PROJECTION_LIMIT_DEFAULT = 50 -LIST_PROJECTION_LIMIT_LE = 100 +- [ ] **Step 5: Add the `reference` list filter** +In `contexts/records.py:85` `_build_list_records_query`, add a `reference: str | None = None` parameter and `if reference is not None: query = query.filter(Record.reference == reference)`. Thread it through `list_dataset_records` (`:24`) and add the query param to `list_dataset_records` in `api/handlers/v1/datasets/records.py:269`, following the existing `metadata`/`sort_by` param style. -@router.get("/me/datasets/projection", response_model=WorkspaceProjection) -async def get_workspace_projection( - *, - workspace_id: Annotated[UUID, Query(description="The workspace to project")], - offset: int = 0, - limit: Annotated[int, Query(ge=1, le=LIST_PROJECTION_LIMIT_LE)] = LIST_PROJECTION_LIMIT_DEFAULT, - db: Annotated[AsyncSession, Depends(get_async_db)], - current_user: Annotated[User, Security(auth.get_current_user)], -): - await authorize(current_user, DatasetPolicy.list(workspace_id)) +- [ ] **Step 6: Run the tests to verify they pass** - # offset/limit count references, not fan-out rows β€” a reference with a stacked table - # question spans several rows and must never be split across a page boundary. - return await projection.build_workspace_view(db, workspace_id=workspace_id, offset=offset, limit=limit) +```bash +cd extralit-server && uv run pytest tests/unit/api/handlers/v1/datasets/test_records_reference.py -v ``` -Register it in `api/routes.py:90` by adding `projection_v1.router` to the router list, with the matching import alongside the other `from extralit_server.api.handlers.v1 import ... as ..._v1` lines. - -**Route-ordering check:** `/me/datasets/projection` must be declared before any `/me/datasets/{dataset_id}`-style route, or FastAPI will match `projection` as a `dataset_id` and return a UUID parse error. Confirm by running the 404 test in Step 6 β€” if it returns 422 instead of a projection, move the `include_router` call for `projection_v1` above `datasets_v1` in the list. +Expected: 6 passed. -- [ ] **Step 8: Run the handler tests** +- [ ] **Step 7: Verify no existing record test regressed** ```bash -cd extralit-server && uv run pytest tests/unit/api/handlers/v1/test_projection.py -v +cd extralit-server && uv run pytest tests/unit/api/handlers/v1/datasets tests/unit/contexts -q ``` Expected: all pass. -- [ ] **Step 9: Commit** +- [ ] **Step 8: Commit** ```bash -git add extralit-server/src/extralit_server/contexts/projection.py \ - extralit-server/src/extralit_server/api/schemas/v1/projection.py \ - extralit-server/src/extralit_server/api/handlers/v1/projection.py \ - extralit-server/src/extralit_server/api/routes.py \ - extralit-server/tests/unit/contexts/test_projection.py \ - extralit-server/tests/unit/api/handlers/v1/test_projection.py -git commit -m "feat(server): move the workspace projection onto v1 tables +git add extralit-server/src/extralit_server/api/schemas/v1/records.py \ + extralit-server/src/extralit_server/contexts/records_bulk.py \ + extralit-server/src/extralit_server/contexts/records.py \ + extralit-server/src/extralit_server/api/handlers/v1/datasets/records.py \ + extralit-server/tests/unit/api/handlers/v1/datasets/test_records_reference.py +git commit -m "feat(server): carry record reference through v1 bulk create/upsert and list -DuckDB denormalization SQL is unchanged. Adds the schema-backed discriminator -(Dataset.current_schema_version_id IS NOT NULL) so plain annotation datasets -in the same workspace do not leak into the extraction grid." +Replaces V2Record.reference. Drops the schema_version_id pin (its CASCADE +silently deleted records) and status=discarded (record status is derived from +response distribution; discard is a response status)." ``` --- -### Task 8: Regression tests for the two bugs the v2 annotation path hid - -No new production code β€” v1's `contexts/datasets.py` already does the right thing. These tests pin the behavior that was missing, so the fold cannot silently regress it. +### Task 9: Question column bindings on v1 questions **Files:** -- Test: `extralit-server/tests/unit/contexts/test_extraction_response_side_effects.py` (create) +- Modify: `extralit-server/src/extralit_server/api/schemas/v1/questions.py` +- Modify: `extralit-server/src/extralit_server/validators/questions.py` +- Test: `extralit-server/tests/unit/api/handlers/v1/datasets/questions/test_column_binding.py` (create) +- Reference (snapshot): `$V2REF/extralit-server/src/extralit_server/validators/v2/questions.py`, `$V2REF/extralit-server/tests/unit/validators/v2/test_question_binding.py` **Interfaces:** -- Consumes: everything from Tasks 1–7. -- Produces: nothing. Tests only. - -- [ ] **Step 1: Write the tests** +- Consumes: `FieldType.column` (Task 5), derived `Field` rows (Task 6). +- Produces: `columns: list[str] | None` on every `QuestionSettings*` variant that can bind (`TextQuestionSettings*`, `TableQuestionSettings*`); `QuestionColumnBindingValidator.validate(settings: dict, dataset: Dataset) -> None` in `validators/questions.py`, called from `QuestionCreateValidator.validate` and `QuestionUpdateValidator.validate`. It takes the raw settings dict rather than the pydantic model because the two callers hold different types (`QuestionCreate.settings` vs a partial `QuestionUpdate.settings`) and both can `model_dump()` into it. -```python -"""Side effects the v2 annotation path deliberately omitted. +- [ ] **Step 1: Write the failing tests** -contexts/v2/annotation.upsert_response never touched record status and was forbidden -by tests/unit/test_annotation_no_index_import.py from reaching any index. Both are -required behavior; v1's contexts/datasets.upsert_response supplies them. These tests -exist so folding onto v1 cannot silently lose them again. -""" +Create `extralit-server/tests/unit/api/handlers/v1/datasets/questions/test_column_binding.py`: +```python import pytest -from extralit_server.api.schemas.v1.responses import ResponseUpsert -from extralit_server.contexts import datasets as datasets_ctx -from extralit_server.enums import RecordStatus, ResponseStatus -from tests.factories import ( - AnnotatorFactory, - DatasetFactory, - QuestionFactory, - RecordFactory, - WorkspaceFactory, -) +from tests.factories import DatasetFactory, FieldFactory @pytest.mark.asyncio -class TestExtractionResponseSideEffects: - async def _setup(self, db): - workspace = await WorkspaceFactory.create() - dataset = await DatasetFactory.create( - workspace=workspace, status="ready", distribution={"strategy": "overlap", "min_submitted": 1} - ) - question = await QuestionFactory.create( - dataset=dataset, name="population", settings={"type": "text", "use_markdown": False} - ) - record = await RecordFactory.create(dataset=dataset, reference="10.1000/j.foo.2020.01") - user = await AnnotatorFactory.create(workspaces=[workspace]) - return dataset, question, record, user - - async def test_submitting_a_response_completes_the_record(self, db, mock_search_engine): - dataset, question, record, user = await self._setup(db) - assert record.status == RecordStatus.pending - - await datasets_ctx.upsert_response( - db, - mock_search_engine, - record, - user, - ResponseUpsert( - record_id=record.id, - status=ResponseStatus.submitted, - values={"population": {"value": "Kenya"}}, - ), - ) - - await db.refresh(record) - assert record.status == RecordStatus.completed - - async def test_a_draft_response_leaves_the_record_pending(self, db, mock_search_engine): - dataset, question, record, user = await self._setup(db) - - await datasets_ctx.upsert_response( - db, - mock_search_engine, - record, - user, - ResponseUpsert( - record_id=record.id, - status=ResponseStatus.draft, - values={"population": {"value": "Kenya"}}, - ), - ) - - await db.refresh(record) - assert record.status == RecordStatus.pending - - async def test_submitting_a_response_reaches_the_search_index(self, db, mock_search_engine): - dataset, question, record, user = await self._setup(db) - - await datasets_ctx.upsert_response( - db, - mock_search_engine, - record, - user, - ResponseUpsert( - record_id=record.id, - status=ResponseStatus.submitted, - values={"population": {"value": "Kenya"}}, - ), - ) - - mock_search_engine.update_record_response.assert_awaited() +class TestQuestionColumnBinding: + async def _dataset_with_columns(self, *names): + dataset = await DatasetFactory.create() + for name in names: + await FieldFactory.create( + dataset=dataset, name=name, settings={"type": "column", "dtype": "string", "nullable": True} + ) + return dataset - async def test_upserting_a_suggestion_reaches_the_search_index(self, db, mock_search_engine): - from extralit_server.api.schemas.v1.suggestions import SuggestionCreate + async def test_question_binds_to_a_declared_column(self, async_client, owner_auth_header): + dataset = await self._dataset_with_columns("population") + response = await async_client.post( + f"/api/v1/datasets/{dataset.id}/questions", + headers=owner_auth_header, + json={ + "name": "population_review", + "title": "Population", + "settings": {"type": "text", "use_markdown": False, "columns": ["population"]}, + }, + ) + assert response.status_code == 201, response.json() + assert response.json()["settings"]["columns"] == ["population"] - dataset, question, record, user = await self._setup(db) + async def test_binding_to_an_undeclared_column_is_rejected(self, async_client, owner_auth_header): + dataset = await self._dataset_with_columns("population") + response = await async_client.post( + f"/api/v1/datasets/{dataset.id}/questions", + headers=owner_auth_header, + json={ + "name": "q", + "title": "Q", + "settings": {"type": "text", "use_markdown": False, "columns": ["nope"]}, + }, + ) + assert response.status_code == 422 + assert "nope" in response.text - await datasets_ctx.upsert_suggestion( - db, - mock_search_engine, - record, - question, - SuggestionCreate(question_id=question.id, value="Kenya", agent="gpt-x", score=0.9), + async def test_a_scalar_question_binds_to_exactly_one_column(self, async_client, owner_auth_header): + dataset = await self._dataset_with_columns("a", "b") + response = await async_client.post( + f"/api/v1/datasets/{dataset.id}/questions", + headers=owner_auth_header, + json={ + "name": "q", + "title": "Q", + "settings": {"type": "text", "use_markdown": False, "columns": ["a", "b"]}, + }, ) + assert response.status_code == 422 - mock_search_engine.update_record_suggestion.assert_awaited() -``` + async def test_a_table_question_binds_to_many_columns(self, async_client, owner_auth_header): + dataset = await self._dataset_with_columns("a", "b") + response = await async_client.post( + f"/api/v1/datasets/{dataset.id}/questions", + headers=owner_auth_header, + json={ + "name": "t", + "title": "T", + "settings": {"type": "table", "columns": ["a", "b"]}, + }, + ) + assert response.status_code == 201, response.json() + assert response.json()["settings"]["columns"] == ["a", "b"] -- [ ] **Step 2: Run them** + async def test_an_empty_binding_is_rejected(self, async_client, owner_auth_header): + dataset = await self._dataset_with_columns("a") + response = await async_client.post( + f"/api/v1/datasets/{dataset.id}/questions", + headers=owner_auth_header, + json={"name": "q", "title": "Q", "settings": {"type": "table", "columns": []}}, + ) + assert response.status_code == 422 -```bash -cd extralit-server && uv run pytest tests/unit/contexts/test_extraction_response_side_effects.py -v + async def test_questions_without_a_binding_are_still_valid(self, async_client, owner_auth_header): + # A plain annotation dataset has no column fields and no bindings β€” unchanged v1 behavior. + dataset = await DatasetFactory.create() + response = await async_client.post( + f"/api/v1/datasets/{dataset.id}/questions", + headers=owner_auth_header, + json={"name": "q", "title": "Q", "settings": {"type": "text", "use_markdown": False}}, + ) + assert response.status_code == 201, response.json() ``` -Expected: 5 passed on the first run β€” v1 already behaves correctly. If `test_submitting_a_response_completes_the_record` fails, read `contexts/distribution.py:61` and check the `distribution` dict shape the factory produced matches what `distribution_strategy` expects; fix the fixture, not the production code. - -- [ ] **Step 3: Commit** +- [ ] **Step 2: Run the tests to verify they fail** ```bash -git add extralit-server/tests/unit/contexts/test_extraction_response_side_effects.py -git commit -m "test(server): pin the record-status and index side effects v2 omitted" +cd extralit-server && uv run pytest tests/unit/api/handlers/v1/datasets/questions/test_column_binding.py -v ``` ---- +Expected: the `columns` key is rejected as an extra field by the settings models. -### Task 9: Delete the v2 API surface +- [ ] **Step 3: Add `columns` to the bindable settings variants** -From here the tasks are removals. Order matters: API first (nothing depends on it), then contexts, then models, then migrations. +In `api/schemas/v1/questions.py`, add `columns: list[str] | None = None` to `TextQuestionSettings`, `TextQuestionSettingsCreate`, `TextQuestionSettingsUpdate`, `TableQuestionSettings`, `TableQuestionSettingsCreate`, and `TableQuestionSettingsUpdate`. Read the existing classes to place it consistently. Do **not** add it to `span` β€” v2 deferred span (`$V2REF/extralit-server/src/extralit_server/validators/v2/questions.py:8`) and nothing binds it. -**Files:** -- Delete: `extralit-server/src/extralit_server/api/v2/` (entire directory: `__init__.py`, `annotation.py`, `projection.py`, `questions.py`, `records.py`, `schemas.py`) -- Delete: `extralit-server/src/extralit_server/api/schemas/v2/` (entire directory) -- Delete: `extralit-server/src/extralit_server/api/policies/v1/schema_policy.py`, `extralit-server/src/extralit_server/api/policies/v1/v2_annotation_policy.py` -- Delete: `extralit-server/tests/integration/api/v2/`, `extralit-server/tests/integration/api/schemas/v2/` -- Modify: `extralit-server/src/extralit_server/_app.py`, `extralit-server/src/extralit_server/api/policies/v1/__init__.py`, `extralit-server/src/extralit_server/cli/openapi_dump.py` +- [ ] **Step 4: Port the binding validator** -**Interfaces:** -- Consumes: Tasks 4 and 7 must be complete β€” their endpoints are the replacements. -- Produces: `/api/v2` no longer exists. +Add to `validators/questions.py`, following the file's existing one-class-per-operation classmethod convention. Port the rules from `$V2REF/extralit-server/src/extralit_server/validators/v2/questions.py:23-47`, replacing `columns_cache` with the dataset's column fields: -- [ ] **Step 1: Write the failing test that pins the deletion** +```python +class QuestionColumnBindingValidator: + """Validate a question's `settings["columns"]` against the dataset's declared columns. -Add to `extralit-server/tests/unit/api/test_api_mounts.py` (create if absent): + Column fields are materialized from the dataset's Pandera schema version at publish + time (contexts/schema_versions.derive_column_fields), so `dataset.fields` is the + authoritative manifest. Requires `dataset.fields` to be eagerly loaded β€” every + question handler already preloads it. + """ -```python -import pytest + @classmethod + def validate(cls, settings: dict, dataset: Dataset) -> None: + columns = settings.get("columns") + if columns is None: + return -from extralit_server._app import create_server_app + if not columns: + raise UnprocessableEntityError("question column binding cannot be empty") + declared = {field.name for field in dataset.fields if field.settings.get("type") == FieldType.column} + unknown = [column for column in columns if column not in declared] + if unknown: + raise UnprocessableEntityError( + f"question binds to columns not declared by the dataset schema: {', '.join(sorted(unknown))}" + ) -class TestApiMounts: - def test_only_v1_is_mounted(self): - app = create_server_app() - mounts = {route.path for route in app.routes if hasattr(route, "app")} - assert "/api/v1" in mounts - assert "/api/v2" not in mounts + if settings.get("type") != QuestionType.table and len(columns) != 1: + raise UnprocessableEntityError( + f"a {settings.get('type')} question must bind to exactly one column, got {len(columns)}" + ) ``` -Confirm the app-factory function name in `_app.py` and use the real one. +Call it from `QuestionCreateValidator.validate` and `QuestionUpdateValidator.validate` in the same file. Confirm `contexts/questions.py:16` `create_question` passes a dataset with `fields` loaded; the handler at `api/handlers/v1/datasets/questions.py:33` may need `selectinload(Dataset.fields)` added to its dataset fetch. -- [ ] **Step 2: Run it to verify it fails** +- [ ] **Step 5: Run the tests to verify they pass** ```bash -cd extralit-server && uv run pytest tests/unit/api/test_api_mounts.py -v +cd extralit-server && uv run pytest tests/unit/api/handlers/v1/datasets/questions/test_column_binding.py -v ``` -Expected: FAIL β€” `/api/v2` is still mounted. +Expected: 6 passed. -- [ ] **Step 3: Delete the directories and tests** +- [ ] **Step 6: Verify existing question tests still pass** ```bash -cd extralit-server && rm -rf src/extralit_server/api/v2 src/extralit_server/api/schemas/v2 \ - src/extralit_server/api/policies/v1/schema_policy.py \ - src/extralit_server/api/policies/v1/v2_annotation_policy.py \ - tests/integration/api/v2 tests/integration/api/schemas/v2 +cd extralit-server && uv run pytest tests/unit/api/handlers/v1 -k question -q && uv run ruff check ``` -- [ ] **Step 4: Rewrite the v2 conftest so the one genuine v1 test in that tree survives** - -`tests/integration/conftest.py` mounts `api_v2` and will fail at collection now. But `tests/integration/test_rq_groups_workflow.py` is a real v1 test (it hits `/api/v1/jobs/...`) that depends on this conftest's `async_client` and `owner_auth_header`. Do not delete the conftest β€” reduce it to what that one test needs, retargeted onto `api_v1`: - -```python -"""Fixtures for the tests remaining in this tree. - -This file used to wire the isolated `/api/v2` suite. That suite is gone; what remains -is `test_rq_groups_workflow.py` (a v1 jobs test) and `index/` (the LanceDB engine, -kept for ENG-36 and fixture-free). New tests belong under `tests/unit/` β€” see the -plan's "The server test tree is named backwards" note. -""" - -from collections.abc import AsyncGenerator - -import pytest -import pytest_asyncio -from httpx import AsyncClient - -from extralit_server.constants import API_KEY_HEADER_NAME -from extralit_server.database import get_async_db -from extralit_server.models import User -from tests.database import TestSession -from tests.factories import OwnerFactory - - -@pytest_asyncio.fixture -async def owner() -> User: - return await OwnerFactory.create(first_name="Owner", username="owner", api_key="owner.apikey") - - -@pytest.fixture -def owner_auth_header(owner: User) -> dict[str, str]: - return {API_KEY_HEADER_NAME: owner.api_key} - +Expected: all pass. -@pytest_asyncio.fixture -async def async_client() -> AsyncGenerator[AsyncClient, None]: - from extralit_server import app - from extralit_server.api.routes import api_v1 +- [ ] **Step 7: Commit** - async def override_get_async_db(): - yield TestSession() +```bash +git add extralit-server/src/extralit_server/api/schemas/v1/questions.py \ + extralit-server/src/extralit_server/validators/questions.py \ + extralit-server/src/extralit_server/api/handlers/v1/datasets/questions.py \ + extralit-server/tests/unit/api/handlers/v1/datasets/questions/test_column_binding.py +git commit -m "feat(server): bind v1 questions to schema columns via settings['columns'] - api_v1.dependency_overrides[get_async_db] = override_get_async_db +Replaces V2Question.columns and validators/v2/questions.QuestionBindingValidator, +retargeted from SchemaVersion.columns_cache to the dataset's column fields." +``` - async with AsyncClient(app=app, base_url="http://testserver") as client: - yield client +--- - api_v1.dependency_overrides.clear() -``` +### Task 10: Retarget the workspace projection onto v1 tables -Note the override now lands on `api_v1` β€” previously it was registered on `api_v2`, so `test_rq_groups_workflow.py` was never actually getting the test session for its v1 route. Run that file before and after this change and compare: +The `/extractions` grid is the live product surface and has no v1 counterpart. This is a move-and-retarget, not a rewrite: the DuckDB denormalization SQL is the value and must survive byte-for-byte apart from column names. -```bash -cd extralit-server && uv run pytest tests/integration/test_rq_groups_workflow.py -v -``` +**Files:** +- Create: `extralit-server/src/extralit_server/contexts/projection.py` +- Create: `extralit-server/src/extralit_server/api/schemas/v1/projection.py` +- Create: `extralit-server/src/extralit_server/api/handlers/v1/projection.py` +- Modify: `extralit-server/src/extralit_server/api/routes.py` +- Test: `extralit-server/tests/unit/contexts/test_projection.py` (create), `extralit-server/tests/unit/api/handlers/v1/test_projection.py` (create) +- Reference (snapshot): `$V2REF/extralit-server/src/extralit_server/contexts/v2/projection.py`, `$V2REF/extralit-server/tests/integration/contexts/v2/test_workspace_projection.py` (15 tests β€” port all of them) -If it was passing only by accident and now fails on the real session, fix the test β€” do not revert the override. Also delete the `annotator` / `annotator_auth_header` fixtures if nothing in the remaining tree uses them: +**Interfaces:** +- Consumes: `Record.reference` (Task 4), `Question.settings["columns"]` (Task 9), `Dataset.current_schema_version_id` (Task 4). +- Produces: `build_workspace_view(db, *, workspace_id: UUID, offset: int, limit: int) -> WorkspaceProjection`; `GET /me/datasets/projection?workspace_id=&offset=&limit=` β†’ `WorkspaceProjection`. `WorkspaceProjection`, `WorkspaceProjectionColumn`, `WorkspaceProjectionRow`, `WorkspaceProjectionCell` move verbatim from the snapshot's `api/schemas/v2/projection.py` (the `ProjectionCell`/`ProjectionRecord`/`ProjectionView` trio does **not** move β€” `build_reference_view` went with `contexts/v2` in Task 2 and is not resurrected). -```bash -cd extralit-server && grep -rn "annotator_auth_header\|annotator\b" tests/integration --include=*.py | grep -v __pycache__ -``` +- [ ] **Step 1: Port the schema models** -- [ ] **Step 5: Unwire the mount and the exports** +Create `api/schemas/v1/projection.py` containing `WorkspaceProjectionColumn`, `WorkspaceProjectionCell`, `WorkspaceProjectionRow`, and `WorkspaceProjection`, copied verbatim from `$V2REF/extralit-server/src/extralit_server/api/schemas/v2/projection.py:30-56`. Rename `schema_id` β†’ `dataset_id` and `schema_name` β†’ `dataset_name` on `WorkspaceProjectionColumn`; the flat column `name` format stays `"{dataset_name}.{question_name}[.{sub_column}]"`. -In `_app.py`: delete `from extralit_server.api.v2 import api_v2` (`:27`) and `app.mount("/api/v2", api_v2)` (`:214`). +- [ ] **Step 2: Write the failing context tests** -In `api/policies/v1/__init__.py`: delete the `SchemaPolicy` export (`:11`) and the `V2QuestionPolicy, V2ResponsePolicy, V2SuggestionPolicy` export (`:14`). +Create `extralit-server/tests/unit/contexts/test_projection.py` by porting every test from `$V2REF/extralit-server/tests/integration/contexts/v2/test_workspace_projection.py`, substituting factories: `SchemaFactory` β†’ `DatasetFactory`, `V2RecordFactory` β†’ `RecordFactory` (with `reference=`), `V2QuestionFactory` β†’ `QuestionFactory` (with `settings={"type": ..., "columns": [...]}`), `V2SuggestionFactory` β†’ `SuggestionFactory`, `V2ResponseFactory` β†’ `ResponseFactory`. Add these two tests, which the v2 version could not express: -In `cli/openapi_dump.py`: repoint `from extralit_server.api.v2 import api_v2` / `api_v2.openapi()` (`:18-20`) to `from extralit_server.api.routes import api_v1` / `api_v1.openapi()`, and update the docstring at `:16`. +```python + async def test_only_schema_backed_datasets_appear_in_the_projection(self, db): + """A plain annotation dataset in the same workspace must not leak into the grid.""" + workspace = await WorkspaceFactory.create() + plain = await DatasetFactory.create(workspace=workspace) + await QuestionFactory.create(dataset=plain, name="sentiment") + await RecordFactory.create(dataset=plain, reference="ref-1") -- [ ] **Step 6: Run the test and the full suite** + projection = await projection_ctx.build_workspace_view( + db, workspace_id=workspace.id, offset=0, limit=10 + ) + assert projection.columns == [] + assert projection.rows == [] + assert projection.total_references == 0 -```bash -cd extralit-server && uv run pytest tests/unit/api/test_api_mounts.py -v && uv run ruff check + async def test_datasets_are_ordered_by_name(self, db): + workspace = await WorkspaceFactory.create() + for name in ("zeta", "alpha"): + dataset = await schema_backed_dataset(workspace, name=name) + await QuestionFactory.create(dataset=dataset, name="q", settings={"type": "text", "columns": ["c"]}) + projection = await projection_ctx.build_workspace_view( + db, workspace_id=workspace.id, offset=0, limit=10 + ) + assert [c.dataset_name for c in projection.columns] == ["alpha", "zeta"] ``` -Expected: the mount test passes; `ruff` reports unresolved imports only in files scheduled for deletion in Tasks 10–11 (`contexts/v2/*`, `validators/v2/*`, `cli/index/*`). Note which, and do not fix them here. +Write a `schema_backed_dataset(workspace, name)` helper in the test module that creates a `Dataset`, a `SchemaVersion`, sets `current_schema_version_id`, and creates one `column` `Field` β€” the discriminator that makes a dataset appear in the projection. -- [ ] **Step 7: Commit** +- [ ] **Step 3: Run the tests to verify they fail** ```bash -git add -A extralit-server/src/extralit_server/api extralit-server/src/extralit_server/_app.py \ - extralit-server/src/extralit_server/cli/openapi_dump.py extralit-server/tests -git commit -m "refactor(server)!: delete the /api/v2 surface - -Removes api/v2, api/schemas/v2, SchemaPolicy and the three V2*Policy classes -(they reproduced DatasetPolicy/QuestionPolicy/ResponsePolicy predicate for -predicate). openapi_dump now dumps v1." +cd extralit-server && uv run pytest tests/unit/contexts/test_projection.py -v ``` ---- - -### Task 10: Delete `contexts/v2`, `validators/v2`, `cli/index`, and the index-sync glue +Expected: `ModuleNotFoundError: extralit_server.contexts.projection`. -**Files:** -- Delete: `extralit-server/src/extralit_server/contexts/v2/` (entire directory) -- Delete: `extralit-server/src/extralit_server/validators/v2/` (entire directory) -- Delete: `extralit-server/src/extralit_server/cli/index/` (entire directory) -- Delete: `extralit-server/tests/integration/contexts/v2/`, `extralit-server/tests/unit/validators/v2/`, `extralit-server/tests/integration/cli/test_index_reindex.py`, `extralit-server/tests/unit/test_annotation_no_index_import.py` -- Modify: `extralit-server/src/extralit_server/cli/__init__.py` -- Keep untouched: `extralit-server/src/extralit_server/index/**` and `extralit-server/tests/{unit,integration}/index/**` +- [ ] **Step 4: Move and retarget the context** -**Interfaces:** -- Consumes: Tasks 3, 6, 7 β€” the survivors are already re-homed. -- Produces: nothing. `contexts/v2`, `validators/v2`, `cli/index` no longer exist. +Copy `$V2REF/extralit-server/src/extralit_server/contexts/v2/projection.py` to `extralit-server/src/extralit_server/contexts/projection.py`, then make exactly these changes. **Do not touch `_INPUT_TABLES_DDL`, `_INSERTS`, `_DENORMALIZE_SQL`, or `_run_denormalization`** β€” the DuckDB staging tables are named independently of the Postgres tables, so the ~130-line denormalization SQL is unaffected. -- [ ] **Step 1: Confirm nothing outside these trees still imports them** +1. Delete `build_reference_view` and `_build_columns`'s `ProjectionCell`/`ProjectionRecord`/`ProjectionView` imports; delete the `contexts.v2.records` import. +2. Imports become `from extralit_server.models.database import Dataset, Question, Record, Response, Suggestion` and `from extralit_server.api.schemas.v1.projection import (...)`. +3. In `build_workspace_view`, replace the schema query with a dataset query filtered to schema-backed datasets β€” this is the new discriminator and the reason the projection cannot simply select every dataset in the workspace: -```bash -cd extralit-server && grep -rn "contexts\.v2\|contexts import v2\|validators\.v2\|validators import v2\|cli\.index\|index_sync" src tests --include=*.py \ - | grep -v "^src/extralit_server/contexts/v2/" \ - | grep -v "^src/extralit_server/validators/v2/" \ - | grep -v "^src/extralit_server/cli/index/" \ - | grep -v "^tests/integration/contexts/v2/" \ - | grep -v "^tests/unit/validators/v2/" +```python + datasets = ( + ( + await db.execute( + select(Dataset) + .where( + Dataset.workspace_id == workspace_id, + # Only schema-backed datasets are extraction projects; a plain + # annotation dataset in the same workspace has no column manifest + # and must not contribute columns or rows to the grid. + Dataset.current_schema_version_id.is_not(None), + ) + .order_by(Dataset.name) + ) + ) + .scalars() + .all() + ) ``` -Expected remaining hits, all of which this task removes: `src/extralit_server/cli/__init__.py:13` and `tests/unit/test_annotation_no_index_import.py`. If anything else appears, stop and fold that caller onto its v1 equivalent before deleting. +4. Substitute throughout: `V2Question` β†’ `Question`, `V2Record` β†’ `Record`, `V2Response` β†’ `Response`, `V2Suggestion` β†’ `Suggestion`, `Schema` β†’ `Dataset`, `.schema_id` β†’ `.dataset_id`, `schema_names` β†’ `dataset_names`. +5. `question.columns` becomes `question.settings.get("columns") or []` β€” there are two sites: `_build_columns`'s table branch and the `question_columns` input tuple. +6. `question.type` still works unchanged: v1 `Question.type` (`models/database.py:322`) is a property reading `settings["type"]`. +7. `Response.status == ResponseStatus.submitted` is unchanged β€” v1's enum has the same member. -- [ ] **Step 2: Delete** +- [ ] **Step 5: Run the context tests** ```bash -cd extralit-server && rm -rf src/extralit_server/contexts/v2 src/extralit_server/validators/v2 \ - src/extralit_server/cli/index tests/integration/contexts/v2 tests/unit/validators/v2 \ - tests/integration/cli/test_index_reindex.py tests/unit/test_annotation_no_index_import.py +cd extralit-server && uv run pytest tests/unit/contexts/test_projection.py -v ``` -`tests/unit/test_annotation_no_index_import.py` goes because the constraint it enforced β€” annotation must never reach the index β€” is the *cause* of bug 2. v1 syncs responses and suggestions to the index by design; a guard forbidding that would now be actively wrong. +Expected: all 17 pass. -- [ ] **Step 3: Unregister the index CLI** +- [ ] **Step 6: Write the failing handler tests** -In `cli/__init__.py`, delete the `index_app` import and `app.add_typer(index_app, name="index")` (`:13`). Leave `cli/search_engine/` alone β€” that is v1's mature reindexer and stays. +Create `extralit-server/tests/unit/api/handlers/v1/test_projection.py` by porting the workspace-projection half of `$V2REF/extralit-server/tests/integration/api/v2/test_projection.py` (drop the `/projection/references/{ref}` tests β€” that endpoint is deleted). Keep the pagination test, the `limit=101` rejection, the missing-`workspace_id` 422, and the annotator authorization test. -- [ ] **Step 4: Fix the one index test that referenced a deleted enum** +- [ ] **Step 7: Write the handler and register it** -`index/`'s *source* is model-agnostic, but one of its tests is not. `tests/integration/index/test_lancedb_engine.py:14-25` defines a local `_Rec` test double that imports `V2RecordStatus` (deleted in Task 1) and sets `schema_version_id` (a column that no longer exists). It is a plain stub, so this is a two-line change: +Create `api/handlers/v1/projection.py`. Authorize with `DatasetPolicy.list(workspace_id)` β€” the same predicate `GET /me/datasets` uses (`api/handlers/v1/datasets/datasets.py:74`) β€” rather than a new policy class. The `/me/` prefix follows the v1 convention for user-scoped reads. ```python -class _Rec: - def __init__(self, title, year, reference="pmid:1", external_id=None): - from extralit_server.enums import RecordStatus +from typing import Annotated +from uuid import UUID - self.id = uuid4() - self.reference = reference - self.status = RecordStatus.pending - self.external_id = external_id - self.fields = {"title": title, "year": year} -``` +from fastapi import APIRouter, Depends, Query, Security +from sqlalchemy.ext.asyncio import AsyncSession -If dropping `schema_version_id` makes `index/mapping.py:record_to_row` fail, that is because `index/mapping.py:17` `SYSTEM_FIELDS` still lists it. Remove it there too and note in ENG-36 that the Lance row layout no longer pins a schema version β€” that pin is gone from `records` deliberately (bug 4). +from extralit_server.api.policies.v1 import DatasetPolicy, authorize +from extralit_server.api.schemas.v1.projection import WorkspaceProjection +from extralit_server.contexts import projection +from extralit_server.database import get_async_db +from extralit_server.models.database import User +from extralit_server.security import auth -- [ ] **Step 5: Verify the index engine still stands alone** +router = APIRouter(tags=["projection"]) -```bash -cd extralit-server && uv run pytest tests/unit/index tests/integration/index -v -``` +LIST_PROJECTION_LIMIT_DEFAULT = 50 +LIST_PROJECTION_LIMIT_LE = 100 -Expected: 24 passed. Any *other* failure means the engine had a hidden dependency on `models/v2` β€” record it in ENG-36 and fix the test, not by resurrecting `index_sync`. -- [ ] **Step 6: Verify the CLI still starts** +@router.get("/me/datasets/projection", response_model=WorkspaceProjection) +async def get_workspace_projection( + *, + workspace_id: Annotated[UUID, Query(description="The workspace to project")], + offset: int = 0, + limit: Annotated[int, Query(ge=1, le=LIST_PROJECTION_LIMIT_LE)] = LIST_PROJECTION_LIMIT_DEFAULT, + db: Annotated[AsyncSession, Depends(get_async_db)], + current_user: Annotated[User, Security(auth.get_current_user)], +): + await authorize(current_user, DatasetPolicy.list(workspace_id)) -```bash -cd extralit-server && uv run python -m extralit_server --help + # offset/limit count references, not fan-out rows β€” a reference with a stacked table + # question spans several rows and must never be split across a page boundary. + return await projection.build_workspace_view(db, workspace_id=workspace_id, offset=offset, limit=limit) ``` -Expected: help text with no `index` subcommand and with `search_engine` still present. +Register it in `api/routes.py:90` by adding `projection_v1.router` to the router list, with the matching import alongside the other `from extralit_server.api.handlers.v1 import ... as ..._v1` lines. -- [ ] **Step 7: Lint and commit** +**Route-ordering check:** `/me/datasets/projection` must be declared before any `/me/datasets/{dataset_id}`-style route, or FastAPI will match `projection` as a `dataset_id` and return a UUID parse error. Confirm by running the 404 test in Step 6 β€” if it returns 422 instead of a projection, move the `include_router` call for `projection_v1` above `datasets_v1` in the list. + +- [ ] **Step 8: Run the handler tests** ```bash -cd extralit-server && uv run ruff check +cd extralit-server && uv run pytest tests/unit/api/handlers/v1/test_projection.py -v ``` +Expected: all pass. + +- [ ] **Step 9: Commit** + ```bash -git add -A extralit-server/src/extralit_server extralit-server/tests -git commit -m "refactor(server)!: delete contexts/v2, validators/v2, and cli/index +git add extralit-server/src/extralit_server/contexts/projection.py \ + extralit-server/src/extralit_server/api/schemas/v1/projection.py \ + extralit-server/src/extralit_server/api/handlers/v1/projection.py \ + extralit-server/src/extralit_server/api/routes.py \ + extralit-server/tests/unit/contexts/test_projection.py \ + extralit-server/tests/unit/api/handlers/v1/test_projection.py +git commit -m "feat(server): move the workspace projection onto v1 tables -The LanceDB engine in index/ is kept untouched; only its v2 glue goes. -Registering it as a SearchEngine implementation is ENG-36. Drops the -no-index-import guard, which is what made v2 review data unsearchable." +DuckDB denormalization SQL is unchanged. Adds the schema-backed discriminator +(Dataset.current_schema_version_id IS NOT NULL) so plain annotation datasets +in the same workspace do not leak into the extraction grid." ``` --- -### Task 11: Delete `models/v2` and the v2 test factories +### Task 11: Regression tests for the two bugs the v2 annotation path hid + +No new production code β€” v1's `contexts/datasets.py` already does the right thing. These tests pin the behavior that was missing, so the fold cannot silently regress it. **Files:** -- Delete: `extralit-server/src/extralit_server/models/v2/` (entire directory) -- Delete: `extralit-server/tests/integration/models/v2/`, `extralit-server/tests/integration/test_enums_v2.py` -- Modify: `extralit-server/src/extralit_server/models/__init__.py`, `extralit-server/tests/factories.py` +- Test: `extralit-server/tests/unit/contexts/test_extraction_response_side_effects.py` (create) **Interfaces:** -- Consumes: Tasks 9 and 10 β€” every importer is gone. -- Produces: `models/v2` no longer exists; `SchemaVersionFactory` is retargeted to `dataset`. +- Consumes: everything from Tasks 4–10. +- Produces: nothing. Tests only. -- [ ] **Step 1: Confirm no importers remain** +- [ ] **Step 1: Write the tests** -```bash -cd extralit-server && grep -rn "models\.v2\|models import v2\|V2Record\|V2Question\|V2Response\|V2Suggestion" src tests --include=*.py \ - | grep -v "^src/extralit_server/models/v2/" -``` +```python +"""Side effects the v2 annotation path deliberately omitted. -Expected hits only in `src/extralit_server/models/__init__.py:8-9`, `tests/factories.py:655-750`, `tests/integration/models/v2/`, and `tests/integration/test_enums_v2.py`. +contexts/v2/annotation.upsert_response never touched record status and was forbidden +by tests/unit/test_annotation_no_index_import.py from reaching any index. Both are +required behavior; v1's contexts/datasets.upsert_response supplies them. These tests +exist so folding onto v1 cannot silently lose them again. +""" -- [ ] **Step 2: Delete** +import pytest -```bash -cd extralit-server && rm -rf src/extralit_server/models/v2 tests/integration/models/v2 \ - tests/integration/test_enums_v2.py -``` +from extralit_server.api.schemas.v1.responses import ResponseUpsert +from extralit_server.contexts import datasets as datasets_ctx +from extralit_server.enums import RecordStatus, ResponseStatus +from tests.factories import ( + AnnotatorFactory, + DatasetFactory, + QuestionFactory, + RecordFactory, + WorkspaceFactory, +) -- [ ] **Step 3: Unwire the metadata registration** -In `models/__init__.py`, delete lines 8–9 (`from .v2 import Schema, SchemaVersion` and `from .v2 import Record as V2Record`). The new `SchemaVersion` lives in `models/database.py` and is already registered by the star-export above. +@pytest.mark.asyncio +class TestExtractionResponseSideEffects: + async def _setup(self, db): + workspace = await WorkspaceFactory.create() + dataset = await DatasetFactory.create( + workspace=workspace, status="ready", distribution={"strategy": "overlap", "min_submitted": 1} + ) + question = await QuestionFactory.create( + dataset=dataset, name="population", settings={"type": "text", "use_markdown": False} + ) + record = await RecordFactory.create(dataset=dataset, reference="10.1000/j.foo.2020.01") + user = await AnnotatorFactory.create(workspaces=[workspace]) + return dataset, question, record, user -- [ ] **Step 4: Retarget the factories** + async def test_submitting_a_response_completes_the_record(self, db, mock_search_engine): + dataset, question, record, user = await self._setup(db) + assert record.status == RecordStatus.pending -In `tests/factories.py`, delete `SchemaFactory` (`:655`), `V2RecordFactory` (`:679`), `V2QuestionFactory` (`:703`), `V2SuggestionFactory` (`:726`), and `V2ResponseFactory` (`:746`). Rewrite `SchemaVersionFactory` (`:666`) against the new model β€” `dataset = SubFactory(DatasetFactory)`, no `columns_cache`, no `review_widgets`: + await datasets_ctx.upsert_response( + db, + mock_search_engine, + record, + user, + ResponseUpsert( + record_id=record.id, + status=ResponseStatus.submitted, + values={"population": {"value": "Kenya"}}, + ), + ) -```python -class SchemaVersionFactory(BaseFactory): - class Meta: - model = SchemaVersion + await db.refresh(record) + assert record.status == RecordStatus.completed - dataset = SubFactory(DatasetFactory) - version = 1 - object_key = LazyAttribute(lambda v: f"schemas/{v.dataset.id}/v{v.version}.json") - etag = "etag" - checksum = "checksum" -``` + async def test_a_draft_response_leaves_the_record_pending(self, db, mock_search_engine): + dataset, question, record, user = await self._setup(db) -Add a `ColumnFieldFactory` next to the existing `TextFieldFactory` so later tests and the projection tests have a one-liner for a declared column: + await datasets_ctx.upsert_response( + db, + mock_search_engine, + record, + user, + ResponseUpsert( + record_id=record.id, + status=ResponseStatus.draft, + values={"population": {"value": "Kenya"}}, + ), + ) -```python -class ColumnFieldFactory(FieldFactory): - settings = {"type": "column", "dtype": "str", "nullable": True} -``` + await db.refresh(record) + assert record.status == RecordStatus.pending -Match the surrounding factory style β€” check whether the file uses `factory.SubFactory` or a bare imported `SubFactory` and follow it. + async def test_submitting_a_response_reaches_the_search_index(self, db, mock_search_engine): + dataset, question, record, user = await self._setup(db) + + await datasets_ctx.upsert_response( + db, + mock_search_engine, + record, + user, + ResponseUpsert( + record_id=record.id, + status=ResponseStatus.submitted, + values={"population": {"value": "Kenya"}}, + ), + ) -- [ ] **Step 5: Run the full suite** + mock_search_engine.update_record_response.assert_awaited() -```bash -cd extralit-server && uv run pytest tests -q --disable-warnings -``` + async def test_upserting_a_suggestion_reaches_the_search_index(self, db, mock_search_engine): + from extralit_server.api.schemas.v1.suggestions import SuggestionCreate -Expected: all pass. Tasks 9–11 delete about 160 v2 tests (β‰ˆ54 in Task 9, β‰ˆ92 in Task 10, β‰ˆ16 in Task 11), so the collected count should drop by roughly that much relative to the run at the end of Task 8. Any *failure* here is a real fold gap β€” fix it in the v1 code, not by restoring a v2 module. + dataset, question, record, user = await self._setup(db) -- [ ] **Step 6: Lint and commit** + await datasets_ctx.upsert_suggestion( + db, + mock_search_engine, + record, + question, + SuggestionCreate(question_id=question.id, value="Kenya", agent="gpt-x", score=0.9), + ) -```bash -cd extralit-server && uv run ruff check + mock_search_engine.update_record_suggestion.assert_awaited() ``` +- [ ] **Step 2: Run them** + ```bash -git add -A extralit-server/src/extralit_server/models extralit-server/tests -git commit -m "refactor(server)!: delete models/v2 +cd extralit-server && uv run pytest tests/unit/contexts/test_extraction_response_side_effects.py -v +``` -Schema folds into Dataset; V2Record/V2Question/V2Response/V2Suggestion fold -into records/questions/responses/suggestions. SchemaVersionFactory is -retargeted to dataset." +Expected: 5 passed on the first run β€” v1 already behaves correctly. If `test_submitting_a_response_completes_the_record` fails, read `contexts/distribution.py:61` and check the `distribution` dict shape the factory produced matches what `distribution_strategy` expects; fix the fixture, not the production code. + +- [ ] **Step 3: Commit** + +```bash +git add extralit-server/tests/unit/contexts/test_extraction_response_side_effects.py +git commit -m "test(server): pin the record-status and index side effects v2 omitted" ``` --- @@ -2375,7 +2478,7 @@ cd extralit-server && uv run alembic -c src/extralit_server/alembic.ini heads \ && uv run alembic -c src/extralit_server/alembic.ini history | head -20 ``` -Expected: exactly one head β€” the Task 1 revision, with `down_revision = "54d65879a68e"`. No revision should mention `schemas`, `v2_records`, `v2_questions`, `v2_responses`, `v2_suggestions`, or any `v2_*_enum`. +Expected: exactly one head β€” the Task 4 revision, with `down_revision = "54d65879a68e"`. No revision should mention `schemas`, `v2_records`, `v2_questions`, `v2_responses`, `v2_suggestions`, or any `v2_*_enum`. - [ ] **Step 2: Verify a from-scratch database builds** @@ -2446,7 +2549,7 @@ The 8 live endpoints must keep working. This task changes paths and response sha - Test: the colocated `*.test.ts` for each modified repository **Interfaces:** -- Consumes: the v1 endpoints from Tasks 4–7. +- Consumes: the v1 endpoints from Tasks 7–10. - Produces: repositories calling `/v1/...`; `ColumnMeta` built from a v1 `Field`; `SchemaVersion` without `columns` or `reviewWidgets`. - [ ] **Step 1: Update the repository tests first** @@ -2652,7 +2755,7 @@ SchemaRecord, V2RecordRepository -> SchemaRecordRepository), and deletes v2/." | `PUT /api/v2/records/{id}/responses` (`:191`) | `POST /api/v1/records/{id}/responses` | | `POST /api/v2/schemas/{id}:rebuild-index` (`:196`) | **delete** β€” v1 indexes on write | -Note the ordering change: `POST /datasets` creates a *draft*, and `POST /datasets/{id}/schema-versions` is what publishes it (Task 3 sets `status=ready` and calls `create_index`). Records can only be created against a ready dataset (`RecordsBulkCreateValidator._validate_dataset_is_ready`), so the seed must publish before it upserts records β€” the v2 script already had that order. +Note the ordering change: `POST /datasets` creates a *draft*, and `POST /datasets/{id}/schema-versions` is what publishes it (Task 6 sets `status=ready` and calls `create_index`). Records can only be created against a ready dataset (`RecordsBulkCreateValidator._validate_dataset_is_ready`), so the seed must publish before it upserts records β€” the v2 script already had that order. - [ ] **Step 2: Rename the e2e project** @@ -2766,7 +2869,7 @@ records, matching v1's RecordsBulkCreateValidator." 4. **`/api/v2` is unreachable** β€” Task 15 Step 6 returns 404. -5. **Full server suite green.** Net test count should land roughly 60–70 below the pre-plan baseline: about 160 v2 tests deleted, about 95 new tests added across Tasks 1–8, and the 24 `index/` tests untouched. +5. **Full server suite green.** Net test count should land roughly 60–70 below the pre-plan baseline: about 160 v2 tests deleted across Tasks 1–3, about 95 new tests added across Tasks 4–11, and the 24 `index/` tests untouched. ```bash cd extralit-server && uv run pytest tests -q --disable-warnings ``` diff --git a/docs/superpowers/specs/2026-06-27-schema-centric-data-model-design.md b/docs/superpowers/specs/2026-06-27-schema-centric-data-model-design.md index 4559c7669..33950a6ce 100644 --- a/docs/superpowers/specs/2026-06-27-schema-centric-data-model-design.md +++ b/docs/superpowers/specs/2026-06-27-schema-centric-data-model-design.md @@ -1,5 +1,7 @@ # Schema-Centric Data Model β€” Design Spec +> **Historical note (2026-07-26):** The `/api/v2` parallel tree described in this document was folded back into `/api/v1`. See `docs/superpowers/plans/2026-07-26-fold-v2-into-v1.md`. This document is kept as a historical record; its API paths, models, and file references may no longer exist. + **Date:** 2026-06-27 **Status:** Approved design (server data model + API; SDK/frontend follow) **Author:** brainstorming session (Jonny + Claude) diff --git a/docs/superpowers/specs/2026-07-09-v2-frontend-vertical-slice-design.md b/docs/superpowers/specs/2026-07-09-v2-frontend-vertical-slice-design.md index 3e92ce4e9..710c02d92 100644 --- a/docs/superpowers/specs/2026-07-09-v2-frontend-vertical-slice-design.md +++ b/docs/superpowers/specs/2026-07-09-v2-frontend-vertical-slice-design.md @@ -1,5 +1,7 @@ # v2 Frontend Vertical Slice β€” Design Spec +> **Historical note (2026-07-26):** The `/api/v2` parallel tree described in this document was folded back into `/api/v1`. See `docs/superpowers/plans/2026-07-26-fold-v2-into-v1.md`. This document is kept as a historical record; its API paths, models, and file references may no longer exist. + **Date:** 2026-07-09 **Status:** Proposed design (frontend phase of the schema-centric v2 model) **Parent spec:** `2026-06-27-schema-centric-data-model-design.md` (Β§19 points here) diff --git a/docs/superpowers/specs/2026-07-13-sdk-v2-redesign-design.md b/docs/superpowers/specs/2026-07-13-sdk-v2-redesign-design.md index 4f073f3b2..d5d71d375 100644 --- a/docs/superpowers/specs/2026-07-13-sdk-v2-redesign-design.md +++ b/docs/superpowers/specs/2026-07-13-sdk-v2-redesign-design.md @@ -1,5 +1,7 @@ # Python SDK v2 β€” schema-centric client, agentic CLI, async performance +> **Historical note (2026-07-26):** The `/api/v2` parallel tree described in this document was folded back into `/api/v1`. See `docs/superpowers/plans/2026-07-26-fold-v2-into-v1.md`. This document is kept as a historical record; its API paths, models, and file references may no longer exist. + **Date:** 2026-07-13 **Status:** Approved design, pending implementation plan **Companions:** `2026-06-27-schema-centric-data-model-design.md` (server model), diff --git a/docs/superpowers/specs/2026-07-19-reference-review.md b/docs/superpowers/specs/2026-07-19-reference-review.md index a2b5a2deb..724477222 100644 --- a/docs/superpowers/specs/2026-07-19-reference-review.md +++ b/docs/superpowers/specs/2026-07-19-reference-review.md @@ -1,5 +1,7 @@ # Handoff β€” ReferenceReview slice: design/correctness interrogation brief +> **Historical note (2026-07-26):** The `/api/v2` parallel tree described in this document was folded back into `/api/v1`. See `docs/superpowers/plans/2026-07-26-fold-v2-into-v1.md`. This document is kept as a historical record; its API paths, models, and file references may no longer exist. + **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. diff --git a/docs/superpowers/specs/2026-07-20-extraction-table-design.md b/docs/superpowers/specs/2026-07-20-extraction-table-design.md index 0d249685d..d6770e601 100644 --- a/docs/superpowers/specs/2026-07-20-extraction-table-design.md +++ b/docs/superpowers/specs/2026-07-20-extraction-table-design.md @@ -1,5 +1,7 @@ # Extraction Table β€” Design Spec +> **Historical note (2026-07-26):** The `/api/v2` parallel tree described in this document was folded back into `/api/v1`. See `docs/superpowers/plans/2026-07-26-fold-v2-into-v1.md`. This document is kept as a historical record; its API paths, models, and file references may no longer exist. + **Date:** 2026-07-20 **Branch:** `feat/v2-ui-extraction-table` (based on `develop` @ `52eab556f`, PR #232 merged) **Predecessors:** `2026-06-27-schema-centric-data-model-design.md` (Β§17.4 projection), diff --git a/docs/superpowers/specs/2026-07-24-extraction-projection-acceptance.md b/docs/superpowers/specs/2026-07-24-extraction-projection-acceptance.md index dccfc70a6..b0db2e8f4 100644 --- a/docs/superpowers/specs/2026-07-24-extraction-projection-acceptance.md +++ b/docs/superpowers/specs/2026-07-24-extraction-projection-acceptance.md @@ -1,5 +1,7 @@ # Extraction Projection Viewer β€” Acceptance Criteria +> **Historical note (2026-07-26):** The `/api/v2` parallel tree described in this document was folded back into `/api/v1`. See `docs/superpowers/plans/2026-07-26-fold-v2-into-v1.md`. This document is kept as a historical record; its API paths, models, and file references may no longer exist. + **Date:** 2026-07-24 **Branch:** `feat/v2-ui-extraction-grid` **Derives from:** `2026-07-20-extraction-table-design.md` Β§3.1–§3.4 (grid semantics, diff --git a/extralit-frontend/.gitignore b/extralit-frontend/.gitignore index e4086ccca..fd16483d1 100644 --- a/extralit-frontend/.gitignore +++ b/extralit-frontend/.gitignore @@ -11,5 +11,5 @@ dist/ .vercel .env* -# v2 e2e generated seed output (regenerated per run) -e2e/v2/seed/seed-output.json +# extraction e2e generated seed output (regenerated per run) +e2e/extraction/seed/seed-output.json diff --git a/extralit-frontend/CLAUDE.md b/extralit-frontend/CLAUDE.md index 5e6987246..7b61d00d0 100644 --- a/extralit-frontend/CLAUDE.md +++ b/extralit-frontend/CLAUDE.md @@ -49,19 +49,20 @@ npm run e2e:report # View test report > `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) +## extraction e2e suite (`e2e/extraction/`, real backend β€” the extraction 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. +Separate Playwright project (`--project=extraction`, `testMatch: extraction/**/*.spec.ts`); +the legacy Argilla specs above are **not** an extraction gate. No network mocking β€” it +exercises real bearer auth on `/api/v1`, slashed-DOI encoding, the suggestionβ†’response loop, +drafts and search freshness. Env knobs (see `e2e/extraction/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) +npm run e2e:extraction:seed # uv run ../extralit-server python e2e/extraction/seed/seed_v2_e2e.py +npm run dev -- --host # dev server reachable from the browser +npm run e2e:extraction # playwright test --project=extraction (local chromium) ``` Requires the full stack up with the server on :6900. On the Orin host the backing services diff --git a/extralit-frontend/components/features/extractions/ExtractionsGrid.client.test.ts b/extralit-frontend/components/features/extractions/ExtractionsGrid.client.test.ts index 3a89ac2d6..f2fbbbe0f 100644 --- a/extralit-frontend/components/features/extractions/ExtractionsGrid.client.test.ts +++ b/extralit-frontend/components/features/extractions/ExtractionsGrid.client.test.ts @@ -1,7 +1,7 @@ import { flushPromises, mount } from "@vue/test-utils"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import ExtractionsGrid from "./ExtractionsGrid.client.vue"; -import { WorkspaceProjection } from "~/v2/domain/entities/projection/WorkspaceProjection"; +import { WorkspaceProjection } from "~/v1/domain/entities/projection/WorkspaceProjection"; // Module-level so the `vi.mock` factory below (evaluated once, hoisted) can close over them. // Call counts and implementations are reset/re-established per spec in `beforeEach` β€” without @@ -25,8 +25,8 @@ const PROJECTION = new WorkspaceProjection( [ { name: "Design.type", - schemaId: "s-1", - schemaName: "Design", + datasetId: "s-1", + datasetName: "Design", questionName: "type", subColumn: null, dtype: "text", @@ -50,8 +50,8 @@ const PROJECTION_2 = new WorkspaceProjection( [ { name: "Design.type", - schemaId: "s-2", - schemaName: "Design", + datasetId: "s-2", + datasetName: "Design", questionName: "type", subColumn: null, dtype: "text", @@ -212,8 +212,8 @@ describe("ExtractionsGrid", () => { [ { name: "Design.type", - schemaId: "s-1", - schemaName: "Design", + datasetId: "s-1", + datasetName: "Design", questionName: "type", subColumn: null, dtype: "text", diff --git a/extralit-frontend/components/features/extractions/ExtractionsGrid.client.vue b/extralit-frontend/components/features/extractions/ExtractionsGrid.client.vue index bf1debed3..9962fe882 100644 --- a/extralit-frontend/components/features/extractions/ExtractionsGrid.client.vue +++ b/extralit-frontend/components/features/extractions/ExtractionsGrid.client.vue @@ -6,8 +6,8 @@ import { onBeforeUnmount, onMounted, ref, watch } from "vue"; import { type HTMLPerspectiveViewerElement } from "@perspective-dev/viewer"; import { initPerspectiveClient } from "~/components/features/extractions/perspective-bootstrap"; -import { type WorkspaceProjection, type ProjectionGridCell } from "~/v2/domain/entities/projection/WorkspaceProjection"; -import { toPerspectiveData, cellAt, bandParity } from "~/v2/domain/entities/projection/grid-adapter"; +import { type WorkspaceProjection, type ProjectionGridCell } from "~/v1/domain/entities/projection/WorkspaceProjection"; +import { toPerspectiveData, cellAt, bandParity } from "~/v1/domain/entities/projection/grid-adapter"; /** * Vue wrapper around `` (Β§3.1/Β§3.3 extraction grid). The `.client.vue` @@ -204,7 +204,7 @@ function handleClick(event: Event): void { if (!cell || !row || !column) { return; } - emit("cell-click", { cell, reference: row.reference, schemaId: column.schemaId, columnName: at.columnName }); + emit("cell-click", { cell, reference: row.reference, schemaId: column.datasetId, columnName: at.columnName }); } /** diff --git a/extralit-frontend/components/features/schemas/RecordsTable.vue b/extralit-frontend/components/features/schemas/RecordsTable.vue index 147fb9499..5b03213fa 100644 --- a/extralit-frontend/components/features/schemas/RecordsTable.vue +++ b/extralit-frontend/components/features/schemas/RecordsTable.vue @@ -21,12 +21,12 @@