refactor(server): fold the /api/v2 parallel tree into v1 - #236
Open
JonnyTran wants to merge 28 commits into
Open
refactor(server): fold the /api/v2 parallel tree into v1#236JonnyTran wants to merge 28 commits into
JonnyTran wants to merge 28 commits into
Conversation
…the v1 model models/v2/schemas.py already binds __tablename__ = "schema_versions" on the shared DatabaseModel metadata, so the new v1 SchemaVersion cannot be declared while it exists. Tasks 9-11 (deletions) become Tasks 1-3; the build tasks shift to 4-11; 12-15 are unchanged. Task 1 now snapshots the v2 tree to a git-ignored workspace dir and tags v2-pre-fold, because every later task's "Reference: .../v2/..." path is deleted before that task runs. Deletion boundaries were also redrawn so each task leaves a green suite: the tests that import contexts/v2 die with api/schemas/v2 and move into Task 1; SchemaVersionFactory is deleted in Task 3 and re-added in Task 4.
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.
Also:
- Retarget tests/integration/conftest.py's async_client override onto api_v1
(it was previously registered on api_v2, so test_rq_groups_workflow.py was
never actually getting the test session for its v1 routes).
- Fix test_rq_groups_workflow.py: its fixtures referenced a non-existent
`async_db` param (should be the root conftest's autouse `db` fixture) and
built Workspace() with title/description kwargs the model no longer has -
both bugs were masked because every test errored at fixture setup before
reaching them. Skip the two tests that hit a separate, pre-existing
SQLite single-writer lock ('database is locked') caused by
create_document_workflow() opening its own AsyncSessionLocal() connection
outside the test's nested transaction - unrelated to this fold, needs a
session-injection seam or different isolation strategy.
- Update test_openapi_dump.py assertions from the v2 schema shape to v1's.
- Add tests/unit/api/test_api_mounts.py pinning that only /api/v1 is mounted.
…or real Task 1 review flagged two skipped tests in test_rq_groups_workflow.py as unaccountable. Root cause (confirmed correct by review): create_document_workflow() opens its own AsyncSessionLocal() connection, which deadlocks under SQLite's single-writer lock against the db fixture's nested-transaction connection. Without touching production code, add a `use_fixture_session_for_workflow` fixture that patches AsyncSessionLocal at the call site to return the test's own db session (neutralizing db.close so the shared session survives past create_document_workflow's `async with` block). This makes test_create_document_workflow_with_rq_groups pass for real - no longer skipped. test_concurrent_workflow_processing cannot be fixed the same way: it runs three create_document_workflow() calls concurrently via asyncio.gather, and a single AsyncSession cannot be used by overlapping coroutines (confirmed empirically - sqlalchemy.exc.IllegalStateChangeError: "bind() is already in progress"). Kept skipped, now citing ENG-37 (test-session architecture: workflows open their own AsyncSessionLocal) as the tracked follow-up.
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.
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.
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.
Migration history rewrite (HIGH, job 282): the four deleted revisions are live on origin/develop, so any database migrated before this branch strands alembic_version at c1510e93882a. Keeps the rewrite per the plan's pre-production constraint but ships the recovery path — 13da2d87e660's docstring and a new CLAUDE.md section cover rebuild and stamp-forward, including the Postgres schema_versions name collision. Verified upgrade -> downgrade -1 -> upgrade round-trips on SQLite. Table-question suggestion scores (job 280): ports the carve-out deleted with validators/v2/values.py into SuggestionCreateValidator._validate_score, so a multi-row table value keeps its single whole-suggestion confidence score instead of 422ing. Not reachable through v1 schemas yet — SuggestionCreate.value has no table-row variant — so the tests drive the branch with list[str] and the module says so. SchemaVersionFactory (job 282/281): object_key dereferenced a SubFactory inside a LazyAttribute, which sees an un-awaited coroutine. Derived from version only, comment restored, pinned by a test that actually calls the factory. Test hygiene: test_api_mounts no longer runs create_server_app (base_url wrapper + configure_app_statics temp-dir leak) and filters on Mount; the rq-groups test asserts the commit via a spy (mutation-verified); both conftests pop only their own dependency_overrides keys so ordering under -p randomly is safe. Also: passive_deletes on Dataset.schema_versions, ColumnFieldFactory dtype str -> string (with the same correction applied to every dtype literal left in the plan), narrowed pytest.raises(Exception), openapi-dump help repointed to v1, index/__init__ documents its ENG-36 parking and the Lance layout break, empty test packages removed. Plan updated for findings binding later tasks: dtype strings, the dropped get_s3_client override (Task 7), and the empty-pandera-body 500 (Task 6).
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.
…t null
nullable is unguarded while ColumnFieldSettings.nullable is non-Optional: a
PATCH body of {"type": "column", "nullable": null} passed validation, then
Field.fill() dict-merged nullable: None into stored settings JSON, breaking
every later parse of that field via Field.settings. Add "nullable" to
__non_nullable_fields__, cover ColumnFieldSettingsUpdate directly (dtype-only
and review-only partial updates, explicit-null rejection for both fields),
and rename a validator test to match its body after Task 5 review.
…olumn fields 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'].
Replaces POST/GET /api/v2/schemas/{id}/versions. GET /schemas/{id}/columns is
dropped: the derived columns are readable from GET /datasets/{id}/fields.
…d list
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).
Also wires reference through the single-record PATCH /api/v1/records/{id}
path (contexts/records.py::update_record) under the same is_set(...)
semantics, closing a silent no-op gap the new RecordUpdate.reference field
would otherwise leave on that endpoint. Updates the pre-existing full-dict
response assertions across test_records.py, test_datasets.py,
test_list_dataset_records.py, and friends to include the new reference key.
…nd PATCH Task 8 review found the omitted-vs-explicit-null distinction for Record.reference was only inferred by analogy to metadata's is_set semantics, not proven. Adds the missing null-clears-value case on both paths: bulk upsert (PUT .../records/bulk) and single-record PATCH.
…ns'] Replaces V2Question.columns and validators/v2/questions.QuestionBindingValidator, retargeted from SchemaVersion.columns_cache to the dataset's column fields.
The create-path suite (POST /datasets/{id}/questions) never exercised
QuestionColumnBindingValidator via QuestionUpdateValidator, so a regression
in the update call site (or the selectinload(Dataset.fields) eager-load
fix) would go undetected. Add PATCH coverage: valid binding persists
(re-read from DB), invalid binding is rejected and not persisted, and the
scalar arity rule holds on update too.
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.
Columns now come from GET /datasets/{id}/fields, search from v1's
Elasticsearch-backed records/search (authoritative total). Deletes
AnnotationRepository and the three review use-cases, orphaned since #234,
and the rebuild-index button (reindex is a CLI). Also drops the now-dead
gen:api codegen scripts and the two CI gates that diffed against the
deleted v2 OpenAPI snapshot/generated client.
Task 4 of the v2-fold plan added current_schema_version_id to the
Dataset model but no task added it to the v1 Dataset response schema,
so GET /api/v1/datasets/{id} and GET /api/v1/me/datasets never
serialized it — breaking the frontend's schema-backed-dataset filter
on the /schemas page.
DatasetGetterDict needs no new branch: the field name matches the ORM
column exactly, so pydantic's default GetterDict.get(key) -> getattr
fallback already resolves it.
Merges the DI containers, drops every V2 name prefix (V2Record -> SchemaRecord, V2RecordRepository -> SchemaRecordRepository), and deletes v2/.
Rewrites seed_v2_e2e.py's calls onto the v1 endpoints per the fold-v2-into-v1
plan's Task 15: dataset CRUD via /api/v1/datasets, publish via POST
.../schema-versions, records via PUT .../records/bulk, suggestions via PUT
/api/v1/records/{id}/suggestions, responses via POST /api/v1/records/{id}/responses.
Drops the v2 :rebuild-index call (v1 indexes on write).
Question creation had to move before the schema-version publish call:
QuestionCreateValidator rejects question creation once a dataset is `ready`,
and publishing a schema version sets `ready` as a side effect. Column
bindings for the `size`/`notes` text questions are therefore added via a
PATCH /api/v1/questions/{id} after publish, once the column Fields the
binding validates against actually exist -- QuestionUpdateValidator, unlike
the create validator, doesn't gate on dataset readiness. This ordering
constraint isn't reflected in the task-15 brief's mapping table; the real v1
validators win.
Renames the Playwright project v2 -> extraction (playwright.config.ts) and
git-mv's e2e/v2 -> e2e/extraction, updating every referrer found by grep:
package.json's e2e:v2* scripts, e2e/extraction/README.md, fixtures.ts and
CLAUDE.md doc comments, the demo/ run script and its READMEs, .gitignore's
seed-output.json pattern, and a comment in pages/index.test.ts.
Steps 3+ (live-stack verification) are explicitly out of scope for this
commit -- local Elasticsearch is wedged at its shard cap and needs clearing
first. Not run: server start, frontend dev server, or Playwright.
auth-smoke.spec.ts and extractions-grid.spec.ts still asserted against dead
v2 routes (/api/v2/schemas, /api/v2/projection) after the Step 2 rename --
carried along by git mv but never repointed. Both would hang until
Playwright's timeout against a live v1-only server, a guaranteed red
unrelated to the product.
Repointed against the frontend's actual v1 callers (confirmed against the
v1 handlers, read-only):
- SchemaRepository.getSchemas() -> GET /api/v1/me/datasets?workspace_id=...
(extralit_server/api/handlers/v1/datasets/datasets.py:63, list_current_user_datasets).
Matched with a regex instead of .includes() so a "/me/datasets" sub-route
(e.g. .../metrics) can't be mistaken for the list call.
- ProjectionRepository.getWorkspaceProjection() -> GET
/api/v1/me/datasets/projection (.../api/handlers/v1/projection.py:20,
registered ahead of datasets_v1.router specifically so this static path
isn't swallowed by /me/datasets/{dataset_id}).
Also updates the /api/v2 mention in auth-smoke.spec.ts's header comment.
…publish
Two Critical defects found by the whole-branch review that every per-task review
structurally missed (they only surface when a real search engine touches the ORM,
and the suite's mock_search_engine never does):
- Critical 1: POST /datasets/{id}/schema-versions raised MissingGreenlet because
dataset.fields was unloaded (or, with a naive eager-load fix, stale) when
create_index iterated it. Fixed with db.refresh(dataset, ["fields"]) between the
commit and the index call. Pinned by a hand-written fake SearchEngine that
actually reads dataset.fields, verified to fail with MissingGreenlet when the
refresh is reverted.
- Critical 2: republishing (a second schema version, or a first schema version on
a dataset already published via PUT /datasets/{id}/publish) always failed
because create_index is not idempotent. Added SearchEngine.index_exists (new
abstract method, shared implementation for both backends) and guard
publish_version's create_index call with it -- an explicit existence check
rather than swallowing a 400, so a genuine mapping error on first create still
surfaces. Also enforces column dtype immutability on republish per product
decision (schemas are immutable, like LanceDB), and adds the dataset.published
webhook notification publish_version was missing.
Also: excludes NULL Record.reference from the workspace projection's count/paging
queries (v1's Record.reference is nullable, unlike v2's), rewords a validator
docstring whose overclaim ("every handler already preloads it") is exactly the
kind of claim that let Critical 1 through review, adds admin/cross-workspace
policy coverage for the schema-versions endpoint, marks demo/seed_demo_workspace.py
as broken against the v1 fold with a hard failure instead of a confusing one, and
files docs/superpowers/plans/2026-07-26-fold-followups.md for what's deliberately
still deferred (ES mapping evolution, field pruning, name collisions, the demo
script repoint, and the remaining ledger minors).
Suite: 1620 passed / 3 failed (same pre-existing baseline) / 68 skipped, run with
--ignore=tests/unit/search_engine (local ES wedged at its shard cap; unaffected --
every new test uses a mock or fake engine, never a live cluster).
…ust fields Re-review of the prior fix (a1f77c0) found Critical 1 was only half-fixed: the endpoint still 500s. `_configure_index_mappings` (search_engine/commons.py) iterates FOUR relationships -- fields, metadata_properties, vectors_settings, questions -- but the previous fix only refreshed `dataset.fields`. The handler still loads dataset with only Dataset.workspace eagerly loaded, so the next relationship touched (metadata_properties) still raised MissingGreenlet. Fix: widen the post-commit refresh to db.refresh(dataset, ["fields", "metadata_properties", "vectors_settings", "questions"]). Also widened the Critical-1 pinning test: the previous fake engine only read dataset.fields, which is exactly why it passed against a still-broken endpoint. The new _RealMappingSearchEngine delegates to the REAL ElasticSearchEngine._configure_index_mappings (a pure function of the ORM object, no live cluster needed) instead of hand-picking which relationships to check, so it automatically covers any relationship that method reads today or in the future. Mutation-verified: narrowing the refresh back to just ["fields"] reproduces MissingGreenlet in this test. Also fixes a Minor the re-review found in the prior fix itself: publish_version fired the dataset.published webhook unconditionally, so every republish (version 2..n) re-fired it for an already-ready dataset -- contexts/datasets.py::publish_dataset can only fire once because it's draft-gated by DatasetPublishValidator. Capture whether the dataset was already ready before the update and skip the notify if so. Mutation-verified the same way. New tests at both the context and handler level assert a republish leaves the webhook queue with exactly one job, from the first publish. Suite: 1622 passed / 3 failed (same pre-existing baseline) / 68 skipped, run with --ignore=tests/unit/search_engine (ES still wedged; unaffected -- these tests use a fake engine backed by the real mapping-construction code, never a live cluster).
Triaged all 19 open roborev reviews on this branch (jobs 283-301, one per commit) and closed them. About half their findings were already stale -- fixed by a later commit on the same branch. This lands the six that were live, unambiguous, and introduced by the fold itself. Server: - `publish_version` now emits `DatasetEvent.updated` on a republish. Gating `published` on the draft -> ready transition (652a0b6) left versions 2..n emitting nothing at all, so a consumer subscribed to both events learned about version 1 and never heard that later versions existed. - Corrected the `index_exists` guard's comment. It said a republish-added column "is not yet queryable"; the real consequence is deterministic and worse -- `"dynamic": "strict"` plus `_map_record_fields_to_es` emitting an entry for every `dataset.fields` row means the next record write is rejected with `strict_dynamic_mapping_exception` and `PUT /datasets/{id}/records/bulk` fails outright. - `_ES_TYPE_BY_COLUMN_DTYPE` gains `"boolean"`. 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 which spelling the Pandera body used. - Unskipped `test_list_dataset_questions` (its `use_table` skip reason no longer held) and added the webhook assertions that pin the PUT /publish-then-schema-version flow, the case that distinguishes the `was_already_ready` guard from a "this is not version 1" alternative. Frontend: - Renamed the schema-slice question entity to `SchemaQuestion` / `SchemaQuestionType` / `SchemaQuestionOption`. The fold left two `export class Question` and two incompatible `QuestionType` symbols side by side under `v1/domain/entities/`, both reachable by absolute imports differing only by directory -- the exact ambiguity the `SchemaRecord` rename was made to avoid. - Fixed the e2e auth-smoke matcher, which armed its `waitForResponse` before `signIn` and so latched onto `DatasetRepository`'s param-less `/api/v1/me/datasets` call from the post-login landing page rather than the schemas page's. Dropped a dead `/references/` guard and a stale "first bearer-token client" rationale. Two High findings need a product decision and are recorded in the follow-ups doc rather than fixed: annotators losing read access to `/schemas/{id}` (the v1 record list/search routes are admin-only where the deleted `SchemaPolicy.list_records` was member-readable, and there is no `/me/` twin for the list path), and `publish_version` bypassing `DatasetPublishValidator` so a `ready` dataset with zero questions is creatable and then permanently unconfigurable. All surviving findings are carried into docs/superpowers/plans/2026-07-26-fold-followups.md sections 7-9. Verified: 52 targeted server tests pass; full server unit suite shows zero new failures against a clean-HEAD baseline (46 pre-existing failures before and after, identical sets -- local ES/env gaps and known-failing JWT tests). Frontend vitest passes for all touched files and `nuxi typecheck` reports no new errors; 7 perspective-bootstrap failures are an unmet `@perspective-dev/*` dependency, pre-existing and unrelated.
`PUT /datasets/{id}/publish` becomes 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 needs no PATCH-after
dance:
POST /datasets -> draft
POST /datasets/{id}/schema-versions -> columns materialized, still draft
POST /datasets/{id}/questions -> settings["columns"] bound inline
PUT /datasets/{id}/publish -> ready + create_index
PUT /datasets/{id}/records/bulk
Removed as consequences rather than as separate edits:
* the `was_already_ready` webhook branch -- a schema version is a dataset
mutation, so it always emits `updated`; `published` belongs to
publish_dataset alone.
* the `create_index` call and its `index_exists` guard. Under the new
ordering a schema-version publish on a draft would create the index early
and make the subsequent `PUT /publish` fail with
`resource_already_exists_exception`, so removing it was required.
* the four-relationship `db.refresh` and its comment. It existed only to
feed `create_index`; `build_dataset_event` re-selects with its own eager
loads, so the webhook never depended on it.
`_reject_dtype_changes` becomes `_reject_incompatible_columns`: one query,
one pass, three rules before any write -- annotation-field name collision
(followups sec 3), dtype immutability, and no new columns once the dataset
is `ready` (followups sec 1, option (b): the index mapping is
`"dynamic": "strict"` and nothing evolves it, so a column added
post-publish would leave the dataset unwritable at the next record write;
the 422 keeps the failure at the call that caused it).
Corollary: an annotation dataset already published via `PUT /publish`
cannot retroactively become schema-backed, since every column of a first
version is a new column. Pinned by a test rather than left latent.
Also deletes `ColumnFieldSettingsUpdate` and drops `column` from the
`FieldSettingsUpdate` union -- `PATCH /fields/{id}` could change a column's
dtype out of band, contradicting the immutability enforced at publish.
Columns are derived from the Pandera body; republish instead. No production
callers existed. Closes followups sec 9's untested dict-merge path by
removing the path.
Closes followups sec 8 (option (c)), sec 1 (option (b)), sec 3, and sec 9's
ColumnFieldSettingsUpdate item. Net -37 lines of server source while adding
two safety checks.
Verification: 47 targeted tests pass. Full server unit suite shows 118
failures before and after against a stashed clean baseline -- identical
sets, only randomized parametrize ids differ. Seed script compiles; not run
against a live stack.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughThe pull request folds extraction functionality from ChangesAPI v2 fold-back
Estimated code review effort: 5 (Critical) | ~120 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
extralit-server/src/extralit_server/api/schemas/v1/fields.py (1)
135-143: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winReject direct creation of
columnfields.
POST /datasets/{dataset_id}/fieldsacceptsColumnFieldSettingsCreate, andcreate_fieldpersists it without validation. This bypasses schema-version materialization and_reject_incompatible_columns. RemoveColumnFieldSettingsCreatefromFieldSettingsCreateor rejecttype: columnincreate_field.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@extralit-server/src/extralit_server/api/schemas/v1/fields.py` around lines 135 - 143, Prevent direct creation of column fields through the field-creation path. Remove ColumnFieldSettingsCreate from the FieldSettingsCreate discriminated union, or add an explicit type: column rejection in create_field before persistence, while preserving column materialization and _reject_incompatible_columns for internally generated columns.extralit-frontend/pages/schemas/[id]/index.vue (1)
51-56: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDisable next navigation at the authoritative total.
When the final page contains exactly
pageSizerecords, this condition enables the next button. The next request then navigates to an empty page.Use
page.totalto disable the button whencurrentOffset + page.items.length >= page.total.Proposed fix
- :disabled="page.items.length < pageSize" + :disabled="page.items.length < pageSize || currentOffset + page.items.length >= page.total"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@extralit-frontend/pages/schemas/`[id]/index.vue around lines 51 - 56, Update the next-navigation disabled condition on the BaseButton to use the authoritative page.total boundary: disable it when currentOffset + page.items.length is greater than or equal to page.total. Replace the existing full-page-length check while preserving the current goToOffset behavior.extralit-server/tests/unit/api/handlers/v1/test_list_dataset_records.py (1)
104-122: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAssert the parameterized response payload.
The
expectedmap, including thesereferencevalues, is not used. The test only asserts the status code. It also always requestsresponses, regardless ofincludes.Build the request from
includes. Then assertresponse.json() == expected.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@extralit-server/tests/unit/api/handlers/v1/test_list_dataset_records.py` around lines 104 - 122, Update the parameterized test request to pass the current includes value instead of always requesting responses, and assert the complete payload with response.json() == expected. Ensure the expected map, including each record’s reference values, is used for the assertion while preserving the existing status-code check.
🧹 Nitpick comments (16)
extralit-server/src/extralit_server/contexts/projection.py (1)
337-345: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename the DuckDB
schema_idandschema_nameidentifiers to dataset terms.These tuples now carry
dataset_idanddataset_namevalues, but they load into DuckDB columns still namedschema_idandschema_name(_INPUT_TABLES_DDL, Lines 63 and 70)._DENORMALIZE_SQLthen joins and projects those same stale names (q.schema_id = er.schema_id,q.schema_name). The behavior is correct, because the join compares dataset IDs on both sides. The naming is now misleading, because no schema entity exists in v1. A reader must trace the Python producer to learn what the column holds.Rename the DuckDB columns and their references in one pass.
♻️ Proposed rename in the DDL and the denormalization statement
CREATE TABLE questions ( - question_id VARCHAR, schema_id VARCHAR, schema_name VARCHAR, question_name VARCHAR, qtype VARCHAR + question_id VARCHAR, dataset_id VARCHAR, dataset_name VARCHAR, question_name VARCHAR, qtype VARCHAR );-CREATE TABLE records (record_id VARCHAR, schema_id VARCHAR, reference VARCHAR, inserted_at TIMESTAMP); +CREATE TABLE records (record_id VARCHAR, dataset_id VARCHAR, reference VARCHAR, inserted_at TIMESTAMP);Then update
_DENORMALIZE_SQLaccordingly:PARTITION BY reference, dataset_id,q.dataset_name,JOIN questions q ON q.dataset_id = er.dataset_id, and the twoschema_name || '.' || ...column-name expressions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@extralit-server/src/extralit_server/contexts/projection.py` around lines 337 - 345, Rename the DuckDB input-table columns from schema_id/schema_name to dataset_id/dataset_name in _INPUT_TABLES_DDL, then update every corresponding reference in _DENORMALIZE_SQL, including PARTITION BY, q.dataset_name, the dataset_id join, and both generated column-name expressions. Keep the existing values and join behavior unchanged.extralit-server/src/extralit_server/models/database.py (1)
267-270: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winDrop the redundant single-column
dataset_idindex onRecord.
Record.dataset_idstill carries its ownindex=True(line 239) in addition to the new composite indexix_records_dataset_id_referenceon(dataset_id, reference)(line 269). A btree composite index already serves any query that filters ondataset_idalone, through the leftmost-prefix rule. The standalone index adds write overhead with no additional query benefit, on a table this PR treats as a bulk-ingestion hot path.Drop the single-column index in the same migration.
♻️ Proposed change
- dataset_id: Mapped[UUID] = mapped_column(ForeignKey("datasets.id", ondelete="CASCADE"), index=True) + dataset_id: Mapped[UUID] = mapped_column(ForeignKey("datasets.id", ondelete="CASCADE"))Add a follow-up Alembic migration to drop the resulting
ix_records_dataset_idindex.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@extralit-server/src/extralit_server/models/database.py` around lines 267 - 270, Remove index=True from the Record.dataset_id column definition, then add a follow-up Alembic migration that drops the existing ix_records_dataset_id index while preserving ix_records_dataset_id_reference and the current constraints.extralit-frontend/CLAUDE.md (1)
62-65: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUpdate the seed command in
extralit-frontend/CLAUDE.md.Use:
npm run e2e:extraction:seed # uv run --project ../extralit-server python e2e/extraction/seed/seed_v2_e2e.py🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@extralit-frontend/CLAUDE.md` around lines 62 - 65, Update the e2e:extraction:seed command in the documented workflow to invoke uv with the --project ../extralit-server option before running seed_v2_e2e.py, leaving the dev and extraction test commands unchanged.extralit-frontend/components/features/schemas/RecordsTable.vue (1)
24-30: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse an interface-based props contract.
Replace the
PropTypedeclarations with a TypeScript props interface. Preserve the existingrecordsandcolumnscontract during the component migration.As per coding guidelines, Vue components must use TypeScript interfaces for component props and event contracts.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@extralit-frontend/components/features/schemas/RecordsTable.vue` around lines 24 - 30, Update the props contract in the component’s default export to use a TypeScript interface describing required records and columns, replacing the current PropType-based declarations while preserving their SchemaRecord[] and ColumnMeta[] types. Remove any now-unused PropType dependency and keep the existing required-prop behavior unchanged.Source: Coding guidelines
extralit-frontend/e2e/extraction/extractions-nav.spec.ts (1)
15-33: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a Playwright page object for tab navigation.
Move the tab selectors, tab selection, and page-title assertion into a page object. Keep this spec focused on the user workflow and expected result.
As per coding guidelines, E2E tests must use Playwright page-object patterns.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@extralit-frontend/e2e/extraction/extractions-nav.spec.ts` around lines 15 - 33, Introduce a Playwright page object for the Extractions navigation flow, moving the tab locator, tab count/order checks, Extractions tab click, URL wait, and scoped page-title assertion out of the test. Keep the test focused on signing in, opening the home page, invoking the page-object navigation/assertion methods, and verifying the expected workflow result; use descriptive methods and preserve the existing selectors and assertions.Source: Coding guidelines
extralit-frontend/v1/domain/usecases/get-schema-records-use-case.ts (1)
1-5: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueUse inline type-only imports for type-only symbols.
Update the type-only imports in both files to match the frontend convention.
verbatimModuleSyntaxis disabled, so this is not required to prevent TS1484.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@extralit-frontend/v1/domain/usecases/get-schema-records-use-case.ts` around lines 1 - 5, Use inline type-only import syntax for the type-only symbols in get-schema-records-use-case.ts and SchemaRecordRepository.ts, including RecordsPage, SchemaRecordRepository, and GetRecordsOptions where applicable; preserve value imports as regular imports.Source: Coding guidelines
extralit-frontend/e2e/extraction/search-roundtrip.spec.ts (1)
5-26: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffUse an extraction page object.
This spec directly owns navigation and element selectors. Move these operations into an extraction page object. Keep the test focused on the search workflow and assertions.
As per coding guidelines, use Playwright page-object patterns for
extralit-frontend/e2e/**/*.{ts,js}.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@extralit-frontend/e2e/extraction/search-roundtrip.spec.ts` around lines 5 - 26, Introduce or reuse an extraction page object for the navigation, schema record lookup, search input, status filter, and empty-state interactions currently handled directly in the test. Update the test to use that page object while retaining the existing seeded-record, eventual-consistency, filtered-search, and empty-result assertions so it focuses on the search workflow.Source: Coding guidelines
extralit-frontend/v1/infrastructure/repositories/SchemaRepository.ts (1)
9-21: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winRemove the stale serialization comment.
The v1
Datasetresponse schema declarescurrent_schema_version_id: UUID | None = None, so the comment stating that the field is not serialized is incorrect. ThegetSchemasfilter does not require a change for this response contract.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@extralit-frontend/v1/infrastructure/repositories/SchemaRepository.ts` around lines 9 - 21, Remove the outdated serialization comment above current_schema_version_id in the BackendDataset interface. Keep the field typing and getSchemas filtering unchanged, since the v1 response schema includes this nullable field.Source: Path instructions
extralit-server/tests/unit/conftest.py (1)
101-106: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated
api_v1.dependency_overridesregister/teardown pattern across two conftest files. Both fixtures mutate the same shared, module-level dict and independently implement "pop only the keys I registered" cleanup, with matching comments acknowledging the pattern is fragile under alternate test-collection orders.
extralit-server/tests/unit/conftest.py#L101-L106: extract a shared helper (e.g., a context manager that registers a set of overrides onapi_v1.dependency_overridesand pops exactly those keys on exit) into a common test-support module, and use it here forget_async_dbandget_search_engine.extralit-server/tests/integration/conftest.py#L32-L49: use the same shared helper forget_async_dbinstead of the parallel hand-written register/pop logic.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@extralit-server/tests/unit/conftest.py` around lines 101 - 106, Create a shared test-support context manager that registers specified overrides in api_v1.dependency_overrides and removes exactly those keys on exit; update extralit-server/tests/unit/conftest.py lines 101-106 to use it for get_async_db and get_search_engine, and update extralit-server/tests/integration/conftest.py lines 32-49 to use the same helper for get_async_db, replacing both hand-written register/pop implementations.extralit-server/tests/unit/validators/test_column_fields.py (1)
66-70: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the specific exception type.
pytest.raises(Exception)also passes on an unrelatedAttributeErrororTypeErrorraised inside the validator. The test then reports success for a real regression. AssertUnprocessableEntityError, which the sibling tests in this PR already use.♻️ Proposed refactor
+from extralit_server.errors.future import UnprocessableEntityError ... async def test_undeclared_columns_are_still_rejected(self): dataset = await self._dataset_with_column_fields() - with pytest.raises(Exception) as excinfo: + with pytest.raises(UnprocessableEntityError) as excinfo: await RecordCreateValidator.validate(RecordCreate(fields={"not_a_column": "x"}), dataset) assert "not_a_column" in str(excinfo.value)Confirm the exact exception type the validator raises before applying.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@extralit-server/tests/unit/validators/test_column_fields.py` around lines 66 - 70, Update test_undeclared_columns_are_still_rejected to assert UnprocessableEntityError instead of the broad Exception type, reusing the same exception symbol and import pattern as the sibling validator tests while preserving the existing message assertion.extralit-server/tests/unit/api/handlers/v1/datasets/test_schema_versions.py (2)
123-123: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse the imported
API_KEY_HEADER_NAMEconstant.These two headers are hard-coded as
"X-Extralit-Api-Key", while lines 153 and 167 useAPI_KEY_HEADER_NAME. Use the constant everywhere in this module for consistency.Also applies to: 395-395
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@extralit-server/tests/unit/api/handlers/v1/datasets/test_schema_versions.py` at line 123, Replace the hard-coded "X-Extralit-Api-Key" header keys in the affected requests with the imported API_KEY_HEADER_NAME constant, including both locations noted in the comment, and preserve the existing API key values.
52-62: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueClose the underlying Elasticsearch client.
ElasticSearchEngine.__init__creates anAsyncElasticsearchclient. The fake never closes it, so each test that instantiates_RealMappingSearchEngineleaves an open transport session. This produces unclosed-session warnings and leaks a resource in the test process. Add a small teardown, for example anasync def close()on the fake that awaits the client close, and call it in thefinallyblock of the test.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@extralit-server/tests/unit/api/handlers/v1/datasets/test_schema_versions.py` around lines 52 - 62, The fake _RealMappingSearchEngine must close the AsyncElasticsearch client created by ElasticSearchEngine.__init__. Add an async close method that awaits the underlying client’s close operation, then invoke it from the test’s finally block so every test instance releases its transport even when assertions or setup fail.extralit-server/tests/unit/search_engine/test_column_field_mapping.py (1)
27-40: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueIndex the mapping by its known key.
next(iter(mapping.values()))assumeses_mapping_for_fieldreturns exactly one entry. Line 44 already pins that key asfields.col. Usemapping["fields.col"]so the test fails with a clearKeyErrorif the namespace changes, instead of silently asserting on an arbitrary entry.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@extralit-server/tests/unit/search_engine/test_column_field_mapping.py` around lines 27 - 40, Update the tests around es_mapping_for_field to access the expected mapping entry explicitly via the known "fields.col" key instead of iterating over mapping.values(). Apply this to the numeric/temporal, string, and unrecognized-dtype tests, preserving their existing assertions.extralit-server/tests/unit/validators/test_suggestion_table_score.py (1)
29-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDrop the database dependency from these pure-function tests.
_validate_scorenever readsquestion_id, and it performs no I/O. Each test still awaitsRecordFactory.create()and requires thedbfixture only to obtain a UUID. The value passed is also a record id assigned to aquestion_idfield, which is misleading. Useuuid4()instead. The tests then become synchronous and need neitherdbnor@pytest.mark.asyncio.♻️ Proposed refactor
-import pytest +from uuid import uuid4 + +import pytest ... -from extralit_server.validators.suggestions import SuggestionCreateValidator -from tests.factories import RecordFactory +from extralit_server.validators.suggestions import SuggestionCreateValidator ... -@pytest.mark.asyncio class TestTableSuggestionScore: - async def test_a_scalar_score_is_allowed_for_a_multi_item_table_value(self, db): - record = await RecordFactory.create() + def test_a_scalar_score_is_allowed_for_a_multi_item_table_value(self): settings = TableQuestionSettings(type=QuestionType.table) - suggestion = _suggestion(record.id, ["row-1", "row-2"], 0.92) + suggestion = _suggestion(uuid4(), ["row-1", "row-2"], 0.92) # Must not raise: two rows, one whole-suggestion confidence score. The generic # cardinality rule would reject this pairing. SuggestionCreateValidator._validate_score(suggestion, settings)Apply the same change to the other three tests.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@extralit-server/tests/unit/validators/test_suggestion_table_score.py` around lines 29 - 60, Refactor all four tests in TestTableSuggestionScore to be synchronous: remove `@pytest.mark.asyncio`, async/await, the db fixture, and RecordFactory.create() calls. Generate a UUID with uuid4() for the _suggestion question_id argument, preserving each test’s existing validation assertions and expected errors.extralit-server/tests/unit/models/test_schema_version_model.py (1)
11-13: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMove the non-schema-version tests out of
TestSchemaVersionModel.
test_field_type_column_exists,test_record_carries_a_reference,test_record_reference_defaults_to_none, andtest_field_is_upsertabledo not testSchemaVersion. Move them to separate classes so the class name matches its coverage.test_field_type_column_existsandtest_field_is_upsertablealso declareasyncwithout awaiting anything; make them synchronous.Also applies to: 52-61
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@extralit-server/tests/unit/models/test_schema_version_model.py` around lines 11 - 13, Move test_field_type_column_exists, test_record_carries_a_reference, test_record_reference_defaults_to_none, and test_field_is_upsertable out of TestSchemaVersionModel into separate model-focused test classes whose names match the covered types. Change test_field_type_column_exists and test_field_is_upsertable from async to synchronous tests since they do not await anything.extralit-server/tests/unit/contexts/test_projection.py (1)
24-32: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptionally pass
dbexplicitly.dband the factories use the task-scopedTestSession, sodataset.current_async_sessionis the same session here. The change would improve clarity, but it is not required for correctness.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@extralit-server/tests/unit/contexts/test_projection.py` around lines 24 - 32, Optionally update schema_backed_dataset to pass the task-scoped TestSession explicitly as db when updating the dataset, rather than accessing dataset.current_async_session. Keep the existing factory and schema setup unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/superpowers/plans/2026-07-03-v2-records.md`:
- Around line 3-4: Remove the blank line separating the historical-note
blockquote from the following agentic-worker blockquote, or replace it with a
`>` line so the blockquotes remain contiguous. Apply this spacing fix at
docs/superpowers/plans/2026-07-03-v2-records.md#L3-L4,
docs/superpowers/plans/2026-06-27-schema-registry-and-versioning.md#L3-L4,
docs/superpowers/plans/2026-07-07-v2-lancedb-index.md#L3-L4,
docs/superpowers/plans/2026-07-08-v2-annotation.md#L3-L4, and
docs/superpowers/specs/2026-07-13-sdk-v2-redesign-design.md#L3-L4, preserving
each historical note.
In `@docs/superpowers/plans/2026-07-10-v2-frontend-vertical-slice.md`:
- Around line 3-4: Remove the blank line inside the historical-note blockquote
in docs/superpowers/plans/2026-07-10-v2-frontend-vertical-slice.md lines 3-4,
docs/superpowers/plans/2026-07-13-sdk-v2-vertical-slice.md lines 3-4, and
docs/superpowers/plans/2026-07-20-extraction-table.md lines 3-4, keeping each
note as a contiguous blockquote.
In `@docs/superpowers/plans/2026-07-26-fold-followups.md`:
- Around line 86-114: Update the mapping-evolution section so it consistently
states that republishing with new column names is rejected during publish with
HTTP 422 until incremental mapping support exists. Mark the earlier
strict-mapping write-failure discussion as historical, and remove or revise the
outdated current-state wording that presents option (a) as active or option (b)
as undecided.
- Around line 56-62: Update the lifecycle endpoint code fence in the documented
sequence to include an appropriate language tag, such as text, while preserving
its contents and formatting.
In `@docs/superpowers/plans/2026-07-26-fold-v2-into-v1.md`:
- Line 2758: Update the lifecycle description around the seed ordering to state
that POST /schema-versions only materializes fields and leaves the dataset
draft, PUT /publish transitions it to ready and creates the index, and PUT
/records/bulk upserts records. Make clear that publishing must occur before
records are created.
- Line 2213: The projection migration plan must address the copied _INSERTS
rather than documenting the staging SQL as unaffected: require named-column
inserts with consistent dataset_* names matching the Python projection code, or
explicitly record the v2-name and positional-binding mismatch as an unresolved
correctness defect.
In `@extralit-frontend/demo/seed_demo_workspace.py`:
- Around line 419-426: Restore the demo seed workflow in seed_demo_workspace.py
by repointing its requests from the removed /api/v2 routes to the supported
/api/v1 API contract, ensuring it can create demo-seed.json for run-demo.sh.
Alternatively, remove or disable the runnable demo path so run-demo.sh cannot
invoke this intentionally failing script.
In `@extralit-frontend/e2e/extraction/README.md`:
- Around line 8-12: Update the fenced command block in the extraction README to
declare the bash language, changing the unlabeled fence to a bash-labeled fence
while leaving the command content unchanged.
In `@extralit-frontend/v1/domain/entities/projection/grid-adapter.ts`:
- Around line 111-123: Update the documentation comment above the annotation
affordance flag to describe resolving dataset identifiers via the projection’s
datasetId instead of v2 schema IDs. Remove or revise references to the removed
v2 stack while preserving the existing explanation of the flag’s disabled state
and remaining work.
In `@extralit-server/src/extralit_server/api/schemas/v1/schema_versions.py`:
- Around line 8-14: Update SchemaVersionCreate to enforce explicit maximum sizes
for body and review_widgets at model validation time, before Pandera parsing or
storage; use the project’s established payload-limit conventions and preserve
the existing defaults and types.
In `@extralit-server/src/extralit_server/contexts/schema_versions.py`:
- Around line 62-65: Update publish_version and _next_version_number to lock the
target dataset row with a row-level database lock before calculating the next
schema version, ensuring concurrent publishes for the same dataset serialize.
Reuse the existing dataset lookup or add a locked dataset query before version
allocation, while preserving the current S3 upload and SchemaVersion insertion
flow.
In `@extralit-server/src/extralit_server/search_engine/commons.py`:
- Around line 152-168: Update _ES_TYPE_BY_COLUMN_DTYPE and the dtype lookup
logic in the surrounding search-engine mapping code to support pandas nullable
"Int64" and "Float64" as long/double, and normalize timezone-aware datetime
dtypes such as "datetime64[ns, UTC]" to the existing date_nanos mapping before
lookup. Preserve the text fallback for unknown dtypes, and add regression tests
covering nullable numeric range filtering and timezone-aware datetime sorting.
In `@extralit-server/tests/integration/test_rq_groups_workflow.py`:
- Around line 396-412: Remove the skip from test_concurrent_workflow_processing
and restore its execution. Update the test setup so each concurrently gathered
create_document_workflow call uses its own isolated AsyncSession, while
retaining production-like session behavior and avoiding sharing one AsyncSession
across tasks. Keep the concurrent workflow assertions intact.
---
Outside diff comments:
In `@extralit-frontend/pages/schemas/`[id]/index.vue:
- Around line 51-56: Update the next-navigation disabled condition on the
BaseButton to use the authoritative page.total boundary: disable it when
currentOffset + page.items.length is greater than or equal to page.total.
Replace the existing full-page-length check while preserving the current
goToOffset behavior.
In `@extralit-server/src/extralit_server/api/schemas/v1/fields.py`:
- Around line 135-143: Prevent direct creation of column fields through the
field-creation path. Remove ColumnFieldSettingsCreate from the
FieldSettingsCreate discriminated union, or add an explicit type: column
rejection in create_field before persistence, while preserving column
materialization and _reject_incompatible_columns for internally generated
columns.
In `@extralit-server/tests/unit/api/handlers/v1/test_list_dataset_records.py`:
- Around line 104-122: Update the parameterized test request to pass the current
includes value instead of always requesting responses, and assert the complete
payload with response.json() == expected. Ensure the expected map, including
each record’s reference values, is used for the assertion while preserving the
existing status-code check.
---
Nitpick comments:
In `@extralit-frontend/CLAUDE.md`:
- Around line 62-65: Update the e2e:extraction:seed command in the documented
workflow to invoke uv with the --project ../extralit-server option before
running seed_v2_e2e.py, leaving the dev and extraction test commands unchanged.
In `@extralit-frontend/components/features/schemas/RecordsTable.vue`:
- Around line 24-30: Update the props contract in the component’s default export
to use a TypeScript interface describing required records and columns, replacing
the current PropType-based declarations while preserving their SchemaRecord[]
and ColumnMeta[] types. Remove any now-unused PropType dependency and keep the
existing required-prop behavior unchanged.
In `@extralit-frontend/e2e/extraction/extractions-nav.spec.ts`:
- Around line 15-33: Introduce a Playwright page object for the Extractions
navigation flow, moving the tab locator, tab count/order checks, Extractions tab
click, URL wait, and scoped page-title assertion out of the test. Keep the test
focused on signing in, opening the home page, invoking the page-object
navigation/assertion methods, and verifying the expected workflow result; use
descriptive methods and preserve the existing selectors and assertions.
In `@extralit-frontend/e2e/extraction/search-roundtrip.spec.ts`:
- Around line 5-26: Introduce or reuse an extraction page object for the
navigation, schema record lookup, search input, status filter, and empty-state
interactions currently handled directly in the test. Update the test to use that
page object while retaining the existing seeded-record, eventual-consistency,
filtered-search, and empty-result assertions so it focuses on the search
workflow.
In `@extralit-frontend/v1/domain/usecases/get-schema-records-use-case.ts`:
- Around line 1-5: Use inline type-only import syntax for the type-only symbols
in get-schema-records-use-case.ts and SchemaRecordRepository.ts, including
RecordsPage, SchemaRecordRepository, and GetRecordsOptions where applicable;
preserve value imports as regular imports.
In `@extralit-frontend/v1/infrastructure/repositories/SchemaRepository.ts`:
- Around line 9-21: Remove the outdated serialization comment above
current_schema_version_id in the BackendDataset interface. Keep the field typing
and getSchemas filtering unchanged, since the v1 response schema includes this
nullable field.
In `@extralit-server/src/extralit_server/contexts/projection.py`:
- Around line 337-345: Rename the DuckDB input-table columns from
schema_id/schema_name to dataset_id/dataset_name in _INPUT_TABLES_DDL, then
update every corresponding reference in _DENORMALIZE_SQL, including PARTITION
BY, q.dataset_name, the dataset_id join, and both generated column-name
expressions. Keep the existing values and join behavior unchanged.
In `@extralit-server/src/extralit_server/models/database.py`:
- Around line 267-270: Remove index=True from the Record.dataset_id column
definition, then add a follow-up Alembic migration that drops the existing
ix_records_dataset_id index while preserving ix_records_dataset_id_reference and
the current constraints.
In `@extralit-server/tests/unit/api/handlers/v1/datasets/test_schema_versions.py`:
- Line 123: Replace the hard-coded "X-Extralit-Api-Key" header keys in the
affected requests with the imported API_KEY_HEADER_NAME constant, including both
locations noted in the comment, and preserve the existing API key values.
- Around line 52-62: The fake _RealMappingSearchEngine must close the
AsyncElasticsearch client created by ElasticSearchEngine.__init__. Add an async
close method that awaits the underlying client’s close operation, then invoke it
from the test’s finally block so every test instance releases its transport even
when assertions or setup fail.
In `@extralit-server/tests/unit/conftest.py`:
- Around line 101-106: Create a shared test-support context manager that
registers specified overrides in api_v1.dependency_overrides and removes exactly
those keys on exit; update extralit-server/tests/unit/conftest.py lines 101-106
to use it for get_async_db and get_search_engine, and update
extralit-server/tests/integration/conftest.py lines 32-49 to use the same helper
for get_async_db, replacing both hand-written register/pop implementations.
In `@extralit-server/tests/unit/contexts/test_projection.py`:
- Around line 24-32: Optionally update schema_backed_dataset to pass the
task-scoped TestSession explicitly as db when updating the dataset, rather than
accessing dataset.current_async_session. Keep the existing factory and schema
setup unchanged.
In `@extralit-server/tests/unit/models/test_schema_version_model.py`:
- Around line 11-13: Move test_field_type_column_exists,
test_record_carries_a_reference, test_record_reference_defaults_to_none, and
test_field_is_upsertable out of TestSchemaVersionModel into separate
model-focused test classes whose names match the covered types. Change
test_field_type_column_exists and test_field_is_upsertable from async to
synchronous tests since they do not await anything.
In `@extralit-server/tests/unit/search_engine/test_column_field_mapping.py`:
- Around line 27-40: Update the tests around es_mapping_for_field to access the
expected mapping entry explicitly via the known "fields.col" key instead of
iterating over mapping.values(). Apply this to the numeric/temporal, string, and
unrecognized-dtype tests, preserving their existing assertions.
In `@extralit-server/tests/unit/validators/test_column_fields.py`:
- Around line 66-70: Update test_undeclared_columns_are_still_rejected to assert
UnprocessableEntityError instead of the broad Exception type, reusing the same
exception symbol and import pattern as the sibling validator tests while
preserving the existing message assertion.
In `@extralit-server/tests/unit/validators/test_suggestion_table_score.py`:
- Around line 29-60: Refactor all four tests in TestTableSuggestionScore to be
synchronous: remove `@pytest.mark.asyncio`, async/await, the db fixture, and
RecordFactory.create() calls. Generate a UUID with uuid4() for the _suggestion
question_id argument, preserving each test’s existing validation assertions and
expected errors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2de41384-f320-4d4a-9ff0-49d53f3c9370
⛔ Files ignored due to path filters (1)
extralit-frontend/v2/infrastructure/api/generated/v2-api.tsis excluded by!**/generated/**
📒 Files selected for processing (232)
.github/workflows/extralit-frontend.yml.github/workflows/extralit-server.ymldocs/superpowers/plans/2026-06-27-schema-registry-and-versioning.mddocs/superpowers/plans/2026-07-03-v2-records.mddocs/superpowers/plans/2026-07-07-v2-lancedb-index.mddocs/superpowers/plans/2026-07-08-v2-annotation.mddocs/superpowers/plans/2026-07-10-v2-frontend-vertical-slice.mddocs/superpowers/plans/2026-07-13-sdk-v2-vertical-slice.mddocs/superpowers/plans/2026-07-20-extraction-table.mddocs/superpowers/plans/2026-07-26-fold-followups.mddocs/superpowers/plans/2026-07-26-fold-v2-into-v1.mddocs/superpowers/specs/2026-06-27-schema-centric-data-model-design.mddocs/superpowers/specs/2026-07-09-v2-frontend-vertical-slice-design.mddocs/superpowers/specs/2026-07-13-sdk-v2-redesign-design.mddocs/superpowers/specs/2026-07-19-reference-review.mddocs/superpowers/specs/2026-07-20-extraction-table-design.mddocs/superpowers/specs/2026-07-24-extraction-projection-acceptance.mdextralit-frontend/.gitignoreextralit-frontend/CLAUDE.mdextralit-frontend/components/features/extractions/ExtractionsGrid.client.test.tsextralit-frontend/components/features/extractions/ExtractionsGrid.client.vueextralit-frontend/components/features/schemas/RecordsTable.vueextralit-frontend/demo/README.mdextralit-frontend/demo/run-demo.shextralit-frontend/demo/seed_demo_workspace.pyextralit-frontend/e2e/extraction/README.mdextralit-frontend/e2e/extraction/auth-smoke.spec.tsextralit-frontend/e2e/extraction/extractions-grid.spec.tsextralit-frontend/e2e/extraction/extractions-nav.spec.tsextralit-frontend/e2e/extraction/fixtures.tsextralit-frontend/e2e/extraction/search-roundtrip.spec.tsextralit-frontend/e2e/extraction/seed/seed_v2_e2e.pyextralit-frontend/package.jsonextralit-frontend/pages/extractions/useExtractionsViewModel.test.tsextralit-frontend/pages/extractions/useExtractionsViewModel.tsextralit-frontend/pages/index.test.tsextralit-frontend/pages/schemas/[id]/index.vueextralit-frontend/pages/schemas/[id]/settings.vueextralit-frontend/pages/schemas/[id]/useSchemaRecordsViewModel.test.tsextralit-frontend/pages/schemas/[id]/useSchemaRecordsViewModel.tsextralit-frontend/pages/schemas/[id]/useSchemaSettingsViewModel.test.tsextralit-frontend/pages/schemas/[id]/useSchemaSettingsViewModel.tsextralit-frontend/pages/schemas/index.test.tsextralit-frontend/pages/schemas/useSchemasViewModel.tsextralit-frontend/playwright.config.tsextralit-frontend/plugins/3.di.tsextralit-frontend/v1/di/di.tsextralit-frontend/v1/domain/entities/projection/WorkspaceProjection.tsextralit-frontend/v1/domain/entities/projection/grid-adapter.test.tsextralit-frontend/v1/domain/entities/projection/grid-adapter.tsextralit-frontend/v1/domain/entities/schema/ColumnMeta.tsextralit-frontend/v1/domain/entities/schema/RecordsPage.tsextralit-frontend/v1/domain/entities/schema/Schema.tsextralit-frontend/v1/domain/entities/schema/SchemaQuestion.tsextralit-frontend/v1/domain/entities/schema/SchemaRecord.tsextralit-frontend/v1/domain/entities/schema/SchemaVersion.test.tsextralit-frontend/v1/domain/entities/schema/SchemaVersion.tsextralit-frontend/v1/domain/entities/search/SearchCriteria.test.tsextralit-frontend/v1/domain/entities/search/SearchCriteria.tsextralit-frontend/v1/domain/usecases/get-schema-records-use-case.tsextralit-frontend/v1/domain/usecases/get-schema-settings-use-case.test.tsextralit-frontend/v1/domain/usecases/get-schema-settings-use-case.tsextralit-frontend/v1/domain/usecases/get-schemas-use-case.test.tsextralit-frontend/v1/domain/usecases/get-schemas-use-case.tsextralit-frontend/v1/domain/usecases/get-workspace-projection-use-case.test.tsextralit-frontend/v1/domain/usecases/get-workspace-projection-use-case.tsextralit-frontend/v1/domain/usecases/search-records-use-case.tsextralit-frontend/v1/infrastructure/repositories/ProjectionRepository.test.tsextralit-frontend/v1/infrastructure/repositories/ProjectionRepository.tsextralit-frontend/v1/infrastructure/repositories/SchemaRecordRepository.test.tsextralit-frontend/v1/infrastructure/repositories/SchemaRecordRepository.tsextralit-frontend/v1/infrastructure/repositories/SchemaRepository.test.tsextralit-frontend/v1/infrastructure/repositories/SchemaRepository.tsextralit-frontend/v1/infrastructure/storage/ExtractionsStorage.tsextralit-frontend/v1/infrastructure/storage/SchemasStorage.tsextralit-frontend/v2/di/di.tsextralit-frontend/v2/di/index.tsextralit-frontend/v2/domain/entities/question/Question.tsextralit-frontend/v2/domain/entities/record/RecordsPage.tsextralit-frontend/v2/domain/entities/review/response-values.test.tsextralit-frontend/v2/domain/entities/review/response-values.tsextralit-frontend/v2/domain/entities/schema/SchemaVersion.test.tsextralit-frontend/v2/domain/entities/schema/SchemaVersion.tsextralit-frontend/v2/domain/entities/search/SearchCriteria.test.tsextralit-frontend/v2/domain/entities/search/SearchCriteria.tsextralit-frontend/v2/domain/usecases/discard-review-use-case.tsextralit-frontend/v2/domain/usecases/get-schema-records-use-case.tsextralit-frontend/v2/domain/usecases/get-schema-settings-use-case.tsextralit-frontend/v2/domain/usecases/rebuild-schema-index-use-case.tsextralit-frontend/v2/domain/usecases/save-review-draft-use-case.tsextralit-frontend/v2/domain/usecases/submit-reference-review-use-case.test.tsextralit-frontend/v2/domain/usecases/submit-reference-review-use-case.tsextralit-frontend/v2/infrastructure/api/openapi.jsonextralit-frontend/v2/infrastructure/repositories/AnnotationRepository.test.tsextralit-frontend/v2/infrastructure/repositories/AnnotationRepository.tsextralit-frontend/v2/infrastructure/repositories/SchemaRepository.test.tsextralit-frontend/v2/infrastructure/repositories/SchemaRepository.tsextralit-frontend/v2/infrastructure/repositories/V2RecordRepository.test.tsextralit-frontend/v2/infrastructure/repositories/V2RecordRepository.tsextralit-frontend/v2/infrastructure/repositories/apiErrors.test.tsextralit-frontend/v2/infrastructure/repositories/apiErrors.tsextralit-server/CLAUDE.mdextralit-server/src/extralit_server/_app.pyextralit-server/src/extralit_server/alembic/versions/13da2d87e660_add_schema_versions_and_record_reference.pyextralit-server/src/extralit_server/alembic/versions/6393b1a01aa0_drop_schemas_kind.pyextralit-server/src/extralit_server/alembic/versions/8136bc88ee3a_create_v2_records_table.pyextralit-server/src/extralit_server/alembic/versions/9f3010c649c8_create_schema_and_schema_version_tables.pyextralit-server/src/extralit_server/alembic/versions/c1510e93882a_create_v2_annotation_tables.pyextralit-server/src/extralit_server/api/handlers/v1/datasets/__init__.pyextralit-server/src/extralit_server/api/handlers/v1/datasets/records.pyextralit-server/src/extralit_server/api/handlers/v1/datasets/schema_versions.pyextralit-server/src/extralit_server/api/handlers/v1/projection.pyextralit-server/src/extralit_server/api/handlers/v1/questions.pyextralit-server/src/extralit_server/api/policies/v1/__init__.pyextralit-server/src/extralit_server/api/policies/v1/schema_policy.pyextralit-server/src/extralit_server/api/policies/v1/v2_annotation_policy.pyextralit-server/src/extralit_server/api/routes.pyextralit-server/src/extralit_server/api/schemas/v1/datasets.pyextralit-server/src/extralit_server/api/schemas/v1/fields.pyextralit-server/src/extralit_server/api/schemas/v1/projection.pyextralit-server/src/extralit_server/api/schemas/v1/questions.pyextralit-server/src/extralit_server/api/schemas/v1/records.pyextralit-server/src/extralit_server/api/schemas/v1/schema_versions.pyextralit-server/src/extralit_server/api/schemas/v2/__init__.pyextralit-server/src/extralit_server/api/schemas/v2/annotation.pyextralit-server/src/extralit_server/api/schemas/v2/questions.pyextralit-server/src/extralit_server/api/schemas/v2/records.pyextralit-server/src/extralit_server/api/schemas/v2/schemas.pyextralit-server/src/extralit_server/api/schemas/v2/search.pyextralit-server/src/extralit_server/api/v2/__init__.pyextralit-server/src/extralit_server/api/v2/annotation.pyextralit-server/src/extralit_server/api/v2/projection.pyextralit-server/src/extralit_server/api/v2/questions.pyextralit-server/src/extralit_server/api/v2/records.pyextralit-server/src/extralit_server/api/v2/schemas.pyextralit-server/src/extralit_server/cli/__init__.pyextralit-server/src/extralit_server/cli/index/__init__.pyextralit-server/src/extralit_server/cli/index/__main__.pyextralit-server/src/extralit_server/cli/index/reindex.pyextralit-server/src/extralit_server/cli/openapi_dump.pyextralit-server/src/extralit_server/contexts/projection.pyextralit-server/src/extralit_server/contexts/records.pyextralit-server/src/extralit_server/contexts/records_bulk.pyextralit-server/src/extralit_server/contexts/schema_versions.pyextralit-server/src/extralit_server/contexts/v2/__init__.pyextralit-server/src/extralit_server/contexts/v2/annotation.pyextralit-server/src/extralit_server/contexts/v2/index_sync.pyextralit-server/src/extralit_server/contexts/v2/records.pyextralit-server/src/extralit_server/contexts/v2/schema_bodies.pyextralit-server/src/extralit_server/contexts/v2/schemas.pyextralit-server/src/extralit_server/enums.pyextralit-server/src/extralit_server/index/__init__.pyextralit-server/src/extralit_server/index/base.pyextralit-server/src/extralit_server/index/mapping.pyextralit-server/src/extralit_server/models/__init__.pyextralit-server/src/extralit_server/models/database.pyextralit-server/src/extralit_server/models/v2/__init__.pyextralit-server/src/extralit_server/models/v2/questions.pyextralit-server/src/extralit_server/models/v2/records.pyextralit-server/src/extralit_server/models/v2/responses.pyextralit-server/src/extralit_server/models/v2/schemas.pyextralit-server/src/extralit_server/models/v2/suggestions.pyextralit-server/src/extralit_server/search_engine/base.pyextralit-server/src/extralit_server/search_engine/commons.pyextralit-server/src/extralit_server/settings.pyextralit-server/src/extralit_server/validators/questions.pyextralit-server/src/extralit_server/validators/records.pyextralit-server/src/extralit_server/validators/suggestions.pyextralit-server/src/extralit_server/validators/v2/__init__.pyextralit-server/src/extralit_server/validators/v2/questions.pyextralit-server/src/extralit_server/validators/v2/values.pyextralit-server/tests/factories.pyextralit-server/tests/integration/api/schemas/v2/__init__.pyextralit-server/tests/integration/api/schemas/v2/test_schema_models.pyextralit-server/tests/integration/api/v2/__init__.pyextralit-server/tests/integration/api/v2/test_annotation.pyextralit-server/tests/integration/api/v2/test_projection.pyextralit-server/tests/integration/api/v2/test_questions.pyextralit-server/tests/integration/api/v2/test_records.pyextralit-server/tests/integration/api/v2/test_records_search.pyextralit-server/tests/integration/api/v2/test_references.pyextralit-server/tests/integration/api/v2/test_schema_versions.pyextralit-server/tests/integration/api/v2/test_schemas.pyextralit-server/tests/integration/cli/__init__.pyextralit-server/tests/integration/cli/test_index_reindex.pyextralit-server/tests/integration/conftest.pyextralit-server/tests/integration/contexts/v2/__init__.pyextralit-server/tests/integration/contexts/v2/test_annotation_context.pyextralit-server/tests/integration/contexts/v2/test_index_sync.pyextralit-server/tests/integration/contexts/v2/test_projection.pyextralit-server/tests/integration/contexts/v2/test_records_context.pyextralit-server/tests/integration/contexts/v2/test_schema_bodies.pyextralit-server/tests/integration/contexts/v2/test_schemas_context.pyextralit-server/tests/integration/index/test_lancedb_engine.pyextralit-server/tests/integration/models/__init__.pyextralit-server/tests/integration/models/v2/__init__.pyextralit-server/tests/integration/models/v2/test_annotation_models.pyextralit-server/tests/integration/models/v2/test_record_models.pyextralit-server/tests/integration/models/v2/test_schema_factories.pyextralit-server/tests/integration/models/v2/test_schema_models.pyextralit-server/tests/integration/test_enums_v2.pyextralit-server/tests/integration/test_rq_groups_workflow.pyextralit-server/tests/unit/api/handlers/v1/datasets/questions/test_column_binding.pyextralit-server/tests/unit/api/handlers/v1/datasets/records/records_bulk/test_dataset_records_bulk.pyextralit-server/tests/unit/api/handlers/v1/datasets/test_create_dataset.pyextralit-server/tests/unit/api/handlers/v1/datasets/test_questions.pyextralit-server/tests/unit/api/handlers/v1/datasets/test_records_reference.pyextralit-server/tests/unit/api/handlers/v1/datasets/test_schema_versions.pyextralit-server/tests/unit/api/handlers/v1/datasets/test_search_current_user_dataset_records.pyextralit-server/tests/unit/api/handlers/v1/datasets/test_search_dataset_records.pyextralit-server/tests/unit/api/handlers/v1/test_datasets.pyextralit-server/tests/unit/api/handlers/v1/test_list_dataset_records.pyextralit-server/tests/unit/api/handlers/v1/test_projection.pyextralit-server/tests/unit/api/handlers/v1/test_questions.pyextralit-server/tests/unit/api/handlers/v1/test_records.pyextralit-server/tests/unit/api/schemas/v1/test_field_settings.pyextralit-server/tests/unit/api/test_api_mounts.pyextralit-server/tests/unit/api/test_not_found_routes.pyextralit-server/tests/unit/conftest.pyextralit-server/tests/unit/contexts/test_extraction_response_side_effects.pyextralit-server/tests/unit/contexts/test_projection.pyextralit-server/tests/unit/contexts/test_schema_versions.pyextralit-server/tests/unit/index/test_mapping.pyextralit-server/tests/unit/models/test_schema_version_model.pyextralit-server/tests/unit/search_engine/test_column_field_mapping.pyextralit-server/tests/unit/test_annotation_no_index_import.pyextralit-server/tests/unit/test_openapi_dump.pyextralit-server/tests/unit/validators/test_column_fields.pyextralit-server/tests/unit/validators/test_suggestion_table_score.pyextralit-server/tests/unit/validators/v2/__init__.pyextralit-server/tests/unit/validators/v2/test_question_binding.pyextralit-server/tests/unit/validators/v2/test_values.py
💤 Files with no reviewable changes (87)
- extralit-server/src/extralit_server/_app.py
- extralit-server/tests/integration/models/v2/test_schema_factories.py
- extralit-frontend/v2/domain/entities/schema/SchemaVersion.test.ts
- extralit-frontend/v2/di/index.ts
- .github/workflows/extralit-frontend.yml
- extralit-server/tests/integration/cli/test_index_reindex.py
- extralit-server/src/extralit_server/cli/index/main.py
- extralit-server/src/extralit_server/models/v2/init.py
- extralit-server/src/extralit_server/api/policies/v1/schema_policy.py
- extralit-server/src/extralit_server/api/policies/v1/init.py
- extralit-frontend/v2/domain/entities/record/RecordsPage.ts
- extralit-frontend/v2/domain/usecases/submit-reference-review-use-case.ts
- extralit-frontend/v2/infrastructure/repositories/V2RecordRepository.ts
- extralit-frontend/v2/domain/entities/schema/SchemaVersion.ts
- extralit-frontend/v2/domain/entities/review/response-values.test.ts
- .github/workflows/extralit-server.yml
- extralit-frontend/v2/infrastructure/repositories/V2RecordRepository.test.ts
- extralit-frontend/v2/infrastructure/repositories/AnnotationRepository.test.ts
- extralit-frontend/v2/domain/entities/review/response-values.ts
- extralit-frontend/v2/domain/entities/search/SearchCriteria.test.ts
- extralit-server/src/extralit_server/cli/index/init.py
- extralit-frontend/v2/di/di.ts
- extralit-server/src/extralit_server/api/v2/init.py
- extralit-server/src/extralit_server/api/schemas/v2/annotation.py
- extralit-server/src/extralit_server/api/policies/v1/v2_annotation_policy.py
- extralit-server/src/extralit_server/api/schemas/v2/search.py
- extralit-frontend/v2/domain/usecases/submit-reference-review-use-case.test.ts
- extralit-frontend/v2/infrastructure/repositories/SchemaRepository.ts
- extralit-server/tests/integration/contexts/v2/test_records_context.py
- extralit-server/src/extralit_server/models/v2/records.py
- extralit-server/src/extralit_server/models/v2/responses.py
- extralit-server/tests/integration/api/v2/test_records_search.py
- extralit-frontend/v2/domain/usecases/get-schema-settings-use-case.ts
- extralit-frontend/v2/domain/usecases/rebuild-schema-index-use-case.ts
- extralit-frontend/v2/infrastructure/repositories/SchemaRepository.test.ts
- extralit-server/tests/integration/api/v2/test_projection.py
- extralit-frontend/v2/domain/entities/search/SearchCriteria.ts
- extralit-server/src/extralit_server/models/init.py
- extralit-server/tests/integration/api/v2/test_schema_versions.py
- extralit-server/src/extralit_server/alembic/versions/6393b1a01aa0_drop_schemas_kind.py
- extralit-server/src/extralit_server/models/v2/schemas.py
- extralit-server/src/extralit_server/contexts/v2/schemas.py
- extralit-server/tests/integration/api/v2/test_records.py
- extralit-server/src/extralit_server/models/v2/questions.py
- extralit-frontend/v2/domain/usecases/discard-review-use-case.ts
- extralit-server/src/extralit_server/models/v2/suggestions.py
- extralit-server/src/extralit_server/api/v2/records.py
- extralit-server/tests/integration/api/schemas/v2/test_schema_models.py
- extralit-server/src/extralit_server/api/v2/projection.py
- extralit-server/src/extralit_server/alembic/versions/9f3010c649c8_create_schema_and_schema_version_tables.py
- extralit-server/src/extralit_server/contexts/v2/index_sync.py
- extralit-server/src/extralit_server/validators/v2/questions.py
- extralit-server/tests/integration/contexts/v2/test_projection.py
- extralit-server/src/extralit_server/contexts/v2/annotation.py
- extralit-server/tests/integration/contexts/v2/test_annotation_context.py
- extralit-server/tests/unit/validators/v2/test_question_binding.py
- extralit-server/tests/integration/api/v2/test_annotation.py
- extralit-frontend/v2/domain/entities/question/Question.ts
- extralit-server/tests/integration/models/v2/test_record_models.py
- extralit-server/src/extralit_server/api/schemas/v2/questions.py
- extralit-server/src/extralit_server/alembic/versions/c1510e93882a_create_v2_annotation_tables.py
- extralit-server/tests/integration/contexts/v2/test_index_sync.py
- extralit-server/src/extralit_server/api/v2/annotation.py
- extralit-server/src/extralit_server/contexts/v2/schema_bodies.py
- extralit-frontend/v2/infrastructure/repositories/AnnotationRepository.ts
- extralit-server/src/extralit_server/validators/v2/values.py
- extralit-server/tests/integration/models/v2/test_schema_models.py
- extralit-server/src/extralit_server/contexts/v2/records.py
- extralit-server/tests/integration/api/v2/test_references.py
- extralit-server/tests/integration/contexts/v2/test_schemas_context.py
- extralit-server/tests/unit/test_annotation_no_index_import.py
- extralit-server/src/extralit_server/api/schemas/v2/records.py
- extralit-server/src/extralit_server/alembic/versions/8136bc88ee3a_create_v2_records_table.py
- extralit-server/src/extralit_server/api/v2/questions.py
- extralit-server/src/extralit_server/api/schemas/v2/schemas.py
- extralit-server/tests/integration/models/v2/test_annotation_models.py
- extralit-server/tests/integration/api/v2/test_schemas.py
- extralit-frontend/v2/domain/usecases/get-schema-records-use-case.ts
- extralit-server/src/extralit_server/api/v2/schemas.py
- extralit-frontend/v2/infrastructure/repositories/apiErrors.ts
- extralit-server/tests/integration/test_enums_v2.py
- extralit-frontend/v2/domain/usecases/save-review-draft-use-case.ts
- extralit-server/tests/integration/contexts/v2/test_schema_bodies.py
- extralit-server/src/extralit_server/cli/index/reindex.py
- extralit-frontend/v2/infrastructure/repositories/apiErrors.test.ts
- extralit-server/tests/unit/validators/v2/test_values.py
- extralit-server/tests/integration/api/v2/test_questions.py
Comment on lines
+419
to
+426
| print( | ||
| "seed_demo_workspace.py is broken: every request in this script targets /api/v2/*\n" | ||
| "routes deleted by the v1 fold (feat/ENG-36-server-v2-to-v1). It has not been\n" | ||
| "repointed at /api/v1 -- see 'Repointing demo/seed_demo_workspace.py' in\n" | ||
| "docs/superpowers/plans/2026-07-26-fold-followups.md before using or fixing this file.", | ||
| file=sys.stderr, | ||
| ) | ||
| raise SystemExit(1) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Restore the demo seed before keeping the demo runner enabled.
run-demo.sh invokes this file before recording. This unconditional exit prevents demo-seed.json creation, so the documented demo command always fails. Repoint the seed workflow to the v1 API contract, or remove the runnable demo path until that work is complete.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@extralit-frontend/demo/seed_demo_workspace.py` around lines 419 - 426,
Restore the demo seed workflow in seed_demo_workspace.py by repointing its
requests from the removed /api/v2 routes to the supported /api/v1 API contract,
ensuring it can create demo-seed.json for run-demo.sh. Alternatively, remove or
disable the runnable demo path so run-demo.sh cannot invoke this intentionally
failing script.
Comment on lines
+8
to
+14
| class SchemaVersionCreate(BaseModel): | ||
| """A new schema version. `body` is a Pandera `DataFrameSchema.to_json()` payload.""" | ||
|
|
||
| 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) |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the schema-version context and handler for size limits and parse error handling.
fd -t f 'schema_versions.py' extralit-server/src --exec ast-grep outline {} --items all
fd -t f 'schema_versions.py' extralit-server/src --exec rg -n -C4 'max_length|len\(|from_json|DataFrameSchema|UnprocessableEntity|try:' {}Repository: Extralit/extralit
Length of output: 5134
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- handler ---'
sed -n '1,125p' extralit-server/src/extralit_server/api/handlers/v1/datasets/schema_versions.py
printf '%s\n' '--- context ---'
sed -n '1,215p' extralit-server/src/extralit_server/contexts/schema_versions.py
printf '%s\n' '--- file context ---'
fd -t f . extralit-server/src/extralit_server/contexts --exec rg -n -C3 'put|upload|object_key|body|ContentLength|content_length' {}
printf '%s\n' '--- request-size controls ---'
rg -n -C3 'max.?body|max.?size|body.?limit|Content-Length|content_length|RequestSize|LimitUpload|multipart' extralit-server/src extralit-server 2>/dev/null | head -250Repository: Extralit/extralit
Length of output: 31990
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- application and middleware ---'
fd -t f . extralit-server/src/extralit_server --exec rg -n -C4 'FastAPI\(|add_middleware|middleware|receive|max_body|body_limit|limit_concurrency|Content-Length|content-length|client_max_body_size' {}
printf '%s\n' '--- deployment/configuration ---'
rg -n -C3 'client_max_body_size|max_body|max.?request|body.?size|413|request.?limit|uvicorn|gunicorn|traefik|nginx|proxy' . --glob '!**/.git/**' --glob '!**/node_modules/**' | head -300
printf '%s\n' '--- static call-order verifier ---'
uv run python - <<'PY'
from pathlib import Path
context = Path("extralit-server/src/extralit_server/contexts/schema_versions.py").read_text()
parse = context.index("field_payloads = derive_column_fields(body, review_widgets)")
write = context.index("metadata = await files_ctx.put_object", parse)
assert "pa.DataFrameSchema.from_json(body_json)" in context
assert parse < write
assert "max_length" not in context
assert "len(body)" not in context
print("parse_before_storage_write=True")
print("context_body_size_bound=False")
PYRepository: Extralit/extralit
Length of output: 5696
Add schema-version payload limits.
body and review_widgets are unbounded. Add explicit limits before Pandera parsing and storage.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@extralit-server/src/extralit_server/api/schemas/v1/schema_versions.py` around
lines 8 - 14, Update SchemaVersionCreate to enforce explicit maximum sizes for
body and review_widgets at model validation time, before Pandera parsing or
storage; use the project’s established payload-limit conventions and preserve
the existing defaults and types.
Comment on lines
+396
to
+412
| @pytest.mark.skip( | ||
| reason="Root cause: create_document_workflow() opens its own AsyncSessionLocal() " | ||
| "connection instead of accepting an injected session - see ENG-37 (test-session " | ||
| "architecture: workflows open their own AsyncSessionLocal). Routing AsyncSessionLocal() " | ||
| "onto the fixture's shared session (as done for test_create_document_workflow_with_rq_groups " | ||
| "above) does NOT fix this test specifically: it runs three create_document_workflow() calls " | ||
| "concurrently via asyncio.gather, and a single AsyncSession cannot be used by overlapping " | ||
| "coroutines - confirmed empirically, raises sqlalchemy.exc.IllegalStateChangeError " | ||
| "('bind() is already in progress'). Fixing this one needs either per-task sessions that " | ||
| "still serialize onto one connection, or the ENG-37 production seam; out of scope here." | ||
| ) | ||
| async def test_concurrent_workflow_processing( | ||
| self, async_db, test_workspace, mock_redis_connection, mock_rq_queues | ||
| self, | ||
| db, | ||
| test_workspace, | ||
| mock_redis_connection, | ||
| mock_rq_queues, |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Restore concurrent workflow coverage.
Line 396 skips the test that creates workflows concurrently. The suite can no longer detect regressions in concurrent create_document_workflow() calls.
Use isolated sessions per concurrent task in a production-like test setup. Keep this test enabled before release.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@extralit-server/tests/integration/test_rq_groups_workflow.py` around lines
396 - 412, Remove the skip from test_concurrent_workflow_processing and restore
its execution. Update the test setup so each concurrently gathered
create_document_workflow call uses its own isolated AsyncSession, while
retaining production-like session behavior and avoiding sharing one AsyncSession
across tasks. Keep the concurrent workflow assertions intact.
Contributor
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
🛑 Comments failed to post (3)
docs/superpowers/plans/2026-07-26-fold-v2-into-v1.md (2)
2213-2213: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Do not document the projection staging SQL as unaffected.
The deferred-followups document records that the copied
_INSERTSstill use v2 names and positional binding. If the Python side usesdataset_*names while these inserts remain unchanged, values can land in the wrong DuckDB columns and produce incorrect grid column names.Require named inserts and consistent dataset names, or mark this as an explicit unresolved correctness defect.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/superpowers/plans/2026-07-26-fold-v2-into-v1.md` at line 2213, The projection migration plan must address the copied _INSERTS rather than documenting the staging SQL as unaffected: require named-column inserts with consistent dataset_* names matching the Python projection code, or explicitly record the v2-name and positional-binding mismatch as an unresolved correctness defect.
2758-2758: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Correct the schema-version publishing lifecycle.
This line says that
POST /datasets/{id}/schema-versionsmakes the dataset ready and creates the index. The final lifecycle is:
POST /schema-versions→ materialize fields; dataset remains draft.PUT /publish→ ready state and index creation.PUT /records/bulk→ records.Update this text so callers do not omit the required publish step.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/superpowers/plans/2026-07-26-fold-v2-into-v1.md` at line 2758, Update the lifecycle description around the seed ordering to state that POST /schema-versions only materializes fields and leaves the dataset draft, PUT /publish transitions it to ready and creates the index, and PUT /records/bulk upserts records. Make clear that publishing must occur before records are created.extralit-frontend/v1/domain/entities/projection/grid-adapter.ts (1)
111-123: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Update the annotation-link documentation to use dataset identifiers.
The comment says annotation mode must resolve v2 schema IDs. The projection now supplies
datasetId, and the v2 stack is removed. Update the remaining-work description before this flag is enabled.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@extralit-frontend/v1/domain/entities/projection/grid-adapter.ts` around lines 111 - 123, Update the documentation comment above the annotation affordance flag to describe resolving dataset identifiers via the projection’s datasetId instead of v2 schema IDs. Remove or revise references to the removed v2 stack while preserving the existing explanation of the flag’s disabled state and remaining work.Source: Coding guidelines
JonnyTran
changed the title
Five of the ten findings were real. Two were not; see the bottom.
**Serialize schema-version publishing (Critical).** `_next_version_number` now
takes a `SELECT ... FOR UPDATE` on the dataset row before allocating. The
follow-ups doc had recorded this race as harmless -- a loser that rolls back
leaving "an orphaned S3 object" -- and that was wrong. `object_key_for` derives
the key from the version number, so two publishers reading the same max also
`put_object` to the SAME key: the second write 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 committed. The committed row then
points at content that does not match its checksum, breaking the immutability
the model exists to provide. The unique constraint fires at `db.flush()`, long
after the object was overwritten. Silent corruption of a committed row outranks
a stray object, so this is fixed rather than deferred. SQLite emits no FOR
UPDATE clause (single-writer already serializes), so it is a no-op there.
**Map pandas extension dtypes to typed ES fields.** A column declared
`pd.Int64Dtype()` -- the ordinary way to get a nullable integer column --
reports "Int64", and that spelling survives to_json/from_json into
`Field.settings["dtype"]`. `_ES_TYPE_BY_COLUMN_DTYPE` only had the lowercase
numpy spellings, so every such column fell through to the text fallback and lost
numeric range queries and numeric sort order. Timezone-aware datetimes spell
their zone into the dtype ("datetime64[ns, UTC]") and missed the lookup once per
zone. New `normalize_column_dtype` case-folds and strips the zone before lookup,
which needs no new map entries. This is the same class of bug the earlier
roborev pass fixed for "bool"/"boolean", fixed generally this time.
Note: `nullable=True` alone does NOT change the spelling -- an earlier comment
in this branch implying it does was wrong. The trigger is the extension-dtype
declaration. Both halves are now pinned by tests so they cannot drift.
**Bound the schema-version body** at 1 MiB. It is parsed in-process and then
uploaded whole, so one request sizes both the parse and the object write.
Generous by design (a 500-column schema serializes to well under 100 KiB); a
whole-request limit belongs at the ASGI layer, which does not exist here yet.
**Fail run-demo.sh closed up front.** `seed_demo_workspace.py` still targets the
deleted /api/v2 and exits 1 unconditionally, so the pipeline could never
succeed -- but it first created (and on a rerun deleted) its output directory
before finding out. Now refuses at the top, with the README saying so.
Repointing the seed stays deferred.
**Fix the fold-followups mapping-evolution section.** It stated the 422 contract
in one place and the superseded "option (a), decision pending" analysis in
another, and still described the `index_exists` guard removed in 1db3acc. One
contract now, with the argument for it kept as explicitly historical. Section 5
rewritten as resolved with the corrected damage analysis above.
Markdown lint: MD028 blank-line-in-blockquote across 7 historical-note docs
(the 8th was a false positive: its next line is not a blockquote), MD040 on two
fences. Note this repo runs no markdownlint -- no config, no hook, no CI job.
**Declined.** (1) "Restore concurrent workflow coverage": the premise is wrong.
`test_concurrent_workflow_processing` was not skipped by this branch to hide a
regression -- on develop the whole class errored at fixture setup (it referenced
a nonexistent `async_db` fixture and built Workspace() with removed kwargs), so
it never provided coverage. This branch fixed the fixtures, made one of the two
pass for real, and skipped the other with an accurate root cause. The suggested
fix (per-task sessions) is precisely what the skip reason documents as
insufficient without the ENG-37 seam. (2) review_widgets was not separately
bounded: an item-count cap does not bound bytes and would only look like a
limit.
Verification: 106 targeted tests pass. Same 115 failures before and after across
contexts/search_engine/dataset-handlers/schemas, measured against a stashed
clean baseline -- identical sets, only randomized parametrize ids differ.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/superpowers/plans/2026-07-26-fold-followups.md`:
- Around line 208-213: Update the `_next_version_number` resolution note to
avoid claiming SQLite allocation is serialized: either implement and test an
explicit SQLite transaction strategy that guarantees concurrent version
allocation, or revise the documentation to state that the serialization
guarantee applies only to PostgreSQL and SQLite may encounter locking conflicts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1976eff6-7b38-4a1b-b5a3-ea99bc74cedf
📒 Files selected for processing (17)
docs/superpowers/plans/2026-06-27-schema-registry-and-versioning.mddocs/superpowers/plans/2026-07-03-v2-records.mddocs/superpowers/plans/2026-07-07-v2-lancedb-index.mddocs/superpowers/plans/2026-07-08-v2-annotation.mddocs/superpowers/plans/2026-07-10-v2-frontend-vertical-slice.mddocs/superpowers/plans/2026-07-13-sdk-v2-vertical-slice.mddocs/superpowers/plans/2026-07-20-extraction-table.mddocs/superpowers/plans/2026-07-26-fold-followups.mdextralit-frontend/demo/README.mdextralit-frontend/demo/run-demo.shextralit-frontend/e2e/extraction/README.mdextralit-server/src/extralit_server/api/schemas/v1/schema_versions.pyextralit-server/src/extralit_server/contexts/schema_versions.pyextralit-server/src/extralit_server/search_engine/commons.pyextralit-server/tests/unit/api/handlers/v1/datasets/test_schema_versions.pyextralit-server/tests/unit/contexts/test_schema_versions.pyextralit-server/tests/unit/search_engine/test_column_field_mapping.py
🚧 Files skipped from review as they are similar to previous changes (11)
- docs/superpowers/plans/2026-07-13-sdk-v2-vertical-slice.md
- docs/superpowers/plans/2026-06-27-schema-registry-and-versioning.md
- docs/superpowers/plans/2026-07-08-v2-annotation.md
- docs/superpowers/plans/2026-07-10-v2-frontend-vertical-slice.md
- docs/superpowers/plans/2026-07-20-extraction-table.md
- extralit-frontend/e2e/extraction/README.md
- extralit-server/tests/unit/search_engine/test_column_field_mapping.py
- docs/superpowers/plans/2026-07-07-v2-lancedb-index.md
- extralit-server/tests/unit/contexts/test_schema_versions.py
- extralit-server/tests/unit/api/handlers/v1/datasets/test_schema_versions.py
- docs/superpowers/plans/2026-07-03-v2-records.md
Comment on lines
+208
to
+213
| ## 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. |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
schema_versions="extralit-server/src/extralit_server/contexts/schema_versions.py"
ast-grep outline "$schema_versions" --items all
rg -n -C 12 \
'_next_version_number|with_for_update|object_key_for|put_object|flush|commit' \
"$schema_versions"
rg -n -C 8 \
'sqlite|postgresql|create_engine|BEGIN IMMEDIATE|transaction' \
extralit-server/src/extralit_server extralit-server/testsRepository: Extralit/extralit
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Relevant implementation and tests =="
sed -n '60,190p' extralit-server/src/extralit_server/contexts/schema_versions.py
rg -n -C 8 'publish_version|_next_version_number|schema_versions|concurr|SQLite|sqlite' \
extralit-server/tests extralit-server/README.md README.md docs 2>/dev/null | head -n 240
echo "== Database defaults and development requirements =="
rg -n -C 6 'PostgreSQL|required development|SQLite|sqlite' \
pyproject.toml extralit-server/pyproject.toml README.md extralit-server/README.md \
docs .github 2>/dev/null | head -n 240
echo "== Deterministic SQLite lock probe =="
python3 - <<'PY'
import os
import sqlite3
import tempfile
fd, path = tempfile.mkstemp(suffix=".db")
os.close(fd)
try:
init = sqlite3.connect(path)
init.executescript("""
CREATE TABLE schema_versions(dataset_id TEXT, version INTEGER);
INSERT INTO schema_versions VALUES ('d', 1);
""")
init.commit()
init.close()
a = sqlite3.connect(path, timeout=0)
b = sqlite3.connect(path, timeout=0)
a.execute("BEGIN DEFERRED")
b.execute("BEGIN DEFERRED")
next_a = (a.execute(
"SELECT COALESCE(MAX(version), 0) + 1 FROM schema_versions WHERE dataset_id='d'"
).fetchone()[0])
next_b = (b.execute(
"SELECT COALESCE(MAX(version), 0) + 1 FROM schema_versions WHERE dataset_id='d'"
).fetchone()[0])
print(f"both deferred reads returned: {next_a}, {next_b}")
print("external object writes would both target:", f"schemas/d/v{next_a}.json")
a.execute("INSERT INTO schema_versions VALUES ('d', ?)", (next_a,))
try:
b.execute("INSERT INTO schema_versions VALUES ('d', ?)", (next_b,))
except sqlite3.OperationalError as exc:
print("second database write:", str(exc))
a.rollback()
b.rollback()
a.close()
b.close()
finally:
os.unlink(path)
PYRepository: Extralit/extralit
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== SQLite deferred-transaction behavior =="
python3 - <<'PY'
import os
import sqlite3
import tempfile
fd, path = tempfile.mkstemp(suffix=".db")
os.close(fd)
try:
conn = sqlite3.connect(path)
conn.executescript("""
CREATE TABLE schema_versions(dataset_id TEXT, version INTEGER);
INSERT INTO schema_versions VALUES ('d', 1);
""")
conn.commit()
conn.close()
first = sqlite3.connect(path, timeout=0)
second = sqlite3.connect(path, timeout=0)
first.execute("BEGIN DEFERRED")
second.execute("BEGIN DEFERRED")
def next_version(conn):
return conn.execute(
"SELECT COALESCE(MAX(version), 0) + 1 "
"FROM schema_versions WHERE dataset_id = 'd'"
).fetchone()[0]
first_version = next_version(first)
second_version = next_version(second)
print("deferred reads:", first_version, second_version)
print("object keys:", f"schemas/d/v{first_version}.json", f"schemas/d/v{second_version}.json")
first.execute("INSERT INTO schema_versions VALUES ('d', ?)", (first_version,))
try:
second.execute("INSERT INTO schema_versions VALUES ('d', ?)", (second_version,))
except sqlite3.OperationalError as exc:
print("second write:", type(exc).__name__, str(exc))
first.rollback()
second.rollback()
first.close()
second.close()
finally:
os.unlink(path)
PY
echo "== Support and concurrency references =="
rg -n -C 4 \
'database_url|sqlite\+aiosqlite|PostgreSQL.*default|PostgreSQL.*Main|runs.*SQLite|same-schema|Concurrent same-schema|concurrent.*publish|max\(version\)' \
README.md extralit-server/README.md .github/copilot-instructions.md \
extralit-server/pyproject.toml docs/superpowers/specs/2026-06-27-schema-centric-data-model-design.md \
docs/superpowers/plans/2026-07-26-fold-followups.md
echo "== Schema-version test files and concurrency tests =="
git ls-files extralit-server/tests | rg 'schema|version|concurr'
rg -n -C 5 \
'publish_version|_next_version_number|object_key_for|same-schema|concurr|database is locked' \
extralit-server/tests --glob '*.py' | head -n 220Repository: Extralit/extralit
Length of output: 33797
Do not mark SQLite allocation as serialized. SQLite ignores with_for_update(). Two deferred transactions can read the same version, write the same object key, and then fail one database write with database is locked. SQLite is supported by default and used by tests. Add an explicit SQLite transaction strategy and concurrency test, or document that the guarantee applies only to PostgreSQL.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/plans/2026-07-26-fold-followups.md` around lines 208 - 213,
Update the `_next_version_number` resolution note to avoid claiming SQLite
allocation is serialized: either implement and test an explicit SQLite
transaction strategy that guarantees concurrent version allocation, or revise
the documentation to state that the serialization guarantee applies only to
PostgreSQL and SQLite may encounter locking conflicts.
Source: Coding guidelines
JonnyTran
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Deletes the parallel
*/v2/tree and runs the schema-based extraction model on top of the v1 annotation platform. One data model, one policy tree, one context layer, one frontend DDD tree.233 files, +5,939 / −18,243, 27 commits. That ratio is the point of the change.
Why
Five PRs (#219, #227, #228, #229, #233) built a schema-centric extraction stack as a self-contained
/api/v2: 6 tables, 26 endpoints, a second policy tree, a second validators tree, a second frontend DDD tree. Most of it restated v1.V2ResponseandV2Suggestionwere column-for-column identical to v1's.SchemaandDatasetwere the same concept: a workspace-scoped, uniquely-named resource with a draft/published lifecycle that owns questions and records.SchemaPolicyreproducedDatasetPolicypredicate for predicate.v2_annotation_policy.pysaid so in a comment.QuestionType.table,FieldType.table,TableQuestionSettings,TableQuestionResponseValueValidator.Of 26 v2 endpoints, 8 were live, 4 were orphaned by #234, and 14 were reachable only from tests and a seed script. After the fold, 22 map onto v1 endpoints that already exist. Four new v1 endpoints carry the rest.
The idea that makes it collapse
An extraction project is a v1
Dataset. Everything follows:schemasfolds intodatasets,v2_records/v2_questions/v2_responses/v2_suggestionsfold into their v1 counterparts,SchemaPolicybecomesDatasetPolicy.One capability had no v1 home: the versioned Pandera body in object storage. It survives as
schema_versionsre-FK'd todatasets, withDataset.current_schema_version_idas head pointer.SchemaVersionholds a pointer plus integrity metadata (object key, etag, checksum,parent_version_idlineage), not a cached column list.Pandera columns become
Fieldrows under a newFieldType.column, indexed for search but deliberately never value-validated (a column is an ingestion input, not an annotator answer). Thefieldstable becomes the column manifest, served by the existingGET /datasets/{id}/fields. That removedcolumns_cacheand, initially,review_widgets.How the specs and plans got here
The v2 stack grew over four weeks of design docs under
docs/superpowers/, each spec followed by a plan:schema-centric-data-model-designschema-registry-and-versioningv2-recordsv2-lancedb-indexv2-annotationv2-frontend-vertical-slice-designv2-frontend-vertical-slicesdk-v2-redesign-designsdk-v2-vertical-slicereference-reviewextraction-table-designextraction-tableextraction-projection-acceptancefold-v2-into-v1(this PR)By 07-26 the duplication was clear enough to plan against, so the fold plan replaced the next feature plan. The 13 v2-era docs stay in the tree with a historical-note banner; their spec section numbers are still cited from code, so deleting them would orphan those references.
The plan changed shape once, before any code.
3ffe70561reordered it.models/v2/schemas.pybinds__tablename__ = "schema_versions"on the sameDatabaseModelmetadata as every v1 model, so declaring the newSchemaVersionwhile the old one exists raisesInvalidRequestErrorat import time. Tasks 9 to 11 became Tasks 1 to 3, and the fold runs delete-first. Task 1 snapshots the v2 tree to a git-ignored directory and tagsv2-pre-fold, because every later task references sources that no longer exist by the time it runs.What changed, by area
Server model.
schema_versionsre-pointed atdatasets;Dataset.current_schema_version_id;Record.reference(plain indexed nullable string mirroringDocument.reference, un-FK'd because a reference may have nodocumentsrow yet);FieldType.columnwithField.is_column. One migration,13da2d87e660.Four new endpoints.
POST/GET/GETunder/datasets/{id}/schema-versions, plusGET /me/datasets/projection.Question bindings. A question binds to schema columns through
Question.settings["columns"], validated against the dataset's declared columns. Table questions bind to many, everything else to exactly one.Projection. The
/extractionsgrid backend moved onto v1 tables. Postgres pages references rather than rows, so a stacked table question never splits across a page boundary; DuckDB denormalizes.Frontend.
v2/is gone as a directory and as a concept. Twots-injectyDI containers merged into one (74 registrations, collision-free),V2RecordbecameSchemaRecord, and the generated OpenAPI client gave way to hand-written response interfaces since no v2 spec remains to generate from.The simplification pass (2026-08-02)
Review of the finished branch found
publish_versiondoing too much.1db3acc57cut it back: publishing a schema version no longer flipsdataset.status.PUT /datasets/{id}/publishis now the sole draft-to-ready transition and the solecreate_indexcaller.The lifecycle is now the obvious one, with no PATCH-after dance:
Three things went away as consequences rather than as separate edits: the
was_already_readywebhook branch, thecreate_indexcall with itsindex_existsguard, and a four-relationshipdb.refreshthat existed only to feed that call. Net 37 fewer lines of server source while adding two safety checks._reject_dtype_changesbecame_reject_incompatible_columns: one query, one pass, three rules before any write (annotation-field name collision, dtype immutability, no new columns onceready).ColumnFieldSettingsUpdatewas deleted, becausePATCH /fields/{id}could otherwise change a column's dtype out of band and contradict the invariant enforced at publish.Verification
nuxi typecheckreports no new errors. Sevenperspective-bootstrapfailures come from an unmet@perspective-dev/*dependency, pre-existing and unrelated.Not verified: the e2e suite has never run against a live stack. See below.
Remaining work
Tracked in
docs/superpowers/plans/2026-07-26-fold-followups.md.High
1. Run the e2e suite against a live stack. This is the merge gate. Every frontend repository test is an axios mock asserting a URL the test itself supplies, so it cannot catch a contract mismatch. Two bugs of exactly that shape already surfaced through reading rather than testing: the
current_schema_version_idfilter that was silently a no-op untilc5f60da7c, and item 2 below. The seed's ordering changed in1db3acc57, which makes this run more load-bearing, not less.2. Annotators lost read access to
/schemas/{id}(followups §7).SchemaRecordRepositorycalls two v1 routes authorizedis_owner or (is_admin and is_member), where the deletedSchemaPolicy.list_recordswasis_owner or is_member. Annotators now get 403 on both the list and the search call, and the page renders empty. A/me/twin exists for search but not for list, so repointing search alone does not restore them. Cleanest fix is a/me/datasets/{id}/recordslist twin. Needs a product call plus a policy-level test, not another axios mock.3. The migration rewrites history.
13da2d87e660deletes four revisions live ondevelop(9f3010c649c8,8136bc88ee3a,6393b1a01aa0,c1510e93882a). Any database migrated fromdeveloppoints at a revision that no longer exists, and bothupgradeanddowngradefail. Extralit is pre-production, so this is defensible, and it is the only change here that cannot be fixed forward after merge. The recovery instructions in the docstring have known errors: they are dialect-wrong (the collision hits SQLite too) and internally out of order (schema_versionscannot be dropped "before upgrading" while five tables still FK into it). Fix or cut them before merge.Medium
4. A published dataset cannot gain columns (followups §1). The index mapping is built
"dynamic": "strict"at publish and nothing evolves it, so a column added afterwards would leave the dataset unwritable at the next record write.1db3acc57converts that into a 422 at publish time, which localizes the failure but narrows the contract: republish may no longer add columns onceready, and an annotation dataset already published cannot retroactively become schema-backed. Lifting the restriction needsput_mappingon republish. Check first whether ENG-36 (LanceDB as aSearchEngine) supersedes it, since Arrow schema evolution may make the work moot.5. A dropped column leaves its
Fieldrow behind (followups §2).derive_column_fieldsplusField.upsert_manyonly ever add or update. This is a real regression from the fold, not a pre-existing v1 gap: v2'scolumns_cachereplaced the column list wholesale on every publish, so there was nothing to prune. Deciding it needs an answer on what happens to aQuestion.settings["columns"]that binds to a dropped name.6.
contexts/projection.pyis the highest-risk-per-line file here. The DuckDB staging layer still speaks v2 vocabulary (schema_id,schema_name) while the Python half was renamed todataset_*, and_INSERTSbinds positionally into all-VARCHARcolumns. Reorder either side and the grid shows wrong column names instead of raising. Rename and switch to named binds. Separately worth asking whether the fan-out and stacking semantics earn their ~130-line CTE chain before a second table question exists in the wild.7.
list_schema_versionsreturns a bare array where every other v1 collection returns{items: [...]}. Free to change now, breaking once anyone paginates it.8.
demo/seed_demo_workspace.pystill targets the deleted/api/v2(followups §4). It fails fast with a clear message rather than 404ing obscurely, and it gates no CI, so it can be scheduled independently. All 13 of its raw HTTP calls need v1 shapes.9. The
batch_alter_tablein the migration is dialect-asymmetric. It renames the pre-existingworkspace_idFK on SQLite but not on Postgres, so the two dialects end up with different constraint names. The deleted9f3010c649c8used a dialect guard and carried no DB-level FK on SQLite, which is simpler and worth reconsidering.Lower-severity items (the
_next_version_numberrace, roughly 30 carried roborev findings, and thee2e_v2_*names that still render in seeded UI) stay in the follow-ups doc.Reviewing this
Read the migration first. It is the only irreversible piece.
Then
contexts/schema_versions.py,models/database.py, and the schema-version handlers, in that order. They set the vocabulary the rest of the PR speaks.The frontend needs one caution:
b178eab58repointed the data layer while files were still underv2/, and538a0e03cmoved them. Reviewing those commits in sequence shows paths that no longer exist.Summary by CodeRabbit
New Features
Improvements
Documentation