Skip to content

feat(registry): add schema validation dispatcher + missing/extra columns - #48

Closed
msto wants to merge 1 commit into
feat/validate-table-schemafrom
feat/registry-validate-primitives
Closed

msto wants to merge 1 commit into
feat/validate-table-schemafrom
feat/registry-validate-primitives

Conversation

@msto

@msto msto commented May 21, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Related to #42. Stacked on #47. Tracked at #53.

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 stacked PR (#48b); 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
    passed through silently — the next PR fills in the comparison.
  • model_type is sourced from info.annotation and typed
    TypeAnnotation (from fgmetric._typing_extensions), so the
    literal annotation is preserved end-to-end. column_type is the
    SDK-resolved Python type on the Column.

Design note (re your "why two layers" comment on the previous
revision):
the internal helper still returns list[SchemaMismatch]
because test introspection is much cleaner that way (no
pytest.raises ceremony to inspect per-field details). The classmethod
that raises lands in #49 and is a thin wrapper.

Test plan

  • MISSING_ON_TABLE populates model_field + model_type and leaves
    column_* None.
  • MISSING_ON_MODEL populates column_name + column_type and
    leaves model_* None.
  • allow_extra_columns=True silences MISSING_ON_MODEL.
  • id / name model fields are skipped.
  • Multiple mismatches are collected in one pass.

Co-Authored-By: Claude noreply@anthropic.com

Comment on lines +15 to +16
if TYPE_CHECKING:
from fglatch.registry._record_model import LatchRecordModel

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question
Is this necessary? Any other way to avoid a circular import?

Comment thread fglatch/registry/_schema.py Outdated
Comment on lines +71 to +76
def _is_nullable(annotation: Any) -> bool:
"""True if the annotation is a `T | None` or `Optional[T]` / `Union[..., None]`."""
origin = get_origin(annotation)
if origin is not Union and origin is not UnionType:
return False
return type(None) in get_args(annotation)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question
Is this the same as https://github.com/fg-labs/fgmetric/blob/9af9921a5a6dcb5b4f45fdf6259d1b2d8eb1887d/fgmetric/_typing_extensions.py#L24-L58 ?

Can we reuse? I can export is_optional in fgmetric to make it part of the public API.

Comment thread fglatch/registry/_schema.py Outdated
Comment on lines +79 to +91
def _unwrap_none(annotation: Any) -> Any:
"""
Given `T | None`, return `T`.

Returns the annotation unchanged when it is not a simple `T | None` (e.g. tagged
unions like `A | B | None`, which are out of scope for v1).
"""
if not _is_nullable(annotation):
return annotation
non_none_args = [a for a in get_args(annotation) if a is not type(None)]
if len(non_none_args) != 1:
return annotation
return non_none_args[0]

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread fglatch/registry/_schema.py Outdated
Comment on lines +94 to +98
def _describe_type(annotation: Any) -> str:
"""Produce a short human-readable string for a type annotation."""
if isinstance(annotation, type):
return annotation.__name__
return str(annotation).replace("typing.", "")

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion(blocking)

Suggested change
def _describe_type(annotation: Any) -> str:
"""Produce a short human-readable string for a type annotation."""
if isinstance(annotation, type):
return annotation.__name__
return str(annotation).replace("typing.", "")

Remove this - I'd rather have the literal annotation preserved. Explicit is better than implicit and I don't want the annotations formatted inconsistently depending on their parent module.

Comment thread fglatch/registry/_schema.py Outdated
expected_type: Any,
actual_type: Any,
*,
message: str,

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question
I think in #47 we are going to make this a property on SchemaMismatch - unless we need to override it in some cases?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps custom formatting could be dispatched through the kind Enum being added in #47?



def _validate_table_schema(
model_cls: "type[LatchRecordModel]",

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question
Why do we need the forward reference?

Comment thread fglatch/registry/_schema.py Outdated
mismatches: list[SchemaMismatch] = []
columns: dict[str, Column] = table.get_columns()

model_annotations: dict[str, Any] = {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion
Can this be typed as TypeAnnotation or similar, instead of Any?

Best to avoid Any where possible.

https://github.com/fg-labs/fgmetric/blob/9af9921a5a6dcb5b4f45fdf6259d1b2d8eb1887d/fgmetric/_typing_extensions.py#L12

Comment thread fglatch/registry/_schema.py Outdated
`_validate_table_schema`.
"""
model_is_nullable = _is_nullable(model_annotation)
column_is_nullable: bool = column.upstream_type["allowEmpty"]

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question
Is this safe, or do we need to guard against a missing allowEmpty key?

Comment thread fglatch/registry/_schema.py Outdated
field_name: str,
expected_type: Any,
actual_type: Any,
) -> list[SchemaMismatch]:

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question
Are these comparison helpers all returning list[SchemaMismatch] just to make it easier to use .extend() to cover the "no mismatch" branch? e.g. instead of SchemaMismatch | None? Seems like they all either return an empty list or a singleton list.

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
msto force-pushed the feat/registry-validate-primitives branch from 80a88d3 to 2255051 Compare May 21, 2026 17:58
@msto msto changed the title feat(registry): validate primitives, nullability, and missing/extra columns feat(registry): add schema validation dispatcher + missing/extra columns May 21, 2026
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
msto force-pushed the feat/registry-validate-primitives branch from 2255051 to 478b377 Compare May 21, 2026 18:42
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
msto force-pushed the feat/registry-validate-primitives branch from 478b377 to 5d9ae53 Compare May 21, 2026 19:22
@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