Python: fix KernelJsonSchemaBuilder ignoring string forward references inside list[...]/dict[...]

[Diwakar Ray Yadav (Diwak4r)]

Description

KernelJsonSchemaBuilder silently dropped the element schema when a container's element type was written as a string forward reference. list["Inner"] produced {"type": "array", "items": {"type": "object"}} — no properties, no required — while list[Inner] produced the full schema. The same happened for dict[str, "Inner"] (which fell into the Python 3.10 empty-properties workaround) and tuple["Inner", ...].

This matters because the generated schema is what gets sent to models as a function-calling parameter definition — a plugin using this annotation style handed the model an untyped blob for that argument.

Why it happened

list["Inner"] goes through list.__class_getitem__, which stores the string verbatim in __args__ with no ForwardRef wrapper, so get_type_hints never evaluates it. handle_complex_type then called cls.build("Inner", ...), which took the isinstance(parameter_type, str) branch and fell through build_from_type_name to the {"type": "object"} fallback.

Fix

Resolve raw-string __args__ entries against the owning model's module globals before recursing, as suggested in the issue:

• build gains an optional globalns parameter; when omitted (direct calls like build(list["Inner"])), it falls back to the caller's module globals, mirroring how forward references resolve at class-definition time.
• build_model_schema threads its already-fetched model_module_globals into build.
• handle_complex_type resolves string args via _resolve_forward_ref for list/set, dict, tuple, and Union/Optional before recursing, and threads globalns through nested containers (e.g. list[list["Inner"]]).

list["Inner"] and list[Inner] now produce identical schemas.

Test plan

• New tests cover list["Inner"], list[list["Inner"]], dict[str, "Inner"], and tuple["Inner", ...] built directly as aliases, plus model-driven forward references (list["Inner"], list[list["Inner"]], dict[str, "Inner"] fields and a top-level "Inner" field).
• The direct-alias tests fail before this change (bare {"type": "object"}) and pass after.
• Full tests/unit/schema/test_schema_builder.py passes: 44 tests green; ruff check and ruff format --check clean on both files.

Fixes #14239

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[github-actions]

Automated Code Review

Reviewers: 5 | Confidence: 77%

✓ Correctness

The implementation is correct. The globalns parameter is properly threaded through all recursive call paths, the _resolve_forward_ref helper degrades gracefully, and the sys._getframe(1) fallback only triggers on direct external calls (never on internal recursion). The fix correctly targets the specific gap where raw strings in generic alias __args__ weren't being resolved.

✓ Security Reliability

The PR fixes forward reference resolution in KernelJsonSchemaBuilder. The main reliability concern is the use of sys._getframe(1).f_globals which is a CPython implementation detail that may break in alternative Python runtimes or when the call depth changes (e.g., internal refactoring adding wrapper methods). The _resolve_forward_ref helper does a safe globalns.get(arg) lookup with a type check, which is fine. Overall the change is sound for its stated purpose.

✓ Test Coverage

The PR adds good test coverage for the new forward-reference resolution behavior. Tests cover list, nested list, dict, tuple, and top-level forward refs both via model fields and direct alias calls. However, there are two minor test coverage gaps: no test for Optional/Union with a string forward reference (the code changes handle this path), and no test for set["ForwardRef"] (also handled in the code). These are low-severity gaps since the logic is identical to the tested list path.

✓ Failure Modes

The change is well-structured and correctly threads globalns through the schema builder to resolve string forward references. The use of sys._getframe(1).f_globals is a CPython implementation detail but is the standard Python pattern for this purpose, and failure to resolve simply preserves pre-existing fallback behavior (bare {"type": "object"}). No new silent failure modes, lost errors, or partial-write risks are introduced.

✓ Design Approach

The forward-reference resolution approach looks correct for list/dict/nested container aliases, but the tuple handling is still incomplete: the changed code continues to treat ... as a literal tuple entry rather than the homogeneous-tuple marker. That means one of the regression paths named in the PR rationale (tuple["Inner", ...]) is still not actually fixed.

---

Automated review by Diwak4r's agents

[github-actions]

The tuple branch does not handle the homogeneous-tuple marker (tuple["Inner", ...]). _resolve_forward_ref only transforms strings, so Ellipsis passes through unchanged and is treated as a real tuple element. The resulting schema is a two-item tuple rather than an array of repeated ForwardRefInnerModels. The Ellipsis sentinel needs to be detected before iterating over args.

Suggested change

	schema["additionalProperties"] = False
	return schema
	if origin is tuple:
	items = [cls.build(arg, structured_output=structured_output) for arg in args]
	items = [
	cls.build(
	cls._resolve_forward_ref(arg, globalns), structured_output=structured_output, globalns=globalns
	)
	if origin is tuple:
	if len(args) == 2 and args[1] is Ellipsis:
	item_type = cls._resolve_forward_ref(args[0], globalns)
	schema = {
	"type": "array",
	"items": cls.build(item_type, structured_output=structured_output, globalns=globalns),
	}
	else:
	items = [
	cls.build(
	cls._resolve_forward_ref(arg, globalns), structured_output=structured_output, globalns=globalns
	)
	for arg in args
	]
	schema = {"type": "array", "items": items}

[Diwakar Ray Yadav (Diwak4r)]

Just verifying this is still in the review queue. All checks green, branch is mergeable. Happy to address any feedback.