diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f00876c..2cf7b5a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -37,6 +37,8 @@ jobs: run: pytest tests/unit/test_llm_service.py::test_create_idea_final_turn_conclusion_parsing tests/unit/test_llm_service.py::test_create_idea_final_turn_fallback_parsing -q - name: Run nested test discovery regression run: pytest tests/unit/test_nested_test_discovery_regression.py -q + - name: Run LLMChain LCEL migration regression + run: pytest tests/unit/test_llmchain_lcel_migration_regression.py -q - name: Run unit tests # Deselect known-broken tests that are tracked by their own open issues # (leave them for those issue-workers; don't scope-creep or race them). diff --git a/requirements-prod.txt b/requirements-prod.txt index 51a49f4..0972d5c 100644 --- a/requirements-prod.txt +++ b/requirements-prod.txt @@ -18,7 +18,7 @@ pydantic==2.11.3 huggingface_hub[inference,cli]>=0.21.0 boto3==1.34.36 python-multipart>=0.0.18 -langchain==0.2.17 -langchain-core==0.2.43 -langchain-groq==0.1.10 -langchain-text-splitters==0.2.4 +langchain==0.3.30 +langchain-core==0.3.86 +langchain-groq==0.2.5 +langchain-text-splitters==0.3.11 diff --git a/requirements.txt b/requirements.txt index 08b8025..b06b53c 100644 --- a/requirements.txt +++ b/requirements.txt @@ -10,8 +10,8 @@ boto3==1.34.36 python-multipart>=0.0.18 pytest==7.4.4 pytest-asyncio==0.23.4 -langchain==0.2.17 -langchain-core==0.2.43 -langchain-groq==0.1.10 -langchain-text-splitters==0.2.4 +langchain==0.3.30 +langchain-core==0.3.86 +langchain-groq==0.2.5 +langchain-text-splitters==0.3.11 httpx==0.27.0 diff --git a/services/llm_service.py b/services/llm_service.py index 38c0b51..9450d3c 100644 --- a/services/llm_service.py +++ b/services/llm_service.py @@ -12,9 +12,9 @@ import time import asyncio import traceback -from langchain.chains import LLMChain from langchain_groq import ChatGroq -from langchain.prompts import PromptTemplate +from langchain_core.prompts import PromptTemplate +from langchain_core.output_parsers import StrOutputParser from models.simulation import LLMLog # Import all prompt-related constants and functions @@ -310,12 +310,10 @@ async def create_idea(self, context: Dict[str, Any]) -> Dict[str, str]: # Always use moonshotai/kimi-k2-instruct via LangChain logger.info(f"Using Groq model via LangChain: {model_name}") llm = self._get_llm_instance(model_name) - chain = LLMChain( - llm=llm, - prompt=PromptTemplate.from_template("{prompt}")) + chain = PromptTemplate.from_template("{prompt}") | llm | StrOutputParser() start_time = time.time() - response = await chain.arun(prompt=formatted_prompt) + response = await chain.ainvoke({"prompt": formatted_prompt}) result = response response_time = time.time() - start_time model_used = model_name @@ -472,9 +470,9 @@ async def create_video_prompt(self, # Ensure the prompt template is correctly initialized for the chain # The input_variables should match what VIDEO_PROMPT_TEMPLATE expects, which is 'scenario' chain_prompt = PromptTemplate(input_variables=["scenario"], template=prompt_template) - chain = LLMChain(llm=groq_llm, prompt=chain_prompt) + chain = chain_prompt | groq_llm | StrOutputParser() - raw_llm_output = await chain.arun(scenario=scenario_text) + raw_llm_output = await chain.ainvoke({"scenario": scenario_text}) end_time = time.time() response_time = end_time - start_time logger.info( diff --git a/services/simulation_service.py b/services/simulation_service.py index 50cb765..c2e0bba 100644 --- a/services/simulation_service.py +++ b/services/simulation_service.py @@ -8,8 +8,6 @@ import logging from typing import Dict, Any, List, Optional -from langchain.chains import LLMChain -from langchain.prompts import PromptTemplate import json import traceback diff --git a/tests/unit/test_llm_service.py b/tests/unit/test_llm_service.py index 1578b15..ab1e2e4 100644 --- a/tests/unit/test_llm_service.py +++ b/tests/unit/test_llm_service.py @@ -1,6 +1,6 @@ import pytest import asyncio -from unittest.mock import AsyncMock, MagicMock, patch +from unittest.mock import AsyncMock, patch import json import os @@ -11,6 +11,19 @@ from models.simulation import LLMLog from prompts.scenario_generation_prompt import FINAL_CONCLUSION_EXAMPLE_JSON # For structure reference +from langchain_core.messages import AIMessage +from langchain_core.runnables import RunnableLambda + + +def _fake_llm_returning(canned_str): + """Offline stand-in for a ChatGroq LLM in an LCEL chain. + + Returned as the ``_get_llm_instance`` value so that + ``prompt | fake | StrOutputParser()`` composes and ``.ainvoke(...)`` + resolves to ``canned_str`` (StrOutputParser reads AIMessage.content). + """ + return RunnableLambda(lambda _prompt_value: AIMessage(content=canned_str)) + # Mock HuggingFaceService if it's a required dependency for LLMService instantiation class MockHuggingFaceService: def __init__(self, api_key, r2_service=None): @@ -61,9 +74,13 @@ async def test_create_idea_final_turn_conclusion_parsing(): ``` """ - # Patch the actual code path: create_idea drives the LLM via LangChain's - # LLMChain.arun (with a ChatGroq instance), NOT service.groq_client. - with patch("langchain.chains.LLMChain.arun", new_callable=AsyncMock, return_value=messy_llm_response_str): + # Patch the actual code path: create_idea drives the LLM via an LCEL chain + # (prompt | llm | StrOutputParser()) built on the _get_llm_instance ChatGroq + # instance, NOT service.groq_client. Mock that instance offline. + with patch.object( + service, "_get_llm_instance", + return_value=_fake_llm_returning(messy_llm_response_str), + ): generated_scenario = await service.create_idea(final_turn_context) # Assertions -- create_idea returns a single dict for the conclusion turn. @@ -114,8 +131,12 @@ async def test_create_idea_final_turn_fallback_parsing(): # Simulate LLM response with leading garbage and no markdown, forcing find {' '}' messy_llm_response_str = f"Some unexpected text before the JSON... \n {json.dumps(mock_llm_output_json)} \n ...and some after." - # Patch the actual code path: LLMChain.arun drives the LLM, not service.groq_client. - with patch("langchain.chains.LLMChain.arun", new_callable=AsyncMock, return_value=messy_llm_response_str): + # Patch the actual code path: the LCEL chain drives the LLM offline via the + # mocked _get_llm_instance, not service.groq_client. + with patch.object( + service, "_get_llm_instance", + return_value=_fake_llm_returning(messy_llm_response_str), + ): generated_scenario = await service.create_idea(final_turn_context) assert generated_scenario is not None diff --git a/tests/unit/test_llmchain_lcel_migration_regression.py b/tests/unit/test_llmchain_lcel_migration_regression.py new file mode 100644 index 0000000..6b9baf4 --- /dev/null +++ b/tests/unit/test_llmchain_lcel_migration_regression.py @@ -0,0 +1,157 @@ +"""Regression tests for issue #24: migrate LLMChain -> LCEL pipe syntax. + +`LLMChain` is deprecated in LangChain 0.2.x and removed in 0.3.x. This suite +locks in the acceptance criteria for the migration: + +1. Neither `services/llm_service.py` nor `services/simulation_service.py` + references the deprecated `LLMChain` class or the `.arun(` call form; the + LCEL `.ainvoke(` form is used instead. +2. `create_idea` drives the LLM through an offline-mocked LCEL chain + (`prompt | llm | StrOutputParser()`) and still returns a parsed dict. +3. `create_video_prompt` drives the LLM through an offline-mocked LCEL chain + and still returns its parsed structure. +4. The requirements files no longer pin the pre-0.3 `langchain==0.2` line. + +All LLM interaction is mocked offline via `_get_llm_instance`, so no network +call is ever made. +""" + +import inspect +import json +import re +from pathlib import Path + +import pytest +from unittest.mock import AsyncMock, patch + +from langchain_core.messages import AIMessage +from langchain_core.runnables import RunnableLambda + +import services.llm_service as llm_service_module +import services.simulation_service as simulation_service_module +from services.llm_service import LLMService + + +REPO_ROOT = Path(__file__).resolve().parents[2] + + +class MockHF: + def __init__(self, *a, **k): + pass + + +def _make_service(): + return LLMService(api_key="fake", huggingface_service=MockHF()) + + +def _fake_llm(canned_output): + """Offline LCEL LLM stand-in: prompt | fake | StrOutputParser() -> canned_output.""" + return RunnableLambda(lambda _prompt_value: AIMessage(content=canned_output)) + + +# --- Criterion 1: no LLMChain / no .arun( in migrated modules ----------------- + + +@pytest.mark.parametrize( + "module", [llm_service_module, simulation_service_module], + ids=["llm_service", "simulation_service"], +) +def test_module_source_has_no_llmchain(module): + source = inspect.getsource(module) + assert "LLMChain" not in source, ( + f"{module.__name__} still references the deprecated LLMChain" + ) + + +def test_llm_service_uses_lcel_not_arun(): + source = inspect.getsource(llm_service_module) + assert ".arun(" not in source, "llm_service still calls the deprecated chain.arun(...)" + assert ".ainvoke(" in source, "llm_service should drive LCEL chains via .ainvoke(...)" + assert "StrOutputParser" in source, ( + "llm_service should use StrOutputParser to preserve the string return of LCEL chains" + ) + + +def test_llm_service_imports_prompttemplate_from_core(): + source = inspect.getsource(llm_service_module) + assert "from langchain_core.prompts import PromptTemplate" in source + # The deprecated import path must be gone. + assert "from langchain.prompts import PromptTemplate" not in source + assert "from langchain.chains import LLMChain" not in source + + +# --- Criterion 2: create_idea via offline LCEL mock returns a dict ------------ + + +@pytest.mark.asyncio +async def test_create_idea_lcel_offline_returns_dict(): + service = _make_service() + service.log_callback = AsyncMock() + + final_turn_context = { + "simulation_history": "History...", + "current_turn_number": 6, + "max_turns": 6, + "previous_turn_number": 5, + "user_prompt_for_this_turn": "Final response", + } + payload = { + "situation_description": "LCEL migration works.", + "rationale": "Because the pipe composes.", + "grade": 88, + "grade_explanation": "Solid.", + } + messy = f"```json\n{json.dumps(payload)}\n```" + + with patch.object(service, "_get_llm_instance", return_value=_fake_llm(messy)): + result = await service.create_idea(final_turn_context) + + assert isinstance(result, dict), "create_idea should return a parsed dict" + assert result["situation_description"] == payload["situation_description"] + assert result["grade"] == payload["grade"] + assert "id" in result + + +# --- Criterion 3: create_video_prompt via offline LCEL mock ------------------- + + +@pytest.mark.asyncio +async def test_create_video_prompt_lcel_offline_returns_scenes(): + service = _make_service() + service.log_interaction = AsyncMock() + + canned = '{"scenes": ["s1", "s2", "s3", "s4"]}' + with patch.object(service, "_get_llm_instance", return_value=_fake_llm(canned)): + scenes = await service.create_video_prompt( + {"situation_description": "A crisis unfolds."}, + turn_number=1, + theme="classic", + ) + + assert scenes == ["s1", "s2", "s3", "s4"] + + +# --- Criterion 4: requirements no longer pin langchain 0.2 -------------------- + + +@pytest.mark.parametrize( + "req_name", ["requirements.txt", "requirements-prod.txt"], +) +def test_requirements_not_pinned_to_langchain_0_2(req_name): + text = (REPO_ROOT / req_name).read_text() + # Find an actual langchain meta-package pin line (not langchain-core etc., + # not comments). + for raw in text.splitlines(): + line = raw.strip() + if line.startswith("#") or not line: + continue + m = re.match(r"^langchain==([0-9]+\.[0-9]+)", line) + if m: + assert m.group(1) != "0.2", ( + f"{req_name} still pins the removed-LLMChain langchain==0.2 line: {line}" + ) + # Any explicit pin must be >= 0.3. + major, minor = (int(x) for x in m.group(1).split(".")) + assert (major, minor) >= (0, 3), ( + f"{req_name} pins langchain below 0.3: {line}" + ) diff --git a/tests/unit/test_theme.py b/tests/unit/test_theme.py index 99730e2..2e42734 100644 --- a/tests/unit/test_theme.py +++ b/tests/unit/test_theme.py @@ -6,10 +6,31 @@ across themes. """ import re -from unittest.mock import AsyncMock, MagicMock, patch +from unittest.mock import AsyncMock, patch import pytest +from langchain_core.messages import AIMessage +from langchain_core.runnables import RunnableLambda + + +def _capturing_fake_llm(canned_output): + """Offline LCEL LLM stand-in that records the formatted prompt string. + + Returned from a mocked ``_get_llm_instance`` so the real code builds + ``prompt | fake | StrOutputParser()``. ``captured['forwarded']`` holds the + fully-rendered prompt text (equivalent to what the old LLMChain.arun + received as its input variables), letting theme assertions inspect what + reached the LLM without any network call. + """ + captured = {"forwarded": None} + + def _run(prompt_value): + captured["forwarded"] = prompt_value.to_string() + return AIMessage(content=canned_output) + + return RunnableLambda(_run), captured + from models.simulation import ( ThemeType, SimulationState, @@ -142,16 +163,13 @@ async def test_video_prompt_includes_theme_visual_style(self): service = self._make_service() service.log_interaction = AsyncMock() - # Offline chain: mock the LLM instance and the LLMChain used inside. - fake_chain = MagicMock() - fake_chain.arun = AsyncMock( - return_value='{"scenes": ["a", "b", "c", "d"]}' - ) + # Offline LCEL chain: mock the LLM instance; the real code composes + # prompt | llm | StrOutputParser() around it. + fake_llm, captured = _capturing_fake_llm('{"scenes": ["a", "b", "c", "d"]}') visual_style = get_theme_instructions("scifi")["visual_style"] - with patch.object(service, "_get_llm_instance", return_value=MagicMock()), \ - patch("services.llm_service.LLMChain", return_value=fake_chain): + with patch.object(service, "_get_llm_instance", return_value=fake_llm): scenes = await service.create_video_prompt( {"situation_description": "A crisis unfolds."}, turn_number=1, @@ -161,8 +179,7 @@ async def test_video_prompt_includes_theme_visual_style(self): assert scenes == ["a", "b", "c", "d"] # The visual style must reach the LLM chain input. - chain_kwargs = fake_chain.arun.call_args.kwargs - forwarded = chain_kwargs.get("scenario", "") + forwarded = captured["forwarded"] or "" assert visual_style in forwarded, ( "Theme visual style must be forwarded to the video-prompt LLM chain." ) @@ -176,20 +193,16 @@ async def test_classic_theme_adds_no_visual_style(self): service = self._make_service() service.log_interaction = AsyncMock() - fake_chain = MagicMock() - fake_chain.arun = AsyncMock( - return_value='{"scenes": ["a", "b", "c", "d"]}' - ) + fake_llm, captured = _capturing_fake_llm('{"scenes": ["a", "b", "c", "d"]}') - with patch.object(service, "_get_llm_instance", return_value=MagicMock()), \ - patch("services.llm_service.LLMChain", return_value=fake_chain): + with patch.object(service, "_get_llm_instance", return_value=fake_llm): await service.create_video_prompt( {"situation_description": "A crisis unfolds."}, turn_number=1, theme="classic", ) - forwarded = fake_chain.arun.call_args.kwargs.get("scenario", "") + forwarded = captured["forwarded"] or "" assert "THEME VISUAL STYLE" not in forwarded @@ -211,16 +224,14 @@ async def test_scenario_flavor_reaches_llm_input(self): service = self._make_service() service.log_interaction = AsyncMock() - fake_chain = MagicMock() - fake_chain.arun = AsyncMock(return_value="{}") # parsing result is irrelevant + fake_llm, captured = _capturing_fake_llm("{}") # parsing result is irrelevant flavor = get_theme_instructions("scifi")["scenario_flavor"] - with patch.object(service, "_get_llm_instance", return_value=MagicMock()), \ - patch("services.llm_service.LLMChain", return_value=fake_chain): + with patch.object(service, "_get_llm_instance", return_value=fake_llm): await service.create_idea({"current_turn_number": 1, "theme": "scifi"}) - forwarded = fake_chain.arun.call_args.kwargs.get("prompt", "") + forwarded = captured["forwarded"] or "" assert flavor in forwarded, ( "Theme scenario flavor must be forwarded to the scenario-generation LLM." ) @@ -230,12 +241,10 @@ async def test_classic_theme_adds_no_scenario_flavor(self): service = self._make_service() service.log_interaction = AsyncMock() - fake_chain = MagicMock() - fake_chain.arun = AsyncMock(return_value="{}") + fake_llm, captured = _capturing_fake_llm("{}") - with patch.object(service, "_get_llm_instance", return_value=MagicMock()), \ - patch("services.llm_service.LLMChain", return_value=fake_chain): + with patch.object(service, "_get_llm_instance", return_value=fake_llm): await service.create_idea({"current_turn_number": 1, "theme": "classic"}) - forwarded = fake_chain.arun.call_args.kwargs.get("prompt", "") + forwarded = captured["forwarded"] or "" assert "THEME (setting)" not in forwarded