Skip to content

Conversation

@s-akhtar-baig
Copy link
Contributor

Problem

As an Application Developer, I want to use the include parameter with the value message.output_text.logprobs, so that I can receive log probabilities for output tokens to assess the model's confidence in its response.

What does this PR do?

  • Updates the include parameter in various resource definitions
  • Updates the inline provider to return logprobs when "message.output_text.logprobs" is passed in the include parameter
  • Converts the logprobs returned by the inference provider from chat completion format to responses format

Closes #4260

Test Plan

@meta-cla meta-cla bot added the CLA Signed This label is managed by the Meta Open Source bot. label Nov 30, 2025
@s-akhtar-baig
Copy link
Contributor Author

Note to the reviewers:

The current behavior when passing include["message.output_text.logprobs"] along with tools is as follows:

Tool type Current behavior (are logprobs returned?) OpenAI behavior (are logprobs returned?)
MCP Yes Yes
Built-in Yes No
Function No No

The behavior gets further complicated when passing include["message.output_text.logprobs"] with multiple tools:

  • Web search call before mcp tool call: OpenAI generated two assistant messages, and logprobs were returned.
  • Mcp tool call before web search call: OpenAI generated one assistant message, and no logprobs were returned.

I wanted to get other opinions to determine how logprobs should be handled with built-in and mcp tools. My preference is to keep it consistent i.e. either return logprobs for both or do not return logprobs for either.

Appreciate any feedback!

Test script: https://github.com/s-akhtar-baig/llama-stack-examples/blob/main/responses/src/include.py

@s-akhtar-baig s-akhtar-baig changed the title feat!: Implement include parameter specifically for adding logprobs in the output message feat: Implement include parameter specifically for adding logprobs in the output message Nov 30, 2025
@ashwinb
Copy link
Contributor

ashwinb commented Dec 1, 2025

This looks good to me. Doing this consistently makes sense!

@mergify
Copy link

mergify bot commented Dec 2, 2025

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

@github-actions
Copy link
Contributor

github-actions bot commented Dec 2, 2025

✱ Stainless preview builds

This PR will update the llama-stack-client SDKs with the following commit message.

feat: Implement include parameter specifically for adding logprobs in the output message

Edit this comment to update it. It will appear in the SDK's changelogs.

llama-stack-client-node studio · code · diff

Your SDK built successfully.
generate ⚠️build ✅lint ✅test ✅

npm install https://pkg.stainless.com/s/llama-stack-client-node/ca079771633ea253598bf3c96e20112d3c13dab3/dist.tar.gz
llama-stack-client-kotlin studio · code · diff

Your SDK built successfully.
generate ⚠️lint ✅test ❗

llama-stack-client-python studio · code · diff

generate ⚠️build ⏳lint ⏳test ⏳

llama-stack-client-go studio · code · diff

Your SDK built successfully.
generate ⚠️lint ❗test ❗

go get github.com/stainless-sdks/llama-stack-client-go@f350020c226e602777780297a3857ba91cae5774

⏳ These are partial results; builds are still running.


This comment is auto-generated by GitHub Actions and is automatically kept up to date as you push.
Last updated: 2025-12-03 20:07:49 UTC

@s-akhtar-baig s-akhtar-baig changed the title feat: Implement include parameter specifically for adding logprobs in the output message feat!: Implement include parameter specifically for adding logprobs in the output message Dec 2, 2025
@s-akhtar-baig s-akhtar-baig force-pushed the add_include_param branch 3 times, most recently from 02424d7 to 8ff43de Compare December 2, 2025 22:48
@mergify
Copy link

mergify bot commented Dec 3, 2025

This pull request has merge conflicts that must be resolved before it can be merged. @s-akhtar-baig 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 Dec 3, 2025
@s-akhtar-baig
Copy link
Contributor Author

This looks good to me. Doing this consistently makes sense!

Thanks, @ashwinb!

It took a while for me to get the recordings in, sorry about that, but now the pipeline is green. Please let me know if the changes look good. Thanks!

@ashwinb
Copy link
Contributor

ashwinb commented Dec 3, 2025

@s-akhtar-baig looks like the docker variant is still red somehow :(

@s-akhtar-baig
Copy link
Contributor Author

@ashwinb, it was failing on acquiring a lock, hopefully it passes successfully this time.

@s-akhtar-baig
Copy link
Contributor Author

s-akhtar-baig commented Dec 3, 2025

@ashwinb, the check failed again on:

File "/home/runner/work/llama-stack/llama-stack/.venv/lib/python3.12/site-packages/httpcore/_synchronization.py", line 268, in __enter__
    self._lock.acquire()
+++++++++++++++++++++++++++++++++++ Timeout ++++++++++++++++++++++++++++++++++++

Can you please help re-run the check (I don't have the required permissions)?

Re: Don't know why it keeps failing.

@ashwinb
Copy link
Contributor

ashwinb commented Dec 3, 2025

Something in the PR is triggering a deadlock in the conversations test case

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.

2 participants