Skip to content

fix(preprocessor): validate embeddings against the fitted dimensions - #40

Open
ChrisW09 wants to merge 1 commit into
mainfrom
fix/validate-embeddings-at-transform
Open

fix(preprocessor): validate embeddings against the fitted dimensions#40
ChrisW09 wants to merge 1 commit into
mainfrom
fix/validate-embeddings-at-transform

Conversation

@ChrisW09

Copy link
Copy Markdown
Collaborator

Fixes #34.

Problem

fit populates embedding_dimensions_ for every embedding array it receives — and nothing
ever reads it back. transform checked only "were embeddings expected at all", so every
mismatch was accepted silently:

fitted embedding_dimensions_: {'embedding_1': 8}
transform(embeddings=(100, 3)) -> (100, 3)                                   accepted
transform(embeddings=(7, 8))   -> {'num_a': (100,1), 'embedding_1': (7,8)}   accepted

That last one is the damaging case: the returned dict has blocks of different heights. A
caller who stacks them gets a shape error a long way from the cause; a caller who indexes
them gets silently misaligned rows.

Two more went unchecked: passing fewer arrays than were fitted (the missing block just
vanished from the dict), and a 1-D array.

Fix

transform now validates against what fit recorded — array count, per-array width, 2-D
shape, and row count against X — raising PretabDataError that names the array and both
shapes:

embedding_1 has 3 columns, but 8 were seen during fit.
embedding_1 has 7 rows, but X has 100.
embedding_1 must be a 2D array, got 1 dimension(s).
Expected 2 embedding array(s) as seen during fit, got 1.

fit and transform share one normalization helper, so a bare array and a one-element list
are treated identically on both sides — previously each branch had its own isinstance
ladder.

Left as is

Fitting with embeddings and transforming without them still returns the dict without the
embedding blocks, rather than raising. The issue flagged the asymmetry with the opposite
direction (which does raise). I left it alone because tightening it could break a caller who
deliberately transforms without embeddings, and it is a design call rather than a defect —
happy to change it if you want the symmetry.

Impact

This is the documented integration point for an embedding host such as DeepTab — the caller
most likely to wire up the wrong array and least likely to spot it.

Tests

Six added to tests/test_preprocessor.py: matching embeddings still pass through (including
the float32 cast), wrong width, wrong row count, wrong array count, a two-array round trip,
and a 1-D array.

Full suite: 486 passed, 9 xfailed. ruff check clean on changed files; pyright unchanged at
71. tests/test_preprocessor.py also picks up the import sort this file has needed since
before these changes.

🤖 Generated with Claude Code

``fit`` recorded ``embedding_dimensions_`` for every embedding array it was
given, but nothing ever read it back, so ``transform`` accepted anything:

    fitted {'embedding_1': 8}
    transform(embeddings=(100, 3))  -> accepted
    transform(embeddings=(7, 8))    -> {'num_a': (100, 1), 'embedding_1': (7, 8)}

The row-count case is the damaging one -- the returned dict has blocks of
different heights, which surfaces either as a shape error far from the cause or,
if the caller indexes rather than stacks, as silently misaligned rows.

Validate in ``transform`` against what ``fit`` saw: the number of arrays, each
array's width, that each is 2D, and that its row count matches ``X``. Raise
``PretabDataError`` naming the offending array and both shapes.

``fit`` and ``transform`` now share one normalization helper, so a single array
and a one-element list are treated identically on both sides.

Closes #34

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ChrisW09
ChrisW09 force-pushed the fix/validate-embeddings-at-transform branch from 48cff5e to 3fc1d11 Compare July 27, 2026 20:48
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.

fix(preprocessor): validate embeddings against the fitted dimensions

1 participant