diff --git a/conversations/services/message_helpers/learning_progress_helpers.py b/conversations/services/message_helpers/learning_progress_helpers.py index 407d1fd4..474e7022 100644 --- a/conversations/services/message_helpers/learning_progress_helpers.py +++ b/conversations/services/message_helpers/learning_progress_helpers.py @@ -90,7 +90,6 @@ async def run_learning_progress_stream( learning_goals=learning_goals, tracking_prompt=tracking_prompt, llm=progress_llm, - max_tokens=2048, temperature=0.7, conversation_history_limit=80, bot_meta=bot_meta, diff --git a/core/services/dtos/builder.py b/core/services/dtos/builder.py index 17ae6299..1ddacf32 100644 --- a/core/services/dtos/builder.py +++ b/core/services/dtos/builder.py @@ -12,9 +12,8 @@ from .request_dto import LLMQueryRequest from .socratic_dto import SocraticConfig -# Minimum max_tokens for Socratic bots to prevent truncated responses -# This is a hotfix - the proper solution is to make this configurable per bot -SOCRATIC_MIN_MAX_TOKENS = 8000 +# Socratic models need room for a full answer; reasoning shares the output budget. +SOCRATIC_MIN_MAX_TOKENS = 32000 # Artifact-creating DARE tools require large output budgets because the entire # artifact payload (docx blocks, mermaid code, React component, etc.) is @@ -139,15 +138,16 @@ def from_message_data( ) bot_meta = message_data.get("bot_meta", {}) socratic_enabled = is_socratic_bots and not message_data.get("prompt_id") + if is_socratic_bots: + # Reflective interviews need earlier answers throughout the session. + # Zero selects all prior messages in the shared history loader. + context = replace(context, history_limit=0) # Get max_tokens from message_data with default max_tokens = message_data.get("max_tokens", 8000) - # HOTFIX: Enforce minimum max_tokens for Socratic bots to prevent truncated responses - # Socratic bot conversations created without explicit config get conversation.max_tokens=2048 - # which is too low for detailed tutoring responses - if is_socratic_bots and max_tokens < SOCRATIC_MIN_MAX_TOKENS: - max_tokens = SOCRATIC_MIN_MAX_TOKENS + if is_socratic_bots: + max_tokens = max(max_tokens, SOCRATIC_MIN_MAX_TOKENS) # The Artifacts toggle injects the full artifact toolkit (see the slug # union below). The artifact token floor must therefore apply whenever the diff --git a/core/services/learning_progress_service.py b/core/services/learning_progress_service.py index b4f15f68..07e1e3a2 100644 --- a/core/services/learning_progress_service.py +++ b/core/services/learning_progress_service.py @@ -24,7 +24,7 @@ async def assess_learning_progress( tracking_prompt: str, last_message: Message = None, llm: LLM = None, - max_tokens: int = 2048, + max_tokens: int = 32000, temperature: float = 0.7, conversation_history_limit: int = 80, # New: include bot metadata for subject/topic/title diff --git a/core/test_optional_learning_progress.py b/core/test_optional_learning_progress.py index 6309fa7b..dee851cf 100644 --- a/core/test_optional_learning_progress.py +++ b/core/test_optional_learning_progress.py @@ -26,7 +26,7 @@ def setUp(self): def run_progress(self, meta, service=None): async_to_sync(run_learning_progress_stream)( - conversation=SimpleNamespace(id=1), + conversation=SimpleNamespace(id=1, conversation_id="QA"), message_data={"bot_meta": meta}, message_obj=SimpleNamespace(id=2), llm=SimpleNamespace(id=1), @@ -98,6 +98,40 @@ async def collect(): self.assertTrue(any(text == "Making progress" for text, usage in result)) self.service._get_ai_service.assert_awaited_once() + def test_live_path_uses_larger_budget_and_preserves_progress_context(self): + from core.services.dtos import LLMStreamEvent + + captured = {} + + async def chunks(**kwargs): + captured.update(kwargs) + yield LLMStreamEvent.text_delta("Updated assessment") + + self.service._get_ai_service.return_value = SimpleNamespace( + stream_chat_completion=chunks + ) + with patch( + "conversations.services.message_helpers.learning_progress_helpers." + "update_message_learning_progress", + new_callable=AsyncMock, + ): + self.run_progress({"learning_goals": "Learn", "tracking_prompt": "Track"}) + + self.assertEqual(captured["max_tokens"], 32000) + self.service._get_conversation_history.assert_awaited_once_with( + SimpleNamespace(id=1, conversation_id="QA"), limit=80 + ) + self.service._get_previous_assessment.assert_awaited_once() + self.assertIn("User: hello", captured["messages"][1]["content"]) + self.assertIn("No assessment", captured["messages"][1]["content"]) + self.assertIn("Learn", captured["messages"][0]["content"]) + self.assertIn("Track", captured["messages"][0]["content"]) + self.assertEqual( + self.service._save_progress_assessment.call_args.kwargs["content"], + "Updated assessment", + ) + self.assertEqual(self.send.call_args.args[0]["type"], "progress_complete") + def test_provider_error_is_not_billed_or_saved_as_an_assessment(self): async def failed(**kwargs): yield "", {"error": True, "message": "Provider unavailable"} diff --git a/core/test_socratic_full_history.py b/core/test_socratic_full_history.py new file mode 100644 index 00000000..270ab98a --- /dev/null +++ b/core/test_socratic_full_history.py @@ -0,0 +1,91 @@ +"""Long Socratic interviews retain early answers in both prompt modes.""" + +from types import SimpleNamespace + +from asgiref.sync import async_to_sync +from django.test import SimpleTestCase, TransactionTestCase + +from conversations.constants import SenderType +from conversations.models import Conversation, Message +from conversations.services.message_validation_service import MessageValidationService +from core.services.dtos.builder import LLMQueryRequestBuilder +from core.services.llm_helpers.socratic_helpers import ( + build_advanced_socratic_messages, + build_classic_socratic_messages, +) +from users.constants import AuthSourceChoice +from users.models import User + + +def request_for( + conversation=None, *, platform=AuthSourceChoice.SOCRATIC_BOTS, **payload +): + return LLMQueryRequestBuilder.from_message_data( + message="Continue the interview", + conversation=conversation, + user=conversation.user if conversation else None, + llm=SimpleNamespace(provider="gemini"), + platform=platform, + message_data=MessageValidationService.validate_and_parse( + {"message": "Continue the interview", **payload} + ), + ) + + +class HistoryPolicyTests(SimpleTestCase): + def test_socratic_includes_full_history_despite_generic_or_stale_limits(self): + for payload in ({}, {"history_limit": 10}, {"history_limit": 20}): + with self.subTest(payload=payload): + self.assertEqual(request_for(**payload).context.history_limit, 0) + + def test_standard_chat_retains_requested_and_default_limits(self): + self.assertEqual(request_for(platform=None).context.history_limit, 10) + self.assertEqual( + request_for(platform=None, history_limit=30).context.history_limit, 30 + ) + + +class LongInterviewTests(TransactionTestCase): + def test_both_modes_include_every_prior_exchange(self): + user = User.objects.create_user(email="history@example.test", password="test") + conversation = Conversation.active_objects.create(user=user, title="Reflection") + prior = [] + for index in range(60): + text = f"Earlier interview message {index:03d}." + prior.append(text) + Message.active_objects.create( + conversation=conversation, + sender_type=( + SenderType.PLAYER if index % 2 == 0 else SenderType.AI_ASSISTANT + ), + message=text, + ) + # Production saves the current user message and assistant placeholder + # before constructing history; those two rows must not be replayed. + Message.active_objects.create( + conversation=conversation, + sender_type=SenderType.PLAYER, + message="Continue the interview", + ) + Message.active_objects.create( + conversation=conversation, + sender_type=SenderType.AI_ASSISTANT, + message="", + ) + for advanced, build in ( + (False, build_classic_socratic_messages), + (True, build_advanced_socratic_messages), + ): + with self.subTest(advanced=advanced): + request = request_for(conversation, is_advanced=advanced) + result = async_to_sync(build)(request, None) + prompt = "\n".join(message["content"] for message in result.messages) + for text in prior: + self.assertEqual(prompt.count(text), 1) + # Advanced mode also includes the question in its system prompt. + history = next( + s for s in result.context_trace["stages"] if s["kind"] == "history" + ) + self.assertEqual(history["turns"], 60) + self.assertEqual(history["limit"], 0) + self.assertIn("Continue the interview", prompt) diff --git a/core/test_socratic_token_budget.py b/core/test_socratic_token_budget.py new file mode 100644 index 00000000..e0c72175 --- /dev/null +++ b/core/test_socratic_token_budget.py @@ -0,0 +1,55 @@ +"""All Socratic models have headroom without raising regular chat limits.""" + +from types import SimpleNamespace + +from django.test import SimpleTestCase + +from conversations.services import message_validation_service +from core.services.dtos.builder import LLMQueryRequestBuilder +from core.services.gemini_service import GeminiService +from users.constants import AuthSourceChoice + + +class SocraticTokenBudgetTests(SimpleTestCase): + def request( + self, *, reasoning=True, platform=AuthSourceChoice.SOCRATIC_BOTS, **payload + ): + llm = SimpleNamespace( + identifier="gemini-3.8-flash", + provider="gemini", + is_reasoning=reasoning, + supports_effort=True, + default_effort="medium", + supports_temperature=True, + ) + return LLMQueryRequestBuilder.from_message_data( + message="List the transcript", + user=None, + llm=llm, + platform=platform, + message_data=message_validation_service.MessageValidationService.validate_and_parse( + {"message": "List the transcript", **payload} + ), + ) + + def test_reasoning_budget_applies_to_both_socratic_modes(self): + for advanced in (True, False): + request = self.request(is_advanced=advanced) + self.assertEqual(request.generation.max_tokens, 32000) + service = GeminiService(llm=request.llm, api_key="test-key") + config = service._build_generation_config( + request.generation.max_tokens, 0.7, None + ) + self.assertEqual(config.max_output_tokens, 35000) + + def test_higher_explicit_allowance_is_preserved(self): + self.assertEqual(self.request(max_tokens=40000).generation.max_tokens, 40000) + + def test_non_reasoning_socratic_uses_same_floor(self): + self.assertEqual(self.request(reasoning=False).generation.max_tokens, 32000) + + def test_regular_chat_retains_default_and_explicit_allowances(self): + self.assertEqual(self.request(platform=None).generation.max_tokens, 2048) + self.assertEqual( + self.request(platform=None, max_tokens=6000).generation.max_tokens, 6000 + )