Skip to content

fix(lmstudio): handle system messages regardless of position - #2071

Open
BlueX888 wants to merge 1 commit into
dottxt-ai:mainfrom
BlueX888:fix/prep-lmstudio-system-message-position-crash
Open

BlueX888 wants to merge 1 commit into
dottxt-ai:mainfrom
BlueX888:fix/prep-lmstudio-system-message-position-crash

Conversation

@BlueX888

Copy link
Copy Markdown

Summary

LMStudioTypeAdapter.format_chat_model_input raised ValueError: Unsupported role: system for any system message that was not the first message of the chat, including chats that contain a second system message. A system message in first position with an empty-string content was silently dropped instead of being passed along.

Why

The Chat input documents that each message's role "can be 'user', 'assistant', or 'system'" with no positional constraint (src/outlines/inputs.py:82). The Gemini adapter already follows that contract: it collects system messages regardless of their position and joins them (src/outlines/models/gemini.py:87-95, introduced in #1967).

The LMStudio adapter instead only lifted a system message when it was the first message, and built the chat with LMSChat(system_prompt) if system_prompt else LMSChat(). Any other system message fell through to the role check and raised ValueError: Unsupported role: system (src/outlines/models/lmstudio.py:122 on main). The if system_prompt test also treated an empty-string system message as absent.

Changes

  • src/outlines/models/lmstudio.py: format_chat_model_input now adds each system message at the position it appears in, via the SDK's Chat.add_system_prompt. Consecutive system messages are joined with "\n", because lmstudio 1.5.0 raises LMStudioRuntimeError: Multi-part or consecutive system prompts are not supported. when two system prompts are added back to back. An empty-string system message is preserved rather than dropped.
  • tests/models/test_lmstudio_type_adapter.py: regression tests for a system message in the middle of the chat, consecutive system messages, an empty-string system message, and a trailing system message.

Testing

Run locally on Python 3.12 with lmstudio 1.5.0:

pytest tests/models/test_lmstudio_type_adapter.py -k "system_not_first or multiple_system or empty_system or trailing_system"
# 4 passed, 17 deselected
# (on main, all 4 fail: ValueError: Unsupported role: system, or the
#  empty-string system message is missing from the resulting chat)

pytest tests/models/test_lmstudio_type_adapter.py tests/models/test_lmstudio.py
# 42 passed, 4 skipped

coverage run --branch --source=outlines -m pytest tests/models/test_lmstudio_type_adapter.py tests/models/test_lmstudio.py
coverage report
# src/outlines/models/lmstudio.py    149 stmts, 58 branches, 0 missed   100%

pre-commit run --all-files
# check for merge conflicts ......... Passed
# debug statements (python) ......... Passed
# fix end of files .................. Passed
# trim trailing whitespace .......... Passed
# mypy .............................. Passed
# ruff .............................. Passed

Checklist:

  • We should be able to understand what the PR does from its title only;
  • There is a high-level description of the changes;
  • If I add a new feature, there is an issue discussing it already; (bug fix, not a new feature)
  • There are links to all the relevant issues, discussions and PRs; (no existing issue covers this; the collect-anywhere system-message behavior was introduced for Gemini in fix(gemini): pass the system message as a system instruction #1967)
  • The branch is rebased on the latest main commit;
  • Commit messages follow these guidelines;
  • One commit per logical change;
  • The code respects the current naming conventions;
  • Docstrings follow the numpy style guide;
  • pre-commit is installed and configured on your machine, and you ran it before opening the PR;
  • There are tests covering the changes;
  • The documentation is up-to-date;

`format_chat_model_input` only lifted a system message when it was the
first message in the chat, and raised `ValueError: Unsupported role:
system` for a system message in any other position. An empty-string
system message was also silently dropped.

Add system messages at the position they appear in, via the SDK's
`Chat.add_system_prompt`. Consecutive system messages are joined, as
the `lmstudio` SDK does not support multiple consecutive system
prompts.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant