Skip to content

fix(v2): handle consecutive None-content messages in merge_consecutive_messages - #2440

Closed
chuenchen309 wants to merge 1 commit into
567-labs:mainfrom
chuenchen309:fix/merge-consecutive-messages-none-content
Closed

fix(v2): handle consecutive None-content messages in merge_consecutive_messages#2440
chuenchen309 wants to merge 1 commit into
567-labs:mainfrom
chuenchen309:fix/merge-consecutive-messages-none-content

Conversation

@chuenchen309

Copy link
Copy Markdown

Describe your changes

merge_consecutive_messages (instructor/v2/core/messages.py) crashes with AttributeError: 'NoneType' object has no attribute 'append' when two consecutive same-role messages both have content=None — a normal shape for back-to-back tool-call-only assistant turns (e.g. {"role": "assistant", "content": None, "tool_calls": [...]}) replayed as conversation history.

Root cause: new_content = message.get("content", "")dict.get(key, default) only substitutes the default when the key is absent, not when its value is explicitly None. So new_content stays None, fails the isinstance(new_content, str) check, and on the second consecutive message of the same role hits new_messages[-1]["content"].append(new_content) where new_messages[-1]["content"] is itself None.

This runs before any network call — it's invoked directly on user-supplied kwargs["messages"] from the OpenAI (Mode.JSON/Mode.MD_JSON), Mistral, and Writer handlers (instructor/v2/providers/{openai,mistral,writer}/handlers.py).

Fix: normalize None content to "" right after extraction, matching the function's existing convention of treating "" as the safe default for missing/empty content.

new_content = message.get("content", "")
if new_content is None:
    new_content = ""

Issue ticket number and link

None found — searched open PRs/issues touching messages.py / merge_consecutive_messages, no overlap.

Checklist before requesting a review

  • I have performed a self-review of my code
  • If it is a core feature, I have added thorough tests.
  • If it is a core feature, I have added documentation. (bug fix, not a new feature — no doc changes needed)

Testing

  • Added test_consecutive_none_content to tests/processing/test_message_processing.py; confirmed (via git stash) it fails with the exact AttributeError against the pre-fix code, passes after.
  • Ran tests/processing/test_message_processing.py, tests/processing/test_utils.py, tests/test_utils.py: 77 passed (incl. the new test), same 2 pre-existing failures before/after (TestUpdateGeminiKwargs::test_*_safety_settings, need the optional google-genai package which isn't installed in my dev environment — unrelated to this change, confirmed identical on main).
  • ruff check, ruff format --check, and ty check all clean on the changed files.
  • Added a CHANGELOG.md entry under [Unreleased] / Fixed per CLAUDE.md's PR guidelines.

AI disclosure

This fix was developed with AI assistance (Claude Code): it located the crash via a targeted search for reachable-but-untested code paths, traced the root cause through dict.get's None-vs-missing-key semantics, and drafted the patch/test/changelog entry. I reviewed the diff, independently re-derived the root cause by reading the function and tracing its callers, and ran the test suite and linters myself before opening this PR.

…e_messages

Two consecutive same-role messages that both have content=None (a normal
shape for back-to-back tool-call-only assistant turns replayed as history)
crashed with AttributeError: 'NoneType' object has no attribute 'append',
since dict.get(key, default) only applies the default when the key is
absent, not when its value is explicitly None. Reached before any network
call from the OpenAI, Mistral, and Writer JSON-mode handlers.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@ErenAta16 ErenAta16 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Confirmed the crash: two consecutive same-role messages with content=None (e.g. back-to-back tool-call-only assistant turns) previously hit the isinstance(new_content, str) branch with None, which isn't a str, so it fell through without normalizing to the [{"type": "text", ...}] list form the merge logic expects, and the subsequent merge step assumed that shape. Normalizing None to "" before the str check brings it in line with how empty/missing content is already handled elsewhere in the same function. Test covers the exact reported shape (two tool-call-only assistant messages back to back). Looks correct.

@jxnl

jxnl commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

Consolidated and shipped in #2495. Closing this focused patch as superseded; thank you for the contribution.

@jxnl jxnl closed this Jul 29, 2026
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.

3 participants