[ErenAta16 (ErenAta16)]

Motivation and Context

KernelJsonSchemaBuilder drops the element schema when a container's element type is written as a string forward reference. list["Inner"] produces {"type": "object"} for the items; list[Inner] produces the full schema. That output is the function-calling parameter definition sent to the model, so a plugin written in the forward-reference style hands the model an untyped blob for that argument, with no error anywhere.

class Inner(KernelBaseModel):
    value: int
    label: str

class HolderDirect(KernelBaseModel):
    items: list[Inner] = []

class HolderFwd(KernelBaseModel):
    items: list["Inner"] = []

model	items schema
HolderDirect	{"type": "array", "items": {"type": "object", "properties": {"value": ..., "label": ...}, "required": [...]}}
HolderFwd	{"type": "array", "items": {"type": "object"}}
dict[str, "Inner"]	additionalProperties: {"type": "object", "properties": {}}
Optional["Inner"]	full schema — works

Why: get_type_hints evaluates an annotation that is a string, but it does not descend into a generic alias that already exists as an object.

HolderFwd.__annotations__["items"]              -> list['Inner']
get_type_hints(HolderFwd)["items"]              -> list['Inner']     # unchanged
get_args(...)                                    -> ('Inner',)        # a str, not a type
HolderFwd.model_fields["items"].annotation      -> list['Inner']     # pydantic doesn't either

list["Inner"] goes through list.__class_getitem__, which stores "Inner" verbatim — no ForwardRef wrapper, so there is nothing for get_type_hints to resolve. handle_complex_type then calls cls.build("Inner", ...), which takes the isinstance(parameter_type, str) branch at the top of build; build_from_type_name has no entry for "Inner" and returns the {"type": "object"} fallback.

Optional["Inner"] behaves differently because typing.Optional[...] converts the string into a real ForwardRef, and get_type_hints does resolve those. That asymmetry — one forward-reference spelling working and the other not — is what makes this easy to miss.

Fixes #14239. Also removes the reason for the TODO at kernel_json_schema_builder.py:80 ("add support for handling forward references, which is not currently tested", issue #6464), though I left the comment alone.

Description

_resolve_nested_forward_refs walks a generic alias and evaluates any str or ForwardRef argument against the owning model's module globals, then rebuilds the alias. It's called once per field in build_model_schema, where model_module_globals is already fetched for the get_type_hints call directly above.

It recurses, so list[dict[str, "Inner"]] resolves. A name that doesn't evaluate leaves the annotation untouched and the existing {"type": "object"} fallback applies — schema building never raises because of this.

How did you test it?

Four tests in python/tests/unit/schema/test_schema_builder.py:

• test_build_list_with_string_forward_reference_matches_direct_reference — asserts the two schemas are equal, so it can't pass by both being empty.
• test_build_dict_with_string_forward_reference
• test_build_optional_with_string_forward_reference_still_works — the path that already worked, pinned so this change can't regress it.
• test_unresolvable_forward_reference_falls_back_instead_of_raising

Stashing only kernel_json_schema_builder.py gives 2 failures (the list and dict tests); the Optional and unresolvable ones pass on both sides, which is the point. With the change, tests/unit/schema is 40 passed. ruff check and ruff format --check clean.

Notes for the reviewer

The eval is against vars(sys.modules[model.__module__]), the same namespace get_type_hints is already given two lines up — no new inputs reach it.

Checklist

• The code builds clean without any errors or warnings
• The PR follows the SK Contribution Guidelines and the pre-submission formatting script raises no violations
• All unit tests pass, and I have added new tests where possible
• I didn't break anyone 😄

[Copilot]

Pull request overview

This PR fixes Python KernelJsonSchemaBuilder so that string forward references nested inside container annotations (e.g., list["Inner"], dict[str, "Inner"]) are resolved to the referenced types, producing complete JSON schemas rather than falling back to {"type": "object"} for container elements/values.

Changes:

• Resolve nested str/ForwardRef arguments inside generic aliases during model schema building.
• Add unit tests covering list/dict nested string forward refs, Optional behavior, and unresolvable reference fallback.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
python/semantic_kernel/schema/kernel_json_schema_builder.py	Adds _resolve_nested_forward_refs and applies it per-field in build_model_schema to resolve nested container forward refs.
python/tests/unit/schema/test_schema_builder.py	Adds regression tests ensuring nested string forward refs produce the same schema as direct references and remain non-throwing when unresolvable.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[github-actions]

Automated Code Review

Reviewers: 5 | Confidence: 90%

✓ Correctness

The implementation correctly resolves string forward references in generic aliases (list, dict, tuple). The recursive approach, identity-based change detection, and fail-safe early return are all well-designed. There is one low-severity edge case: Literal annotations containing string values that happen to match module/builtin names (e.g., Literal["list", "dict"]) would be silently "resolved" to types via copy_with, since Literal aliases have that method. Currently this is unobservable because the schema builder doesn't handle Literal types specially anyway (both original and corrupted produce {"type": "object"}), but it could become an issue if Literal support is added later.

✓ Security Reliability

The PR introduces _resolve_nested_forward_refs which uses eval() to resolve string forward references in type annotations. The eval() operates on the same namespace (vars(sys.modules[model.__module__])) and same class of inputs (type annotation strings from source code) as get_type_hints() which already internally uses eval() two lines above at line 86. No new trust boundary is crossed — the strings are developer-written type annotations, not runtime user input. Error handling is robust: unresolvable references fall back gracefully to the existing {"type": "object"} behavior without raising. No security or reliability issues found.

✓ Test Coverage

The test suite is well-structured and covers the primary scenarios (list, dict, Optional forward refs, and unresolvable fallback). The main gap is the absence of a test for nested generics like list[dict[str, "Inner"]], which the PR rationale explicitly claims to support via recursion. The recursive branch at line 189 of the implementation is only trivially exercised (for plain str which has no type args) — true recursive resolution where an inner call actually resolves a forward ref is not tested.

✓ Failure Modes

The PR adds a well-designed _resolve_nested_forward_refs method that conservatively resolves string forward references inside generic aliases. The error handling is sound: any eval failure returns the original annotation unchanged, preserving the existing fallback behavior. The eval operates on the same namespace already used by get_type_hints (line 86), introducing no new attack surface. The recursive structure correctly handles nested generics, and the rebuild logic correctly uses Python's subscript semantics for both typing generics (copy_with) and built-in generics (origin[tuple(args)]). No concrete failure modes identified.

✓ Design Approach

The new helper fixes nested string forward references only when they appear inside model fields, but the same unresolved generic aliases still flow through the function-parameter schema path unchanged. Because the PR rationale explicitly frames this as fixing the function-calling schema sent to the model, I think the current design stops one layer too early and leaves the direct type_object=list["Inner"] case broken.

---

Automated review by ErenAta16's agents

[github-actions]

This only resolves nested forward refs for fields coming from build_model_schema. Direct kernel-function parameters still miss the fix: kernel_function_decorator.py:122-129 stores the raw annotation in type_object, and kernel_parameter_metadata.py:49-50 later calls KernelJsonSchemaBuilder.build(type_object, ...) on that value. For an input like ``@kernel_function def f(items: list["InerForward"]): ..., `build()` goes straight to `handle_complex_type` without ever hitting this line, so the function-calling schema path described in the PR rationale still silently degrades to an untyped object item.

[ErenAta16 (ErenAta16)]

Both points are fair. The eval one I've fixed; the coverage one I'd rather scope deliberately than quietly widen this PR.

On the eval surface. It executes, confirmed:

get_args(list["__import__('os').getcwd()"]) -> ("__import__('os').getcwd()",)
eval of that string                          -> '/sessions/.../work'   # ran

Emptying __builtins__ isn't enough on its own — globalns is the owning model's module namespace, so any callable defined or imported in that module stays reachable. A canary function in the module still fired with builtins stripped.

What the resolver actually needs is narrower than an expression evaluator: a forward reference names a type. So the string is now parsed first and only the grammar of a type expression is admitted — Name, Attribute, Subscript, Tuple, List, BinOp/BitOr, and Constant limited to str/int/bool/None. Anything else returns the annotation untouched and the existing fallback applies.

reference	admitted	eval runs
InnerForward	yes	yes
dict[str, InnerForward]	yes	yes
InnerForward | None	yes	yes
tuple[int, str]	yes	yes
__import__('os').getcwd()	no	no
_canary()	no	no
(lambda: 1)()	no	no
[x for x in ().__class__.__base__.__subclasses__()]	no	no

with the canary never firing. Two tests cover it: one asserts _resolve_nested_forward_refs(list["_canary()"], ...) returns the annotation unchanged with nothing executed, one parametrises the guard over the table above. Both go through the resolver directly rather than a model class, because pydantic resolves annotations at class-creation time and would raise before the resolver is reached.

On the kernel-function parameter path. That's right — kernel_function_decorator.py stores the raw annotation in type_object and kernel_parameter_metadata.py hands it to build(), which reaches handle_complex_type without passing through build_model_schema, so def f(items: list["InnerForward"]) still degrades to an untyped object. I'd rather not fold that in here: it needs the module globals of the function's module rather than a model's, so it's a different lookup with its own failure modes, and this PR is already carrying a security fix. Happy to open it as a follow-up if that's the preferred shape — say which and I'll do it.

One note on the test suite: test_unresolvable_forward_reference_falls_back_instead_of_raising fails for me locally on pydantic 2.13.4, and it fails identically on this branch before my change, so it isn't from either commit. Pydantic resolves the annotation at class-creation time there and raises NameError before the builder runs. Worth knowing if this repo's pin moves.

[ErenAta16 (ErenAta16)]

Same note as on #14242, so a reviewer does not spend time twice.

#14268 fixes the same KernelJsonSchemaBuilder forward-reference behaviour this PR does. It was opened on 4 August; this PR and its issue, #14239, are from 29 July.

No claim on which patch is better. If #14268 is preferred, tell me and I will close this one. The only thing worth avoiding is both being reviewed as if they were independent.

[Eduard van Valkenburg (eavanvalkenburg)]

Review finding (warning): Forward-reference resolution substitutes non-type globals

The evaluated value is always substituted into the rebuilt alias. If a reference name collides with a module, helper function, or plain global value, downstream schema generation can emit the wrong schema or raise instead of using the previous object fallback. Please only substitute values that are types or supported typing constructs; otherwise leave the original string or ForwardRef unchanged.

[ErenAta16 (ErenAta16)]

Reproduced on Python 3.10 and fixed in e5b2d38.

Measured with plain classes, so pydantic does not resolve anything at class creation. Each row is the schema build() gives for items:

nested reference names	main	previous head (2b99ec6)	now
a module (list["json"])	{"type": "array", "items": {"type": "object"}}	AttributeError: module 'json' has no attribute '__module__'	same as main
a function, def helper(count: int) -> str	same	items become an object schema with count and return properties	same as main
a list global	same	TypeError: unhashable type: 'list'	same as main
a string global set to "int"	same	{"type": "array", "items": {"type": "integer"}}	same as main

The evaluated value is now substituted only when it is a class or a typing construct that get_origin recognizes, such as dict[str, Inner], Optional[Inner] or Inner | None. Anything else returns the annotation unchanged, the same way an unresolvable name already did, so the existing fallback applies. New tests call the resolver directly for all four names, both as a bare string and as a ForwardRef (8 cases, all of which fail on the previous head). Another test checks that the typing constructs above still resolve, and a schema-level test pins the table.

I also need to correct two things I said earlier.

First, the bug this PR fixes only exists on Python 3.10. From 3.11 on, typing.get_type_hints evaluates strings nested in list[...] itself (I measured 3.11.14, 3.12.10 and 3.13.11). So on those versions list["Inner"] already gives the full schema on main, and the resolver never sees a string. The line in the description saying get_type_hints leaves list['Inner'] unchanged is only true on 3.10, which is the version the issue was reported on.

Second, in my earlier comment I blamed pydantic for the test_unresolvable_forward_reference_falls_back_instead_of_raising failure. That was wrong. The traceback shows get_type_hints raising NameError inside build_model_schema on 3.11+, for the same reason as above. The fallback that test checks only exists on 3.10, so d4f03cd skips it from 3.11 on, together with the new schema-level test. With both commits, tests/unit/schema gives 66 passed on 3.10, and 61 passed with 5 skipped on 3.11, 3.12 and 3.13 (Windows, pydantic 2.13.5).

This also means the problem you describe is already on main for 3.11+, regardless of this PR. On those versions get_type_hints substitutes the module or function itself, and main produces the same AttributeError, TypeError and wrong schemas as the middle column above, plus a NameError for a name that does not exist. This PR does not change that, for better or worse. Making the builder fall back on those versions too would need a guard after get_type_hints, not in the resolver, so I would rather do it as a separate change. I can open one if you want it.

Not blocking: Literal["int"] still becomes Literal[int], because int is a type. The builder currently renders both as {"type": "object"}, so the output does not change. I can make the resolver skip Literal if you prefer.

On CI: every failing check in the last run stopped at the setup-uv step with "API rate limit exceeded for installation" before any test ran, so that run does not tell us anything about this branch.

Related, since #14198 has the same finding open as a blocker: its two list-based cycle tests, test_build_with_self_referential_model_via_list and test_build_with_mutually_referential_models, fail on 3.10 at 96e6cff and pass on 3.11, 3.12 and 3.13, for the same get_type_hints reason described above. I merged this branch into #14198 locally to check how the two fit. There is one conflict in build_model_schema, resolved by keeping the #14198 loop and adding the _resolve_nested_forward_refs call to it. With that, both tests pass on 3.10 too, and tests/unit/schema gives 71 passed on 3.10 and 66 passed with 5 skipped on 3.11, 3.12 and 3.13. So the two changes work together: this one turns the nested string into the class, and #14198 handles the cycle once the class arrives. Whichever lands second needs that conflict resolved.

[ErenAta16 (ErenAta16)]

One more commit, 63e0178. Once the workflows on this PR are approved to run, the pre-commit job would fail: it runs bandit over python/semantic_kernel with --all-files, and the eval() in _resolve_nested_forward_refs is the only finding on this branch (B307), while main is clean. _is_type_expression already limits a reference to names, attribute access, subscription, |, tuples, lists and simple literals, so the resolver now evaluates those nodes directly, the same way _SafeFilterEvaluator in connectors/in_memory.py does, and no eval is left. A parametrized test checks that every admitted expression gives the same object eval would. bandit, mypy 1.19.1 and ruff 0.9.6 are clean, and tests/unit/schema is 76 passed on 3.10 and 71 passed with 5 skipped on 3.11, 3.12 and 3.13.