fix(providers): request JSON-schema structured output for models that reject forced tool calls - #529
stefanoamorelli wants to merge 1 commit into
Conversation
945b6fc to
ab87ba8
Compare
rng1995
left a comment
There was a problem hiding this comment.
[SkillSpector Review]
Reviewed current head ab87ba8bdfd37bd2fa2c15d32b009b4387ecfd49. The capability hook fixes the forced-tool failure for the providers that implement it, but the same documented failure remains for supported Bedrock models because BedrockProvider has no structured-output-method selection. Add Bedrock model-family/ID/ARN recognition and regressions, or make capability selection generic so semantic analysis does not silently disappear on those models.
All required checks pass, but this incomplete provider fix and mergeStateStatus=BEHIND block merging.
| return {"method": override} | ||
| if provider is None: | ||
| provider = get_active_provider() | ||
| hint = getattr(provider, "structured_output_method", None) |
There was a problem hiding this comment.
[P1] Apply JSON-schema selection to affected Bedrock models
Only providers implementing structured_output_method reach this branch. BedrockProvider has no hook even though SkillSpector uses ChatBedrockConverse, and supported Bedrock model families can reject forced toolChoice with the same 400 this PR is intended to prevent; the pinned LangChain AWS integration supports json_schema but defaults to function calling. Add robust Bedrock family/model-ID/ARN capability selection plus tests (and registry metadata if appropriate), or move this decision to a generic capability layer.
with_structured_output defaults to a forced tool call, which some model families answer with HTTP 400, so every semantic batch failed and reports silently lost the LLM findings. The two direct-API providers bind json_schema for those models (registry entry or model name). Bedrock has no JSON-schema output for them, so it leaves toolChoice at auto, asks for the tool call in the prompt and retries prose. SKILLSPECTOR_STRUCTURED_OUTPUT_METHOD overrides the method for any provider. Signed-off-by: Stefano Amorelli <stefano@amorelli.tech>
ab87ba8 to
9e1de33
Compare
LangChain forces a tool call when binding the analyzers' response schema, and the current model generation answers that with HTTP 400, so every semantic batch failed and reports silently fell back to static analysis.
I route those models to the native JSON-schema response format on the two direct-API providers. Bedrock rejects that format for them as well, so there I keep toolChoice at auto, ask for the tool call in the prompt, and retry a prose answer. langchain-aws made the same choice in 1.7.6. SKILLSPECTOR_STRUCTURED_OUTPUT_METHOD overrides the method for any provider.
Related history: #66, #76, #132.