Skip to content

fix: tolerate null response content joins#5298

Open
giulio-leone wants to merge 2 commits intollamastack:mainfrom
giulio-leone:fix/response-null-content-crash
Open

fix: tolerate null response content joins#5298
giulio-leone wants to merge 2 commits intollamastack:mainfrom
giulio-leone:fix/response-null-content-crash

Conversation

@giulio-leone
Copy link
Copy Markdown

Summary

  • tolerate TextContentItem(text=None) when flattening interleaved content
  • tolerate None entries when joining ChatCompletionResult.content_text
  • add focused regressions for both null-content join paths

Why

Current main still reproduces the null-content crash from #4996:

  1. interleaved_content_as_str([TextContentItem.model_construct(text=None)])
  2. ChatCompletionResult(..., content=[None, "tool result"], ...).content_text

Both currently raise:

TypeError: sequence item 0: expected str instance, NoneType found

The earlier closed attempt in #5029 fixed the same underlying issue, but it targeted the older meta_reference responses layout. After the current builtin/responses refactor, the same bug is still present on main under the new path.

Validation

  • .venv/bin/ruff check src/llama_stack/providers/utils/inference/prompt_adapter.py src/llama_stack/providers/inline/agents/builtin/responses/types.py tests/unit/providers/utils/inference/test_prompt_adapter_none_safety.py tests/unit/providers/inline/agents/builtin/responses/test_types.py
  • .venv/bin/python -m pytest tests/unit/providers/utils/inference/test_prompt_adapter_none_safety.py tests/unit/providers/inline/agents/builtin/responses/test_types.py tests/unit/providers/inline/agents/builtin/responses/test_streaming.py -q
  • exact source-tree proof on clean origin/main vs this branch showing main still raises TypeError in both paths while this branch returns "" / "tool result"

Closes #4996
Supersedes #5029

⚠️ This reopens #5244 which was accidentally closed due to fork deletion.

@meta-cla meta-cla bot added the CLA Signed This label is managed by the Meta Open Source bot. label Mar 25, 2026
@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Mar 25, 2026

This pull request has merge conflicts that must be resolved before it can be merged. @giulio-leone please rebase it. https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify bot added the needs-rebase label Mar 25, 2026
@cdoern
Copy link
Copy Markdown
Collaborator

cdoern commented Mar 25, 2026

This PR is out of date, please rebase.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@giulio-leone giulio-leone force-pushed the fix/response-null-content-crash branch from 27375df to 7c177ad Compare March 26, 2026 08:42
@giulio-leone
Copy link
Copy Markdown
Author

@cdoern rebased this PR onto current main and pushed the updated branch.

Notes:

  • the upstream agents -> responses rename moved one test import path during the rebase; the fix itself stayed the same
  • targeted regression validation passed after the rebase with:
    • uv run --python 3.12 --group dev --group unit pytest tests/unit/providers/inline/responses/builtin/responses/test_types.py tests/unit/providers/utils/inference/test_prompt_adapter_none_safety.py

This should clear the out-of-date/conflict blocker from the earlier review.

@mergify mergify bot removed the needs-rebase label Mar 26, 2026
@giulio-leone
Copy link
Copy Markdown
Author

@cdoern quick follow-up: the earlier out-of-date/rebase blocker should now be cleared on the current head.

The PR was rebased onto current main, the targeted regression validation was rerun after the rebase, and the effective PR check state is now 72 successful, 0 failing, 2 skipped (with only non-blocking cancelled entries in the matrix history).

If you have a moment, I would appreciate a fresh look to confirm there are no remaining blockers. Thanks!

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

Labels

CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: prevent NoneType join crash in Responses API when model returns tool_calls with content: null

3 participants