Skip to content

fix: correct params in llm api call #223

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.

Sign up for GitHub

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

Merged
mraman-2U merged 5 commits into from
Apr 27, 2026
Merged

fix: correct params in llm api call #223

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
088ee20
fix: correct placement for conversation_id param
krishatcher Apr 24, 2026
2c1f14d
fix: avoid potential collisions
krishatcher Apr 24, 2026
7acff90
fix: correct formatting error
krishatcher Apr 24, 2026
15c359a
fix: reference correct variables
krishatcher Apr 24, 2026
dcd4065
chore: update changelog for version 4.11.5
krishatcher Apr 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ Change Log

Unreleased
**********
4.11.5 - 2026-04-24
*******************
* Fixes a bug where the Conversation-ID value was not being properly included in the API call to the Xpert backend, which caused learning assistant messages to not be included in downstream reports and analysis.

4.11.4 - 2026-04-17
*******************
* To include additional, optional parameters so that learning assistant messages are included in downstream reports and analysis.
Expand Down
2 changes: 1 addition & 1 deletion learning_assistant/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@
Plugin for a learning assistant backend, intended for use within edx-platform.
"""

__version__ = '4.11.4'
__version__ = '4.11.5'

default_app_config = 'learning_assistant.apps.LearningAssistantConfig' # pylint: disable=invalid-name
13 changes: 7 additions & 6 deletions learning_assistant/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ def get_reduced_message_list(prompt_template, message_list):
return new_message_list


def create_request_body(prompt_template, message_list, user_id):
def create_request_body(prompt_template, message_list, user_id, convo_id):
"""
Form request body to be passed to the chat endpoint.
"""
Expand All @@ -73,6 +73,7 @@ def create_request_body(prompt_template, message_list, user_id):
'system_message': prompt_template,
'messages': messages,
'external_id': str(user_id),
'conversation_id': convo_id,
}

return response_body
Expand All @@ -89,12 +90,12 @@ def create_conversation_history_id(user_id, course_run_id):
Returns:
* str: the conversation history id
"""
convo_hist_seed = f'{user_id}_{course_run_id}'
convo_hist_seed = json.dumps([str(user_id), str(course_run_id)], separators=(',', ':'))

# Use a pre-defined namespace (e.g., DNS, URL, OID, or X500)
namespace = uuid.NAMESPACE_URL

# Generate the same UUID every time for the same string
# Generate the same UUID every time for the same user/course_run pair
deterministic_uuid = uuid.uuid5(namespace, convo_hist_seed)

return str(deterministic_uuid)
Expand All @@ -107,12 +108,12 @@ def get_chat_response(prompt_template, message_list, user_id, course_run_id):
completion_endpoint = getattr(settings, 'CHAT_COMPLETION_API_V2', None) if v2_endpoint_enabled() \
else getattr(settings, 'CHAT_COMPLETION_API', None)
if completion_endpoint:
conversation_history_id = create_conversation_history_id(user_id, course_run_id)
headers = {'Content-Type': 'application/json', 'Conversation-History-ID': conversation_history_id}
headers = {'Content-Type': 'application/json'}
connect_timeout = getattr(settings, 'CHAT_COMPLETION_API_CONNECT_TIMEOUT', 1)
read_timeout = getattr(settings, 'CHAT_COMPLETION_API_READ_TIMEOUT', 15)

body = create_request_body(prompt_template, message_list, user_id)
conversation_history_id = create_conversation_history_id(user_id, course_run_id)
body = create_request_body(prompt_template, message_list, user_id, conversation_history_id)
Comment on lines +115 to +116

Copilot AI Apr 24, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

create_request_body() now requires convo_id, but the v1 request body ignores it. This forces get_chat_response() to always compute a conversation id even when the v2 endpoint is disabled, and makes create_request_body() harder to reuse correctly. Consider making convo_id optional (e.g., defaulting to None) and only generating/passing it when building the v2 payload.

Copilot uses AI. Check for mistakes.

try:
response = requests.post(
Expand Down
10 changes: 5 additions & 5 deletions tests/test_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,12 +108,10 @@ def test_timeout(self, exception, mock_requests):
def test_post_request_structure(self, mock_requests):
mock_requests.post = MagicMock()

mock_convo_hist_id = str(uuid.uuid5(uuid.NAMESPACE_URL, f'{self.user_id}_{self.course_id}'))

completion_endpoint = settings.CHAT_COMPLETION_API
connect_timeout = settings.CHAT_COMPLETION_API_CONNECT_TIMEOUT
read_timeout = settings.CHAT_COMPLETION_API_READ_TIMEOUT
headers = {'Content-Type': 'application/json', 'Conversation-History-ID': mock_convo_hist_id}
headers = {'Content-Type': 'application/json'}

response_body = {
'message_list': [{'role': 'system', 'content': self.prompt_template}] + self.message_list,
Expand All @@ -133,18 +131,20 @@ def test_post_request_structure_v2_endpoint(self, mock_requests, mock_v2_enabled
mock_requests.post = MagicMock()
mock_v2_enabled.return_value = True

mock_convo_hist_id = str(uuid.uuid5(uuid.NAMESPACE_URL, f'{self.user_id}_{self.course_id}'))
mock_convo_id_seed = json.dumps([str(self.user_id), str(self.course_id)], separators=(',', ':'))
mock_convo_hist_id = str(uuid.uuid5(uuid.NAMESPACE_URL, str(mock_convo_id_seed)))
Comment on lines +134 to +135

Copilot AI Apr 24, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test re-implements the conversation-id generation logic inline (json.dumps(...) + uuid.uuid5(...)). To avoid future drift from create_conversation_history_id() (which is part of what you’re validating), consider importing and calling create_conversation_history_id(self.user_id, self.course_id) here instead of duplicating the algorithm.

Copilot uses AI. Check for mistakes.

completion_endpoint_v2 = settings.CHAT_COMPLETION_API_V2
connect_timeout = settings.CHAT_COMPLETION_API_CONNECT_TIMEOUT
read_timeout = settings.CHAT_COMPLETION_API_READ_TIMEOUT
headers = {'Content-Type': 'application/json', 'Conversation-History-ID': mock_convo_hist_id}
headers = {'Content-Type': 'application/json'}

response_body = {
'client_id': 'edx_olc_la',
'system_message': self.prompt_template,
'messages': self.message_list,
'external_id': self.user_id,
'conversation_id': mock_convo_hist_id,
}

self.get_response()
Expand Down
Loading