fix: preserve falsy values like empty strings in _copy_messages

[jnMetaCode]

Problem

In _copy_messages, the dictionary comprehension uses if v to filter out entries:

{k: list(_copy_images(v)) if k == 'images' else v for k, v in dict(message).items() if v}

This filter removes all falsy values, not just None. In practice this means:

• content: "" (empty string) gets silently dropped
• content: 0 (integer zero) gets silently dropped
• content: False (boolean) gets silently dropped

This is particularly problematic for the content field. An empty string is a valid message content value (for example, tool-call messages where the model response has no text content, just a tool call). Dropping it changes the message structure and can cause issues with downstream processing or serialization that expects the key to be present.

Fix

Changed the filter from if v to if v is not None:

{k: list(_copy_images(v)) if k == 'images' else v for k, v in dict(message).items() if v is not None}

This preserves empty strings, zero, False, and empty lists while still filtering out fields that are genuinely unset (None).

Testing

• Verified that messages with content: "" now correctly preserve the empty string
• Verified that None values are still properly filtered out
• Verified that normal messages with non-empty content are unaffected

[guicybercode]

Code Review: ollama/_client.py – _copy_messages Filter Logic

Overview

This is a single-line change in the _copy_messages helper function that modifies how message fields are filtered during serialization:

Before:

{k: list(_copy_images(v)) if k == 'images' else v for k, v in dict(message).items() if v},

After:

{k: list(_copy_images(v)) if k == 'images' else v for k, v in dict(message).items() if v is not None},

---

Analysis

The Change

The filter condition was changed from if v (truthiness check) to if v is not None (explicit None check).

Why This Matters

Value	if v	if v is not None	Should be kept?
None	False	False	No
False	False	True	Yes
0	False	True	Yes
""	False	True	Yes (context-dependent)
[]	False	True	Yes (context-dependent)
"text"	True	True	Yes

Impact

Bug Fix: The original logic would silently drop any message field with a falsy value, even when that value was intentionally set. For example:

• A boolean flag like "prompt_cache": False would be removed from the payload
• A numeric parameter like "temperature": 0 would be removed
• An empty string "prefix": "" would be removed

This could lead to unexpected behavior where the API receives different parameters than the user intended, or falls back to defaults when an explicit falsy value was specified.

Correct Behavior: Only None should be treated as "not set" and excluded from the serialized payload. All other values, including falsy ones, should be preserved to respect the user's explicit intent.

---

Strengths

1. Precision: Using is not None is the Pythonic way to check for unset values without conflating them with other falsy values.

2. Consistency: This aligns with the pattern used elsewhere in the codebase (model_dump(exclude_none=True)), ensuring uniform handling of optional fields.

3. Minimal and Focused: The change is surgical and addresses a specific edge case without introducing broader refactoring risk.

---

Considerations

1. Empty Strings and Collections: While preserving "", [], and {} is generally correct, verify that the backend API handles these values as expected. If the API treats empty strings the same as omitted fields, this change is still correct because it preserves user intent at the client level.

2. Documentation: Consider adding a brief comment to clarify the intent:

   # Filter out only None values; preserve falsy values like False, 0, or ""
   {k: list(_copy_images(v)) if k == 'images' else v for k, v in dict(message).items() if v is not None},

3. Test Coverage: Ensure test cases exist for messages containing falsy but non-None values:

   # Example test case
   messages = [{"role": "user", "content": "test", "some_flag": False, "count": 0}]
   result = list(_copy_messages(messages))
   assert result[0].some_flag is False
   assert result[0].count == 0

---

Potential Edge Cases

• If a field is explicitly set to None by the user, it will now be excluded. This is the intended behavior and matches the exclude_none=True pattern used in model_dump.
• If the Message model has fields with default values, ensure that omitting None fields does not cause validation issues downstream.

---

Recommendation: Approve

This is a correct and necessary fix that prevents silent data loss for falsy but meaningful values. The change is minimal, low-risk, and improves the reliability of message serialization.

Actions before merge:

1. Verify test coverage for falsy non-None values in message fields
2. (Optional) Add an inline comment to document the filtering intent
3. Confirm that downstream API behavior aligns with preserving explicit falsy values

No further changes required.