[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.

[Diwakar Ray Yadav (Diwak4r)]

Closing this to clear out a backlog I let build up rather than leave it sitting in your queue. It was never picked up and at this age it isn't going to be. If it's still wanted, say so and I'll reopen it against current main.