fix(langgraph): add description to OCI structured-output schema - #234
fix(langgraph): add description to OCI structured-output schema#234AmirF194 wants to merge 2 commits into
Conversation
LlmNodeExecutor built the LangGraph structured-output JSON schema with only "title", "type" and "properties". langchain_oci's CohereProvider requires "description" to also be present and raises ValueError for any dict missing it, so every Cohere-family OciGenAiConfig model with a non-string LlmNode output failed at construction time. The generic provider (Meta/OpenAI/Gemini) only needs "title" and "properties", so it was unaffected, which is why this depends on model_id / provider choice. Fixes oracle#232 Signed-off-by: Amir Fathi <amirfathi.me@gmail.com>
|
Thank you for your pull request and welcome to our community! To contribute, please sign the Oracle Contributor Agreement (OCA).
To sign the OCA, please create an Oracle account and sign the OCA in Oracle's Contributor Agreement Application. When signing the OCA, please provide your GitHub username. After signing the OCA and getting an OCA approval from Oracle, this PR will be automatically updated. If you are an Oracle employee, please make sure that you are a member of the main Oracle GitHub organization, and your membership in this organization is public. |
… unit test Per review: the previous test built a real ChatOCIGenAI per provider via AgentSpecToLangGraphConverter, which needs a throwaway OCI API-key config and does not directly assert on the schema LlmNodeExecutor builds. Replace it with a unit test that monkeypatches _node_execution.BaseChatModel with a fake that captures the with_structured_output schema, and asserts its description and properties directly, near LlmNodeExecutor. Signed-off-by: Amir Fathi <amirfathi.me@gmail.com>
|
Pushed 3694bb6: replaced the parametrized OCI test with a unit test near LlmNodeExecutor that monkeypatches |
LGTM. To proceed with the next steps to merge this PR, I would need you to sign the Oracle Contributor Agreement (OCA). Please check the comment at #234 (comment) |
|
Closing this one. I've decided not to sign the Oracle Contributor Agreement, so it can't go any further, and it isn't fair to leave it sitting in your queue. Thanks for the review time, the fix itself is small if anyone wants to pick it up: the OCI structured-output schema needs a description field or the LangGraph path drops it. |
Root cause
LlmNodeExecutor.__init__(_node_execution.py) builds the LangGraph structured-outputschema for an
LlmNodeas a bare dict with onlytitle,typeandproperties, thenpasses it to
self.llm.with_structured_output(json_schema).For an
OciGenAiConfig-backed model, that call reacheslangchain_oci'sChatOCIGenAI.bind_tools->convert_to_oci_tool. The generic provider (Meta/OpenAI/Geminimodel ids) only requires
titleandproperties, so it happens to accept this dict. TheCohere provider (
cohere.*model ids) additionally requiresdescription:so any Cohere-family model configured with a structured (non-single-string)
LlmNodeoutput fails at executor construction time, before any request reaches OCI, matching the
report on #232.
Fix
Add a static
descriptionkey to the schema dict, mirroring the existingtitlecomment.Verification
python:3.12-slimcontainer (pyagentspecinstalled editable from this checkout, provenance checked via
module.__file__):constructing
LlmNodeExecutorfor anOciGenAiConfig(model_id="cohere.command-a-...")node with two non-string outputs raises the
ValueErrorabove; a Meta/generic model iddoes not, which is why this is provider-specific rather than universal.
test_llmnodeexecutor_structured_output_supported_across_providers(
test_ocigenai_conversion.py), parametrized over a Cohere and a Meta model id, using athrowaway locally-generated OCI API key so it needs no real account or network call. It
fails on
mainfor the Cohere case and passes on this branch for both, run both ways inthe same container.
black,isort,flake8 --select C801(including the copyright check) andbandit -c bandit.yaml -r pyagentspec/all clean on the two changed files.mypyis not run here:the CI job excludes
pyagentspec/src/pyagentspec/adapters.pytest tests(both changed and unchanged) withSKIP_LLM_TESTS=1, matching thisrepo's CI env: same 10 failed / 1042 passed / 666 skipped / 26 errors before and after this
diff, all pre-existing and traced to optional extras not installed in this container
(
langchain_ollama,langgraph_swarm), not to this change.OciGenAiConfig(including the pre-existing
test_reverse_convert_chatocigenai_to_agentspec), the new testis skipped under the project's standard
SKIP_LLM_TESTS=1CI run; I could only exercise itlocally with that flag unset.
Fixes #232