Skip to content

test: validate nested values in response assertions - #3914

Open
1fanwang wants to merge 2 commits into
openai:mainfrom
1fanwang:1fannnw/validate-nested-test-types
Open

1fanwang wants to merge 2 commits into
openai:mainfrom
1fanwang:1fannnw/validate-nested-test-types

Conversation

@1fanwang

Copy link
Copy Markdown
Contributor
  • I understand that this repository is auto-generated and my pull request may not be merged

Changes being requested

API tests can pass when response fields inside lists or sequences have the wrong types. Their verifier calls a static typing helper that does not validate values when the tests run.

Check each member with the existing recursive verifier and retain its position in the diagnostic path. Unconstrained annotations still accept any value. This changes test verification, not the SDK's response-parsing behavior.

Additional context & links

The added regression starts a real local HTTP server and calls the sync and async clients. In loose-validation mode, an invalid model-creation timestamp reaches the verifier as a string. The verifier now rejects it; the valid integer response still passes.

Testing Done

The baseline is 69a2c1d.

Set up the project's locked environment with uv sync --locked. Create a separate checkout at that baseline, copy the added regression module into it, and share the environment:

git worktree add --detach ../openai-type-assertions-before 69a2c1db6feacf32be6693809e7cab1c3b49cad7
cp tests/test_utils/test_assert_matches_type.py ../openai-type-assertions-before/tests/test_utils/
ln -s "$PWD/.venv" ../openai-type-assertions-before/.venv

Run the same command from the baseline and PR checkouts:

PYTHONPATH=src:. .venv/bin/python -m pytest -q -n 0 -s tests/test_utils/test_assert_matches_type.py -k models_api_types

Raw output before the fix:

sync invalid=False created_type=int rejected=False
.sync invalid=True created_type=str rejected=False
Fasync invalid=False created_type=int rejected=False
.async invalid=True created_type=str rejected=False
F

Both invalid-response cases fail with:

E   assert False == True

Raw output after the fix:

sync invalid=False created_type=int rejected=False
.sync invalid=True created_type=str rejected=True
.async invalid=False created_type=int rejected=False
.async invalid=True created_type=str rejected=True
.

The baseline exits 1; the patched checkout exits 0. The same HTTP checks also pass with Pydantic v1.

  • Local code review completed.

Signed-off-by: 1fanwang <1fannnw@gmail.com>
Signed-off-by: 1fanwang <1fannnw@gmail.com>
@1fanwang
1fanwang requested a review from a team as a code owner September 20, 2026 00:55
@jnohclee-rgb

Copy link
Copy Markdown

Independent offline verification (AI-assisted): I extended the verifier check to actual SDK Model instances constructed with a deliberately wrong created type through list, sequence-of-lists, dictionary-of-lists, list-of-dictionaries-of-sequences, nullable-list and Annotated-list shapes, with corresponding valid controls plus Any/Literal boundaries (17 cases). Main passed 9/17; this head passed 17/17. The deep invalid case retained the diagnostic path response / 0 / <dict item> / 0 / created, and nullable-list failures correctly pointed to element 1. These are test-verifier checks, not a change to SDK response parsing. I executed the unchanged AST-selected verifier functions/imports, omitting only the rich display and unrelated infrastructure; no server or full test suite was run.

Compared main 919b6236382f3f9f76d8453efb9f9ad03cff1126 with this PR head a16c8b3329f53f5470b6fc3ad406a9f83c9a6481, isolated source trees, Python 3.14.7, Pydantic 2.13.5 and shared dependencies plus isolated legacy HTTPX compatibility dependencies. Network-denied and fictional values only. Not a historical lockfile matrix, full SDK or merge approval.

Runnable separately against each source tree (pass its tests/utils.py path as the first argument):

import ast,importlib,json,pathlib,sys
from typing import Annotated,Any,Sequence,Literal
from openai.types import Model
path=pathlib.Path(sys.argv[1]);tree=ast.parse(path.read_text());names={'evaluate_forwardref','assert_matches_model','assert_matches_type','_assert_list_type'}
# Execute unchanged verifier functions and their existing imports; omit rich display/server/test infrastructure.
body=[n for n in tree.body if isinstance(n,(ast.Import,ast.ImportFrom)) and not (isinstance(n,ast.Import) and any(a.name=='rich' for a in n.names)) or isinstance(n,ast.Assign) and any(isinstance(t,ast.Name) and t.id=='BaseModelT' for t in n.targets) or isinstance(n,ast.FunctionDef) and n.name in names]
ns={};exec(compile(ast.Module(body=body,type_ignores=[]),str(path),'exec'),ns)
valid=Model.model_construct(id='fictional',created=0,object='model',owned_by='fictional');invalid=Model.model_construct(id='fictional',created='wrong',object='model',owned_by='fictional')
shapes=[('list',list[Model],lambda m:[m]),('sequence_list',Sequence[list[Model]],lambda m:([m],)),('dict_list',dict[str,list[Model]],lambda m:{'first':[m]}),('list_dict_sequence',list[dict[str,Sequence[Model]]],lambda m:[{'first':(m,)}]),('nullable_list',list[Model|None],lambda m:[None,m]),('annotated_list',Annotated[list[Model],'metadata'],lambda m:[m])]
cases=[(name+'_'+('valid' if good else 'invalid'),annotation,wrap(valid if good else invalid),good) for name,annotation,wrap in shapes for good in [False,True]]
cases += [('direct_any',Any,{'unknown':object()},True),('list_any',list[Any],[1,'fictional',None],True),('sequence_any',Sequence[Any],(1,'fictional',None),True),('literal_valid',list[Literal['fixture']],['fixture'],True),('literal_invalid',list[Literal['fixture']],['wrong'],False)]
rows=[]
for name,annotation,value,expected in cases:
 try:ns['assert_matches_type'](annotation,value,path=['response']);accepted=True;trace_paths=[]
 except AssertionError as exc:
  accepted=False;trace_paths=[];tb=exc.__traceback__
  while tb:
   frame_path=tb.tb_frame.f_locals.get('path')
   if isinstance(frame_path,list):trace_paths.append(list(frame_path))
   tb=tb.tb_next
 rows.append({'case':name,'accepted':accepted,'expected_accepted':expected,'diagnostic_paths':trace_paths,'pass':accepted==expected})
print(json.dumps({'source_verifier':str(path),'cases':rows,'passed':sum(r['pass'] for r in rows),'failed':sum(not r['pass'] for r in rows),'scope':'Unchanged AST-selected test verifier functions; actual SDK Model constructed without coercion, nested model/union/Annotated/Any/Literal boundaries; no full test suite, server or SDK parser behavior claim.'}))

This branch has not been deployed

No deployments
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.

2 participants