From b3978841851fb06916922317d802ac200d0d84c9 Mon Sep 17 00:00:00 2001 From: HAL9000 Date: Thu, 23 Apr 2026 12:46:55 +0000 Subject: [PATCH] fix(providers): resolve lint and unit test failures in OllamaProvider and MistralProvider - Fix import order in ollama_provider.py (langchain_community before langchain_core) - Remove duplicate shared step definitions from ollama_provider_steps.py - Remove duplicate shared step definitions from mistral_provider_steps.py The duplicate @given step definitions caused AmbiguousStep errors when running the full Behave test suite. Shared steps (provider domain inputs, plan generation graph setup) are already defined in openai_provider_steps.py and loaded by Behave for all feature files. --- features/steps/mistral_provider_steps.py | 81 +++---------------- features/steps/ollama_provider_steps.py | 65 +-------------- .../providers/llm/ollama_provider.py | 2 +- 3 files changed, 11 insertions(+), 137 deletions(-) diff --git a/features/steps/mistral_provider_steps.py b/features/steps/mistral_provider_steps.py index 38707435c..3dba0c48e 100644 --- a/features/steps/mistral_provider_steps.py +++ b/features/steps/mistral_provider_steps.py @@ -7,12 +7,6 @@ from unittest.mock import MagicMock, patch from behave import given, then, when -from cleveragents.domain.models.core import ( - Context, - OperationType, - Plan, - Project, -) from cleveragents.providers.llm.mistral_provider import MistralChatProvider @@ -85,61 +79,6 @@ def _setup_plan_generation_graph(context) -> MagicMock: return mock_graph_instance -@given("I have sample provider domain inputs") -def step_sample_provider_inputs(context): - context.project = MagicMock(spec=Project) - context.plan = MagicMock(spec=Plan) - context.plan.prompt = "Add placeholder coverage" - context.contexts = [MagicMock(spec=Context)] - context.contexts[0].content = "Sample context entry" - - -@given('the plan generation graph returns a generated change for "{file_path}"') -def step_plan_generation_returns_change(context, file_path): - change = { - "plan_id": 1, - "file_path": file_path, - "operation": OperationType.MODIFY.value, - "new_content": "# updated content", - } - context.plan_generation_state_override = { - "generated_changes": [change], - "validation_result": {"status": "PASS"}, - "error": None, - } - - -@given('the plan generation graph emits streaming nodes "{node_list}"') -def step_plan_generation_stream_nodes(context, node_list): - nodes = [node.strip() for node in node_list.split(",") if node.strip()] - state = getattr(context, "plan_generation_state_override", {}) or {} - events: list[dict[str, Any]] = [] - for node in nodes: - payload: dict[str, Any] = {"status": "completed"} - if node == "generate_plan" and isinstance(state.get("generated_changes"), list): - payload = { - "generated_changes": state["generated_changes"], - "status": "completed", - } - elif node == "validate" and isinstance(state.get("validation_result"), dict): - payload = { - "validation_result": state["validation_result"], - "status": "completed", - } - events.append({node: payload}) - context.plan_generation_stream_events = events - - -@given('the plan generation graph raises ValueError "{message}"') -def step_plan_generation_raises_value_error(context, message): - context.plan_generation_invoke_side_effect = ValueError(message) - - -@given('the plan generation graph raises RuntimeError "{message}"') -def step_plan_generation_runtime_error(context, message): - context.plan_generation_invoke_side_effect = RuntimeError(message) - - @given('I set the MISTRAL_API_KEY environment variable to "{api_key}"') def step_set_mistral_env_var(context, api_key): context.mistral_env_api_key = api_key @@ -152,12 +91,8 @@ def step_set_mistral_provider_kwargs(context, kwargs_string): context.mistral_provider_kwargs = _parse_kwargs_string(kwargs_string) -@given( - 'I create a Mistral chat provider with API key "{api_key}" and model "{model}"' -) -@when( - 'I create a Mistral chat provider with API key "{api_key}" and model "{model}"' -) +@given('I create a Mistral chat provider with API key "{api_key}" and model "{model}"') +@when('I create a Mistral chat provider with API key "{api_key}" and model "{model}"') def step_create_mistral_provider(context, api_key, model): patcher = patch("cleveragents.providers.llm.mistral_provider.ChatMistralAI") context.chat_mistral_patcher = patcher @@ -178,9 +113,7 @@ def step_create_mistral_provider(context, api_key, model): context.chat_mistral_instance = mock_chat_mistral_instance -@when( - 'I create a Mistral chat provider without explicit API key and model "{model}"' -) +@when('I create a Mistral chat provider without explicit API key and model "{model}"') def step_create_mistral_provider_from_env(context, model): patcher = patch("cleveragents.providers.llm.mistral_provider.ChatMistralAI") context.chat_mistral_patcher = patcher @@ -242,7 +175,9 @@ def step_stream_plan_generation(context): 'the Mistral provider should construct ChatMistralAI with api key "{api_key}" and model "{model}"' ) def step_assert_chat_mistral_constructor(context, api_key, model): - assert context.chat_mistral_call is not None, "ChatMistralAI should have been called" + assert context.chat_mistral_call is not None, ( + "ChatMistralAI should have been called" + ) call_args, call_kwargs = context.chat_mistral_call assert call_args == (), "ChatMistralAI should be called with keyword arguments" assert call_kwargs["api_key"] == api_key @@ -259,7 +194,9 @@ def step_assert_chat_mistral_constructor(context, api_key, model): 'the Mistral provider should include kwargs "{kwargs_string}" in the ChatMistralAI call' ) def step_assert_chat_mistral_kwargs(context, kwargs_string): - assert context.chat_mistral_call is not None, "ChatMistralAI should have been called" + assert context.chat_mistral_call is not None, ( + "ChatMistralAI should have been called" + ) _, call_kwargs = context.chat_mistral_call expected_kwargs = _parse_kwargs_string(kwargs_string) for key, value in expected_kwargs.items(): diff --git a/features/steps/ollama_provider_steps.py b/features/steps/ollama_provider_steps.py index 792439d88..2cfe0bbd2 100644 --- a/features/steps/ollama_provider_steps.py +++ b/features/steps/ollama_provider_steps.py @@ -6,12 +6,6 @@ from unittest.mock import MagicMock, patch from behave import given, then, when -from cleveragents.domain.models.core import ( - Context, - OperationType, - Plan, - Project, -) from cleveragents.providers.llm.ollama_provider import OllamaChatProvider @@ -84,61 +78,6 @@ def _setup_plan_generation_graph(context) -> MagicMock: return mock_graph_instance -@given("I have sample provider domain inputs") -def step_sample_provider_inputs(context): - context.project = MagicMock(spec=Project) - context.plan = MagicMock(spec=Plan) - context.plan.prompt = "Add placeholder coverage" - context.contexts = [MagicMock(spec=Context)] - context.contexts[0].content = "Sample context entry" - - -@given('the plan generation graph returns a generated change for "{file_path}"') -def step_plan_generation_returns_change(context, file_path): - change = { - "plan_id": 1, - "file_path": file_path, - "operation": OperationType.MODIFY.value, - "new_content": "# updated content", - } - context.plan_generation_state_override = { - "generated_changes": [change], - "validation_result": {"status": "PASS"}, - "error": None, - } - - -@given('the plan generation graph emits streaming nodes "{node_list}"') -def step_plan_generation_stream_nodes(context, node_list): - nodes = [node.strip() for node in node_list.split(",") if node.strip()] - state = getattr(context, "plan_generation_state_override", {}) or {} - events: list[dict[str, Any]] = [] - for node in nodes: - payload: dict[str, Any] = {"status": "completed"} - if node == "generate_plan" and isinstance(state.get("generated_changes"), list): - payload = { - "generated_changes": state["generated_changes"], - "status": "completed", - } - elif node == "validate" and isinstance(state.get("validation_result"), dict): - payload = { - "validation_result": state["validation_result"], - "status": "completed", - } - events.append({node: payload}) - context.plan_generation_stream_events = events - - -@given('the plan generation graph raises ValueError "{message}"') -def step_plan_generation_raises_value_error(context, message): - context.plan_generation_invoke_side_effect = ValueError(message) - - -@given('the plan generation graph raises RuntimeError "{message}"') -def step_plan_generation_runtime_error(context, message): - context.plan_generation_invoke_side_effect = RuntimeError(message) - - @given('I set the Ollama provider extra kwargs "{kwargs_string}"') def step_set_ollama_provider_kwargs(context, kwargs_string): context.ollama_provider_kwargs = _parse_kwargs_string(kwargs_string) @@ -147,9 +86,7 @@ def step_set_ollama_provider_kwargs(context, kwargs_string): @given( 'I create an Ollama chat provider with model "{model}" and base_url "{base_url}"' ) -@when( - 'I create an Ollama chat provider with model "{model}" and base_url "{base_url}"' -) +@when('I create an Ollama chat provider with model "{model}" and base_url "{base_url}"') def step_create_ollama_provider(context, model, base_url): patcher = patch("cleveragents.providers.llm.ollama_provider.ChatOllama") context.chat_ollama_patcher = patcher diff --git a/src/cleveragents/providers/llm/ollama_provider.py b/src/cleveragents/providers/llm/ollama_provider.py index 4f313215e..cbc548394 100644 --- a/src/cleveragents/providers/llm/ollama_provider.py +++ b/src/cleveragents/providers/llm/ollama_provider.py @@ -2,8 +2,8 @@ from __future__ import annotations from typing import Any, cast -from langchain_core.language_models import BaseLanguageModel from langchain_community.chat_models import ChatOllama +from langchain_core.language_models import BaseLanguageModel from cleveragents.providers.llm.langchain_chat_provider import LangChainChatProvider