genai: Fixed nested pydantic structures recursion

[Shahar-Y]

PR Description

While working with gregpr07/browser-use, @MahlerTom and I found several bugs in langchain handling of nested Pydantic structured in with_structured_output. Specifically, the issues occurred in _get_properties_from_schema method in libs\genai\langchain_google_genai\_function_utils.py.

1. The current implementation did not recursively handle cases of nested glm.Type.OBJECT and only took care of the first layer.
2. In cases where pydantic fields are required (no default value is provided), the method did not properly add these fields to the required list.
3. In cases where an object of type glm.Type.OBJECT had empty properties, there was an error that properties of type glm.Type.OBJECT must not be empty.

Relevant issues

Fixes #657,
May be related to langchain-ai/langchain#24225

Type

🐛 Bug Fix

Changes

As mentioned above:

1. The previous implementation did not recursively handle cases of nested glm.Type.OBJECT and only took care of the first layer. We fixed the recursion with:

if properties_item.get("type_") == glm.Type.OBJECT:
    if v.get('anyOf') and isinstance(v['anyOf'], list) and isinstance(v['anyOf'][0], dict):
        v = v['anyOf'][0]
    v_properties = v.get("properties")
    if v_properties:
        properties_item["properties"] = _get_properties_from_schema_any(v_properties)

However, we found that when sending nested pedantic objects with required fields as expected output structure, like the case in the following method from gregpr07/browser-use below:

@time_execution_async('--get_next_action')
	async def get_next_action(self, input_messages: list[BaseMessage]) -> AgentOutput:
		"""Get next action from LLM based on current state"""

		structured_llm = self.llm.with_structured_output(self.AgentOutput, include_raw=True)
		response: dict[str, Any] = await structured_llm.ainvoke(input_messages)  # type: ignore

		parsed: AgentOutput = response['parsed']

		self._log_response(parsed)
		self.n_steps += 1

		return parsed

Here, when we observed the response['parsed'] was often None and response['parsing_error'] included validation errors for missing fields that were required.

2. To fix that error, we realized we needed to add the required list when parsing the pydantic objects. We added the nested if to our fix:

if properties_item.get("type_") == glm.Type.OBJECT:
    if v.get('anyOf') and isinstance(v['anyOf'], list) and isinstance(v['anyOf'][0], dict):
        v = v['anyOf'][0]
    v_properties = v.get("properties")
    if v_properties:
        properties_item["properties"] = _get_properties_from_schema_any(v_properties)
        if isinstance(v_properties, dict):
            properties_item["required"] = [
                k for k, v in v_properties.items() if "default" not in v
            ]

But still got this error:

Unexpected error: Invalid argument provided to Gemini: 400 * GenerateContentRequest.tools[0].function_declarations[0].parameters.properties[action].properties[go_back].properties: should be non-empty for OBJECT type

This was because in cases where an object of type glm.Type.OBJECT had empty properties, as was the case with the 'go_back' action in gregpr07/browser-use, there was an error that properties of type glm.Type.OBJECT must not be empty. We changed that by adding dummy type in such cases adding the last else statement:

if properties_item.get("type_") == glm.Type.OBJECT:
    if v.get('anyOf') and isinstance(v['anyOf'], list) and isinstance(v['anyOf'][0], dict):
        v = v['anyOf'][0]
    v_properties = v.get("properties")
    if v_properties:
        properties_item["properties"] = _get_properties_from_schema_any(v_properties)
        if isinstance(v_properties, dict):
            properties_item["required"] = [
                k for k, v in v_properties.items() if "default" not in v
            ]
    else:
        # Providing dummy type for object without properties
        properties_item["type_"] = glm.Type.STRING

Testing

Tested on the code described in #657

Note

• PR title and description are appropriate
• Necessary tests and documentation have been added
• Lint and tests pass successfully
• The following additional guidelines are adhered to:
  • Optional dependencies are imported within functions
  • No unnecessary dependencies added to pyproject.toml files (except those required for unit tests)
  • PR doesn't touch more than one package
  • Changes are backwards compatible

[MagMueller]

Great work!

[Shahar-Y]

Hi @lkuligin , could you please review this pull request when you get a chance?
Let me know if there's anything that needs clarification. Thanks!

[lkuligin]

thanks for your contribution! could you add a unit test for a nested Pydantic, please?

[Shahar-Y]

thanks for your contribution! could you add a unit test for a nested Pydantic, please?

Hi, @MahlerTom and I added several tests regarding the PR.
Please let us know if anything else is missing 👍