-
Notifications
You must be signed in to change notification settings - Fork 2
fix: Remove evaluation metric key from schema which failed on some LLMs #105
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
f8c6eba
49f5e2e
916df2a
1d78f8a
75f75cf
1ed23cf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -83,44 +83,43 @@ async def invoke_structured_model( | |
| :param response_structure: Dictionary defining the output structure | ||
| :return: StructuredResponse containing the structured data | ||
| """ | ||
| structured_response = StructuredResponse( | ||
| data={}, | ||
| raw_response='', | ||
| metrics=LDAIMetrics( | ||
| success=False, | ||
| usage=TokenUsage(total=0, input=0, output=0), | ||
| ), | ||
| ) | ||
| try: | ||
| langchain_messages = LangChainProvider.convert_messages_to_langchain(messages) | ||
| structured_llm = self._llm.with_structured_output(response_structure) | ||
| structured_llm = self._llm.with_structured_output(response_structure, include_raw=True) | ||
| response = await structured_llm.ainvoke(langchain_messages) | ||
|
|
||
| if not isinstance(response, dict): | ||
| log.warning( | ||
| f'Structured output did not return a dict. ' | ||
| f'Got: {type(response)}' | ||
| ) | ||
| return StructuredResponse( | ||
| data={}, | ||
| raw_response='', | ||
| metrics=LDAIMetrics( | ||
| success=False, | ||
| usage=TokenUsage(total=0, input=0, output=0), | ||
| ), | ||
| ) | ||
|
|
||
| return StructuredResponse( | ||
| data=response, | ||
| raw_response=str(response), | ||
| metrics=LDAIMetrics( | ||
| success=True, | ||
| usage=TokenUsage(total=0, input=0, output=0), | ||
| ), | ||
| ) | ||
| return structured_response | ||
|
|
||
| raw_response = response.get('raw') | ||
| if raw_response is not None: | ||
| if hasattr(raw_response, 'content'): | ||
| structured_response.raw_response = raw_response.content | ||
| structured_response.metrics = LangChainProvider.get_ai_metrics_from_response(raw_response) | ||
|
|
||
| if response.get('parsing_error'): | ||
| log.warning(f'LangChain structured model invocation had a parsing error') | ||
| structured_response.metrics.success = False | ||
| return structured_response | ||
|
|
||
| structured_response.metrics.success = True | ||
| structured_response.data = response.get('parsed') or {} | ||
| return structured_response | ||
| except Exception as error: | ||
| log.warning(f'LangChain structured model invocation failed: {error}') | ||
|
|
||
| return StructuredResponse( | ||
| data={}, | ||
| raw_response='', | ||
| metrics=LDAIMetrics( | ||
| success=False, | ||
| usage=TokenUsage(total=0, input=0, output=0), | ||
| ), | ||
| ) | ||
| return structured_response | ||
|
|
||
| def get_chat_model(self) -> BaseChatModel: | ||
| """ | ||
|
|
@@ -135,18 +134,18 @@ def map_provider(ld_provider_name: str) -> str: | |
| """ | ||
| Map LaunchDarkly provider names to LangChain provider names. | ||
|
|
||
| This method enables seamless integration between LaunchDarkly's standardized | ||
| provider naming and LangChain's naming conventions. | ||
|
|
||
| :param ld_provider_name: LaunchDarkly provider name | ||
| :return: LangChain-compatible provider name | ||
| """ | ||
| lowercased_name = ld_provider_name.lower() | ||
| # Bedrock is the only provider that uses "provider:model_family" (e.g. Bedrock:Anthropic). | ||
| if lowercased_name.startswith('bedrock:'): | ||
| return 'bedrock_converse' | ||
|
|
||
| mapping: Dict[str, str] = { | ||
| 'gemini': 'google-genai', | ||
| 'bedrock': 'bedrock_converse', | ||
| } | ||
|
|
||
| return mapping.get(lowercased_name, lowercased_name) | ||
|
|
||
| @staticmethod | ||
|
|
@@ -169,7 +168,13 @@ def get_ai_metrics_from_response(response: BaseMessage) -> LDAIMetrics: | |
| """ | ||
| # Extract token usage if available | ||
| usage: Optional[TokenUsage] = None | ||
| if hasattr(response, 'response_metadata') and response.response_metadata: | ||
| if hasattr(response, 'usage_metadata') and response.usage_metadata: | ||
| usage = TokenUsage( | ||
| total=response.usage_metadata.get('total_tokens', 0), | ||
| input=response.usage_metadata.get('input_tokens', 0), | ||
| output=response.usage_metadata.get('output_tokens', 0), | ||
| ) | ||
| if not usage and hasattr(response, 'response_metadata') and response.response_metadata: | ||
| token_usage = response.response_metadata.get('tokenUsage') or response.response_metadata.get('token_usage') | ||
| if token_usage: | ||
| usage = TokenUsage( | ||
|
|
@@ -227,10 +232,15 @@ def create_langchain_model(ai_config: AIConfigKind) -> BaseChatModel: | |
|
|
||
| model_name = model_dict.get('name', '') | ||
| provider = provider_dict.get('name', '') | ||
| parameters = model_dict.get('parameters') or {} | ||
| parameters = dict(model_dict.get('parameters') or {}) | ||
| mapped_provider = LangChainProvider.map_provider(provider) | ||
|
|
||
| # Bedrock requires the foundation provider (e.g. Bedrock:Anthropic) passed in | ||
| # parameters separately from model_provider, which is used for LangChain routing. | ||
| if mapped_provider == 'bedrock_converse' and 'provider' not in parameters: | ||
| parameters['provider'] = provider | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bedrock provider parameter passes wrong format to LangChainHigh Severity The |
||
| return init_chat_model( | ||
| model_name, | ||
| model_provider=LangChainProvider.map_provider(provider), | ||
| model_provider=mapped_provider, | ||
| **parameters, | ||
| ) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,79 +1,53 @@ | ||
| """Internal class for building dynamic evaluation response schemas.""" | ||
| """Internal class for building evaluation response schemas.""" | ||
|
|
||
| from typing import Any, Dict, Optional | ||
| from typing import Any, Dict | ||
|
|
||
|
|
||
| class EvaluationSchemaBuilder: | ||
| """ | ||
| Internal class for building dynamic evaluation response schemas. | ||
| Internal class for building evaluation response schemas. | ||
| Not exported - only used internally by Judge. | ||
| Schema is a fixed shape: one "evaluation" object with score and reasoning. | ||
| The judge config's evaluation_metric_key is only used when keying the result, | ||
| not in the schema. | ||
| """ | ||
|
|
||
| @staticmethod | ||
| def build(evaluation_metric_key: Optional[str]) -> Optional[Dict[str, Any]]: | ||
| def build() -> Dict[str, Any]: | ||
| """ | ||
| Build an evaluation response schema from evaluation metric key. | ||
| Build the evaluation response schema. No parameters; the schema is | ||
| always the same. The judge keys the parsed result by its config's | ||
| evaluation_metric_key. | ||
|
|
||
| :param evaluation_metric_key: Evaluation metric key, or None if not available | ||
| :return: Schema dictionary for structured output, or None if evaluation_metric_key is None | ||
| """ | ||
| if not evaluation_metric_key: | ||
| return None | ||
| In practice the model returns JSON like: | ||
| {"evaluation": {"score": 0.85, "reasoning": "The response is accurate."}} | ||
|
|
||
| :return: Schema dictionary for structured output | ||
| """ | ||
| return { | ||
| 'title': 'EvaluationResponse', | ||
| 'description': f"Response containing evaluation results for {evaluation_metric_key} metric", | ||
| 'description': 'Response containing an evaluation (score and reasoning).', | ||
| 'type': 'object', | ||
| 'properties': { | ||
| 'evaluations': { | ||
| 'evaluation': { | ||
| 'type': 'object', | ||
| 'description': ( | ||
| f"Object containing evaluation results for " | ||
| f"{evaluation_metric_key} metric" | ||
| ), | ||
| 'properties': EvaluationSchemaBuilder._build_key_properties(evaluation_metric_key), | ||
| 'required': [evaluation_metric_key], | ||
| 'description': 'The evaluation result.', | ||
| 'properties': { | ||
| 'score': { | ||
| 'type': 'number', | ||
| 'minimum': 0, | ||
| 'maximum': 1, | ||
| 'description': 'Score between 0.0 and 1.0.', | ||
| }, | ||
| 'reasoning': { | ||
| 'type': 'string', | ||
| 'description': 'Reasoning behind the score.', | ||
| }, | ||
| }, | ||
| 'required': ['score', 'reasoning'], | ||
| 'additionalProperties': False, | ||
| }, | ||
| }, | ||
| 'required': ['evaluations'], | ||
| 'additionalProperties': False, | ||
| } | ||
|
|
||
| @staticmethod | ||
| def _build_key_properties(evaluation_metric_key: str) -> Dict[str, Any]: | ||
| """ | ||
| Build properties for a single evaluation metric key. | ||
|
|
||
| :param evaluation_metric_key: Evaluation metric key | ||
| :return: Dictionary of properties for the key | ||
| """ | ||
| return { | ||
| evaluation_metric_key: EvaluationSchemaBuilder._build_key_schema(evaluation_metric_key) | ||
| } | ||
|
|
||
| @staticmethod | ||
| def _build_key_schema(key: str) -> Dict[str, Any]: | ||
| """ | ||
| Build schema for a single evaluation metric key. | ||
|
|
||
| :param key: Evaluation metric key | ||
| :return: Schema dictionary for the key | ||
| """ | ||
| return { | ||
| 'type': 'object', | ||
| 'properties': { | ||
| 'score': { | ||
| 'type': 'number', | ||
| 'minimum': 0, | ||
| 'maximum': 1, | ||
| 'description': f'Score between 0.0 and 1.0 for {key}', | ||
| }, | ||
| 'reasoning': { | ||
| 'type': 'string', | ||
| 'description': f'Reasoning behind the score for {key}', | ||
| }, | ||
| }, | ||
| 'required': ['score', 'reasoning'], | ||
| 'required': ['evaluation'], | ||
| 'additionalProperties': False, | ||
| } |


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exception handler may return success=True after partial mutation
Low Severity
The
excepthandler returns the shared mutablestructured_responsewithout resettingmetrics.success. After line 110,get_ai_metrics_from_responsereplaces the metrics withsuccess=True. If any exception occurs between that point and the explicit returns, the handler returns a response indicating success despite the failure. The previous code defensively created a freshStructuredResponsewithsuccess=Falsein the handler.Additional Locations (1)
packages/ai-providers/server-ai-langchain/src/ldai_langchain/langchain_provider.py#L109-L110