Skip to content

Tracker: validate_table_schema decomposition (#47-#52) - #53

Closed
msto wants to merge 2 commits into
mainfrom
meta/validate-table-schema-tracker
Closed

msto wants to merge 2 commits into
mainfrom
meta/validate-table-schema-tracker

Conversation

@msto

@msto msto commented May 21, 2026 •

Copy link
Copy Markdown
Collaborator

Do not merge. This PR is a tracker for the validate_table_schema stack; close it when every child PR has merged.

Related to #42.

Stack (in stack order)

The original three combined PRs (#48 primitives, #50 enum+blob, #51 array+link) were each split into two so every child PR is ≤400 lines. The full per-PR review-fix checklist lives in agent_notes/2026-05-21_validate-table-schema-tracker.md.

Cross-cutting refactors (applied across the stack)

  • Drop Any for TypeAnnotation (imported from fgmetric._typing_extensions).
  • Reuse fgmetric type introspection: is_optional / unpack_optional / is_list. Adds fgmetric>=0.3,<1 as a runtime dep.
  • Rename expected / actual on SchemaMismatch → two pairs of optional fields, model_field / column_name and model_type / column_type, populated by kind.
  • SchemaMismatchKind is a StrEnum; SchemaMismatch.message is a Pydantic @computed_field dispatched on kind. Callers never construct messages.
  • Comparators return SchemaMismatch | None (was list[SchemaMismatch]); only the top-level _validate_table_schema returns a list (batch report).
  • Enum comparison fixed: Registry .name ↔ Model .value (model values carry the user-assigned strings; the previous .name-on-both-sides comparison only worked because the test fixture used identifier=value pairs).
  • Structural check _looks_like_latch_record_model replaces the lazy-import-from-_record_model dance.
  • _validate_table_schema defaults missing allowEmpty to False (defensive — the SDK TypedDict requires it but a malformed payload shouldn't crash validation).

Per-PR review fixes

Highlights of what changed in this round of revisions, by PR:

Test plan

  • When each child PR merges, tick its checkbox above and in the tracker doc.
  • When the stack lands, close (don't merge) this PR.

🤖 Generated with Claude Code

Adds a tracker doc for the #47–#52 stack: per-PR scope, child PR list
with checkboxes, cross-cutting refactors (drop `Any`, reuse `fgmetric`
typing extensions, rename expected/actual), and per-PR review fixes.

Not intended to merge — the PR opened against this doc is a draft
tracker that should be closed once every child PR lands.

Related to #42.
@msto msto self-assigned this May 21, 2026
- fgmetric runtime dep approved; private `_typing_extensions` import
  acceptable, TODO to switch when promoted to public API.
- `SchemaMismatch` gets two pairs of optional fields (model_field /
  column_name, model_type / column_type) populated by kind.
- Enum comparison: Registry .name ↔ Model .value. Test fixture's
  identifier=value coincidence masked the bug.
msto added a commit that referenced this pull request May 21, 2026
Adds the types that the rest of the schema-validation work hangs off,
revised per #47 review feedback.

- `SchemaMismatchKind`: a `StrEnum` (was a `Literal`); wire-stable
  string values. Also exposed as `SchemaMismatch.Kind` for
  discoverability.
- `SchemaMismatch`: frozen Pydantic model. Replaces the ambiguous
  `expected` / `actual` fields with two optional pairs — `model_field`
  / `column_name` and `model_type` / `column_type` — explicit about
  which side each value comes from. Which slots are populated depends
  on `kind` and is enforced by a `@model_validator`. `message` is now
  a `@computed_field` derived from the populated slots; callers never
  pass message strings.
- `RegistryTableSchemaError`: unchanged — subclasses `ValueError`,
  carries `mismatches: list[SchemaMismatch]`, and `str(exc)`
  aggregates the computed messages.

Type annotations on the `model_type` / `column_type` slots use
`TypeAnnotation` from `fgmetric._typing_extensions` (the underscored
import is intentional pending the symbol's promotion to fgmetric's
public API). Adds `fgmetric>=0.3,<1` as a runtime dependency.

Related to #42. Tracked at #53.
msto added a commit that referenced this pull request May 21, 2026
Adds the schema-validation entry point and the enumeration-only error
paths (`missing_on_table`, `missing_on_model`). Per-field type
comparison is layered on in the next commit; this PR is intentionally
scoped to "do columns line up", not "do their types agree".

- `_validate_table_schema(model_cls, table, *, allow_extra_columns)`
  iterates the model's `model_fields` (excluding the base `id` / `name`)
  against the table's columns. Reports a `MISSING_ON_TABLE` mismatch
  for any model field with no matching column. When
  `allow_extra_columns=False`, reports `MISSING_ON_MODEL` for any column
  the model does not declare. Fields present on both sides are silently
  passed through — the next commit fills in the comparison.
- `model_type` is sourced from `info.annotation` and is typed
  `TypeAnnotation` (from fgmetric) — no string-stringification.
  `column_type` is the SDK-resolved Python type on the `Column`.

Per @msto's review: this PR (originally #48) is now split. The internal
helper still returns `list[SchemaMismatch]` for test introspection;
the classmethod that raises lands in #49 and is a thin wrapper that
calls the helper and raises if the list is non-empty.

Related to #42. Tracked at #53.
msto added a commit that referenced this pull request May 21, 2026
Adds the types that the rest of the schema-validation work hangs off,
revised per #47 review feedback.

- `SchemaMismatchKind`: a `StrEnum` (was a `Literal`); wire-stable
  string values. Also exposed as `SchemaMismatch.Kind` for
  discoverability.
- `SchemaMismatch`: frozen Pydantic model. Replaces the ambiguous
  `expected` / `actual` fields with two optional pairs — `model_field`
  / `column_name` and `model_type` / `column_type` — explicit about
  which side each value comes from. Which slots are populated depends
  on `kind` and is enforced by a `@model_validator`. `message` is now
  a `@computed_field` derived from the populated slots; callers never
  pass message strings.
- `RegistryTableSchemaError`: unchanged — subclasses `ValueError`,
  carries `mismatches: list[SchemaMismatch]`, and `str(exc)`
  aggregates the computed messages.

Type annotations on the `model_type` / `column_type` slots use
`TypeAnnotation` from `fgmetric._typing_extensions` (the underscored
import is intentional pending the symbol's promotion to fgmetric's
public API). Adds `fgmetric>=0.3,<1` as a runtime dependency.

Related to #42. Tracked at #53.
msto added a commit that referenced this pull request May 21, 2026
Adds the schema-validation entry point and the enumeration-only error
paths (`missing_on_table`, `missing_on_model`). Per-field type
comparison is layered on in the next commit; this PR is intentionally
scoped to "do columns line up", not "do their types agree".

- `_validate_table_schema(model_cls, table, *, allow_extra_columns)`
  iterates the model's `model_fields` (excluding the base `id` / `name`)
  against the table's columns. Reports a `MISSING_ON_TABLE` mismatch
  for any model field with no matching column. When
  `allow_extra_columns=False`, reports `MISSING_ON_MODEL` for any column
  the model does not declare. Fields present on both sides are silently
  passed through — the next commit fills in the comparison.
- `model_type` is sourced from `info.annotation` and is typed
  `TypeAnnotation` (from fgmetric) — no string-stringification.
  `column_type` is the SDK-resolved Python type on the `Column`.

Per @msto's review: this PR (originally #48) is now split. The internal
helper still returns `list[SchemaMismatch]` for test introspection;
the classmethod that raises lands in #49 and is a thin wrapper that
calls the helper and raises if the list is non-empty.

Related to #42. Tracked at #53.
msto added a commit that referenced this pull request May 21, 2026
…rdModel

Wraps the internal `_validate_table_schema` helper in a classmethod so
subclasses can call `MyModel.validate_table_schema(table_id)` at startup
or in CI to fail fast on schema drift.

Behavior:
- Constructs `Table(id=table_id)` and calls `.load()`, which performs
  the network round-trip needed to populate the table's columns. The
  validator's `table.get_columns()` returns the empty dict on an
  un-loaded table, so the load is required (and documented inline).
- Defaults `allow_extra_columns=True`. Pass `False` for strict 1:1
  checks (CI use).
- Raises `RegistryTableSchemaError` (carrying `list[SchemaMismatch]` on
  `.mismatches`) when the helper finds disagreements; returns `None`
  on a clean match.

**Two-layer design (re your previous "why not raise?" question):**
the internal helper still returns `list[SchemaMismatch]` so tests can
introspect per-mismatch details without `pytest.raises` ceremony, and
so future non-raising consumers (e.g. workspace-wide drift reports)
can call the helper directly. The classmethod is the raising surface.

**Existing callers of `LatchRecordModel` are unaffected** — only a new
classmethod is added; no existing call sites change behavior.

Related to #42. Tracked at #53.
msto added a commit that referenced this pull request May 21, 2026
Extends `_compare_unwrapped` with a dispatch branch for `Enum`
subclasses, plus `_compare_enum`.

**Comparison fix (per @msto's review):** members are compared by
**Registry `.name`** ↔ **model `.value`**, not name-on-both-sides.
The SDK builds column enums via `Enum("Enum", members)`, which puts
the Registry strings in `.name` and auto-ints in `.value`. Per Python
convention, model enums put the identifier (e.g. "FOO") in `.name`
and the user-assigned string (e.g. "Foo") in `.value`. The previous
test fixture used identifier=value (`ALPHA = "ALPHA"`), masking the
bug — new fixtures use realistic identifier!=value pairs.

Model enums whose `.value` is not a string (e.g. `IntEnum`, `Enum`
with non-string values) fail wholesale with `ENUM_MEMBER_MISMATCH`,
which is intentional: the user must explicitly pick the Registry
strings via `.value`. This is verified by dedicated tests.

This PR was previously bundled with blob support; per @msto's split
request, blob lands in a follow-up stacked PR.

Related to #42. Tracked at #53.
msto added a commit that referenced this pull request May 21, 2026
Extends `_compare_unwrapped` with a dispatch branch for `Enum`
subclasses, plus `_compare_enum`.

**Comparison fix (per @msto's review):** members are compared by
**Registry `.name`** ↔ **model `.value`**, not name-on-both-sides.
The SDK builds column enums via `Enum("Enum", members)`, which puts
the Registry strings in `.name` and auto-ints in `.value`. Per Python
convention, model enums put the identifier (e.g. "FOO") in `.name`
and the user-assigned string (e.g. "Foo") in `.value`. The previous
test fixture used identifier=value (`ALPHA = "ALPHA"`), masking the
bug — new fixtures use realistic identifier!=value pairs.

Model enums whose `.value` is not a string (e.g. `IntEnum`, `Enum`
with non-string values) fail wholesale with `ENUM_MEMBER_MISMATCH`,
which is intentional: the user must explicitly pick the Registry
strings via `.value`. This is verified by dedicated tests.

This PR was previously bundled with blob support; per @msto's split
request, blob lands in a follow-up stacked PR.

Related to #42. Tracked at #53.
msto added a commit that referenced this pull request May 21, 2026
Extends `_compare_unwrapped` with a dispatch branch for `list[T]`,
plus `_compare_list`.

Uses `fgmetric._typing_extensions.is_list` to identify both sides as
arrays. `_compare_list` then recurses on element types via
`_compare_unwrapped`, so any element kind (primitive, enum, blob, or
nested list) is handled by the same dispatch that runs at the top
level. Element-level mismatches surface with the inner `kind` and a
`[*]`-qualified `model_field` — e.g. `list[MyEnum]` against an
`array<enum>` with disagreeing members produces an
`ENUM_MEMBER_MISMATCH` with `model_field="statuses[*]"`.

Nullability is enforced at the column boundary, not per-element,
because Registry arrays carry `allowEmpty` on the array itself.

Split out of the original combined array+link PR per @msto's request.

Related to #42. Tracked at #53.
msto added a commit that referenced this pull request May 21, 2026
Adds the types that the rest of the schema-validation work hangs off,
revised per #47 review feedback.

- `SchemaMismatchKind`: a `StrEnum` (was a `Literal`); wire-stable
  string values. Also exposed as `SchemaMismatch.Kind` for
  discoverability.
- `SchemaMismatch`: frozen Pydantic model. Replaces the ambiguous
  `expected` / `actual` fields with two optional pairs — `model_field`
  / `column_name` and `model_type` / `column_type` — explicit about
  which side each value comes from. Which slots are populated depends
  on `kind` and is enforced by a `@model_validator`. `message` is now
  a `@computed_field` derived from the populated slots; callers never
  pass message strings.
- `RegistryTableSchemaError`: unchanged — subclasses `ValueError`,
  carries `mismatches: list[SchemaMismatch]`, and `str(exc)`
  aggregates the computed messages.

Type annotations on the `model_type` / `column_type` slots use
`TypeAnnotation` from `fgmetric._typing_extensions` (the underscored
import is intentional pending the symbol's promotion to fgmetric's
public API). Adds `fgmetric>=0.3,<1` as a runtime dependency.

Related to #42. Tracked at #53.
msto added a commit that referenced this pull request May 21, 2026
Adds the schema-validation entry point and the enumeration-only error
paths (`missing_on_table`, `missing_on_model`). Per-field type
comparison is layered on in the next commit; this PR is intentionally
scoped to "do columns line up", not "do their types agree".

- `_validate_table_schema(model_cls, table, *, allow_extra_columns)`
  iterates the model's `model_fields` (excluding the base `id` / `name`)
  against the table's columns. Reports a `MISSING_ON_TABLE` mismatch
  for any model field with no matching column. When
  `allow_extra_columns=False`, reports `MISSING_ON_MODEL` for any column
  the model does not declare. Fields present on both sides are silently
  passed through — the next commit fills in the comparison.
- `model_type` is sourced from `info.annotation` and is typed
  `TypeAnnotation` (from fgmetric) — no string-stringification.
  `column_type` is the SDK-resolved Python type on the `Column`.

Per @msto's review: this PR (originally #48) is now split. The internal
helper still returns `list[SchemaMismatch]` for test introspection;
the classmethod that raises lands in #49 and is a thin wrapper that
calls the helper and raises if the list is non-empty.

Related to #42. Tracked at #53.
msto added a commit that referenced this pull request May 21, 2026
Layers per-field type comparison on top of the enumeration dispatcher
added in the previous PR. Each model field that has a matching column
is now compared in two stages:

- `_compare_field_to_column`: checks nullability first. Uses
  `fgmetric._typing_extensions.is_optional` / `unpack_optional` to
  decide whether the model annotation includes `None`; checks against
  the column's `allowEmpty`. If they disagree, emits
  `NULLABILITY_MISMATCH` (carrying both sides' types for the message).
  Otherwise, unwraps the optional on both sides and forwards to the
  inner comparator.
- `_compare_unwrapped`: primitive identity check on the unwrapped
  types. The column's Python type comes from the SDK's `to_python_type`.
  Returns `TYPE_MISMATCH` when the two sides disagree. Subsequent
  commits extend this dispatcher with enum / blob / array / link
  branches.

Per @msto's review: comparators now return `SchemaMismatch | None` (was
`list[SchemaMismatch]`). The caller hoists the `None` check inline.

The `allowEmpty` lookup defaults to `False` when absent — the SDK's
TypedDict marks it required, but a malformed payload shouldn't crash
validation. A dedicated test asserts the defensive default.

Related to #42. Tracked at #53.
msto added a commit that referenced this pull request May 21, 2026
…rdModel

Wraps the internal `_validate_table_schema` helper in a classmethod so
subclasses can call `MyModel.validate_table_schema(table_id)` at startup
or in CI to fail fast on schema drift.

Behavior:
- Constructs `Table(id=table_id)` and calls `.load()`, which performs
  the network round-trip needed to populate the table's columns. The
  validator's `table.get_columns()` returns the empty dict on an
  un-loaded table, so the load is required (and documented inline).
- Defaults `allow_extra_columns=True`. Pass `False` for strict 1:1
  checks (CI use).
- Raises `RegistryTableSchemaError` (carrying `list[SchemaMismatch]` on
  `.mismatches`) when the helper finds disagreements; returns `None`
  on a clean match.

**Two-layer design (re your previous "why not raise?" question):**
the internal helper still returns `list[SchemaMismatch]` so tests can
introspect per-mismatch details without `pytest.raises` ceremony, and
so future non-raising consumers (e.g. workspace-wide drift reports)
can call the helper directly. The classmethod is the raising surface.

**Existing callers of `LatchRecordModel` are unaffected** — only a new
classmethod is added; no existing call sites change behavior.

Related to #42. Tracked at #53.
msto added a commit that referenced this pull request May 21, 2026
Extends `_compare_unwrapped` with a dispatch branch for `Enum`
subclasses, plus `_compare_enum`.

**Comparison fix (per @msto's review):** members are compared by
**Registry `.name`** ↔ **model `.value`**, not name-on-both-sides.
The SDK builds column enums via `Enum("Enum", members)`, which puts
the Registry strings in `.name` and auto-ints in `.value`. Per Python
convention, model enums put the identifier (e.g. "FOO") in `.name`
and the user-assigned string (e.g. "Foo") in `.value`. The previous
test fixture used identifier=value (`ALPHA = "ALPHA"`), masking the
bug — new fixtures use realistic identifier!=value pairs.

Model enums whose `.value` is not a string (e.g. `IntEnum`, `Enum`
with non-string values) fail wholesale with `ENUM_MEMBER_MISMATCH`,
which is intentional: the user must explicitly pick the Registry
strings via `.value`. This is verified by dedicated tests.

This PR was previously bundled with blob support; per @msto's split
request, blob lands in a follow-up stacked PR.

Related to #42. Tracked at #53.
msto added a commit that referenced this pull request May 21, 2026
Extends `_compare_unwrapped` with a dispatch branch for `LatchFile`
and `LatchDir`, plus `_compare_blob`.

The SDK's `to_python_type` routes blob columns through
`get_blob_nodetype`, which returns `LatchDir` when
`metadata.nodeType == "dir"` and `LatchFile` otherwise (including when
metadata is missing). So a model declaring `LatchFile` against a
column whose nodeType is `"dir"` produces a `BLOB_TYPE_MISMATCH`; a
model declaring either against a non-blob column produces a
`TYPE_MISMATCH`.

Split out of the original combined enum+blob PR per @msto's request.

Related to #42. Tracked at #53.
msto added a commit that referenced this pull request May 21, 2026
Extends `_compare_unwrapped` with a dispatch branch for `list[T]`,
plus `_compare_list`.

Uses `fgmetric._typing_extensions.is_list` to identify both sides as
arrays. `_compare_list` then recurses on element types via
`_compare_unwrapped`, so any element kind (primitive, enum, blob, or
nested list) is handled by the same dispatch that runs at the top
level. Element-level mismatches surface with the inner `kind` and a
`[*]`-qualified `model_field` — e.g. `list[MyEnum]` against an
`array<enum>` with disagreeing members produces an
`ENUM_MEMBER_MISMATCH` with `model_field="statuses[*]"`.

Nullability is enforced at the column boundary, not per-element,
because Registry arrays carry `allowEmpty` on the array itself.

Split out of the original combined array+link PR per @msto's request.

Related to #42. Tracked at #53.
msto added a commit that referenced this pull request May 21, 2026
Extends `_compare_unwrapped` with a dispatch branch for
`LatchRecordModel` subclasses, plus `_compare_link`.

Per spec, the link's target table is not checked — any
`LatchRecordModel` subclass matches any `link` column, keeping the
check tolerant of models that represent only a subset of a larger
linked table. The SDK's `to_python_type` maps link columns to
`Record`, so a column is a link iff `column_type is Record`.

**`_looks_like_latch_record_model` is a structural check** (per
@msto's review): instead of `issubclass(t, LatchRecordModel)` —
which would force a lazy local import to avoid the circular
`_record_model` ↔ `_schema` dependency — it verifies that `t` is a
Pydantic `BaseModel` subclass with the `id` and `name` fields
`LatchRecordModel` mandates. A dedicated test confirms an unrelated
`BaseModel` without those fields is NOT treated as a link.

Split out of the original combined array+link PR per @msto's request.

Related to #42. Tracked at #53.
@msto msto closed this Sep 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant