Skip to content

fix(partial): don't fail final stream validation on explicit nulls for required nullable fields#2452

Open
sohumt123 wants to merge 1 commit into
567-labs:mainfrom
sohumt123:fix-partial-final-validation-explicit-nulls
Open

fix(partial): don't fail final stream validation on explicit nulls for required nullable fields#2452
sohumt123 wants to merge 1 commit into
567-labs:mainfrom
sohumt123:fix-partial-final-validation-explicit-nulls

Conversation

@sohumt123

Copy link
Copy Markdown

Describe your changes

Streaming with create_partial raises a spurious ValidationError on valid, completed streams whenever the response model has a required-but-nullable field (e.g. email: Optional[str] with no default) that the LLM legitimately returns as JSON null.

Repro (fails on main at 47fdb2c, both sync and async):

from typing import Optional
from pydantic import BaseModel
from instructor.dsl.partial import Partial

class User(BaseModel):
    name: str
    email: Optional[str]  # required, but nullable

chunks = ['{"name": "Al', 'ice", "email"', ': null}']
for obj in Partial[User].model_from_chunks(chunks):
    print(obj)
# pydantic_core ValidationError: 1 validation error for User
# email: Field required [type=missing, input_value={'name': 'Alice'}, ...]

Root cause: after the last chunk, PartialBase.model_from_chunks / model_from_chunks_async re-validate the accumulated object via

original_model.model_validate(final_obj.model_dump(exclude_none=True), **kwargs)

The model_dump(exclude_none=True) round-trip cannot distinguish "field explicitly null in the JSON" from "field never streamed" (both end up as None on the constructed partial), so explicit nulls are stripped from the dump and pydantic reports the field as missing — even though the stream produced complete, valid JSON and the per-chunk validation of the complete root already succeeded. The same round-trip would also mis-validate models with Field(exclude=True) fields.

Fix: validate the accumulated JSON string itself (parsed with jiter.from_json, already imported) instead of the lossy dump, in both the sync and async paths. This preserves explicit nulls while keeping the guard's intent intact:

  • a required field genuinely absent from the completed JSON still raises (existing TestFinalValidationAfterStreaming tests still pass), and
  • structurally incomplete streams still skip final validation.

Tests: added three regression tests in tests/dsl/test_partial.py — explicit-null nullable field completes cleanly (sync + async; both fail on main with the exact error above), and a genuinely-absent nullable field still raises. Ran pytest tests/dsl/ tests/coverage/test_dsl_partial_coverage.py: 90 passed, 2 skipped. ruff check/ruff format --check and ty check clean on the touched files. (The pre-existing test_citation_keeps_quotes_without_context_and_recovers_fuzzy_quotes failure in tests/coverage/test_dsl_small_coverage.py reproduces identically on a clean checkout and is unrelated — it's targeted by #2430/#2443.)

Issue ticket number and link

None — found while exercising partial streaming with nullable required fields.

Checklist before requesting a review

  • I have performed a self-review of my code
  • If it is a core feature, I have added thorough tests.
  • If it is a core feature, I have added documentation.

…after streaming

After the last chunk of a partial stream, model_from_chunks and
model_from_chunks_async re-validated the constructed object via
original_model.model_validate(final_obj.model_dump(exclude_none=True)).
The exclude_none round-trip cannot distinguish a field the LLM explicitly
returned as JSON null from a field that never streamed, so required-but-
nullable fields (e.g. email: Optional[str] with no default) were stripped
from the dump and the final validation raised ValidationError 'Field
required' on perfectly valid, complete streams.

Validate the accumulated JSON string itself (parsed with jiter.from_json)
instead of the lossy dump. Explicit nulls are preserved, while the guard's
intent is kept: a required field genuinely absent from the completed JSON
still fails validation, and incomplete streams still skip it.

Adds regression tests for explicit-null nullable fields (sync and async)
and for the still-rejected genuinely-absent case.
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