Skip to content

Python: Preserve Mistral prompt-cache usage details - #7597

Open
Ruiming Zhao (uuzzrm) wants to merge 5 commits into
microsoft:mainfrom
uuzzrm:codex/fix-mistral-cached-token-usage
Open

Python: Preserve Mistral prompt-cache usage details#7597
Ruiming Zhao (uuzzrm) wants to merge 5 commits into
microsoft:mainfrom
uuzzrm:codex/fix-mistral-cached-token-usage

Conversation

@uuzzrm

@uuzzrm Ruiming Zhao (uuzzrm) commented Aug 10, 2026

Copy link
Copy Markdown

Motivation & Context

Mistral chat-completion responses can include prompt cache hits under usage.prompt_tokens_details.cached_tokens. The Mistral adapter currently drops that field, so callers cannot observe cache reads through standard UsageDetails. This is especially visible in cost and telemetry reporting for cached prompts.

Description & Review Guide

  • What are the major changes?
    • Map prompt_tokens_details.cached_tokens to both prompt/cached_tokens and cache_read_input_token_count in _parse_usage.
    • Add regression coverage for non-streaming responses and final streaming usage chunks.
  • What is the impact of these changes?
    • Existing usage fields are unchanged. Mistral cache-read information is now available through the same standard field used by other providers.
  • What do you want reviewers to focus on?
    • Whether the mapping and type guard match the shape of Mistral usage payloads and the conventions used by other provider clients.

Related Issue

Fixes #7589

Contribution Checklist

  • The code builds clean without any errors or warnings
  • All unit tests pass, and I have added new tests where possible
  • The PR follows the Contribution Guidelines
  • This PR is linked to an issue and there is no other open PR for this issue (see Related Issue above).
  • This is not a breaking change. If it is a breaking change, add the breaking change label (or add "[BREAKING]" to the title prefix) — a workflow keeps the label and title prefix in sync automatically.

Map prompt cache hits from Mistral chat usage into the standard usage details. Add regression coverage for regular and streaming responses.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Preserves Mistral prompt-cache usage in standard framework usage details.

Changes:

  • Maps cached prompt tokens to provider-specific and standard fields.
  • Adds non-streaming and streaming regression coverage.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
python/packages/mistral/agent_framework_mistral/_chat_client.py Parses cached prompt-token usage.
python/packages/mistral/tests/mistral/test_mistral_chat_client.py Tests cache usage propagation.

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Comment thread python/packages/mistral/agent_framework_mistral/_chat_client.py Outdated
@uuzzrm
Ruiming Zhao (uuzzrm) marked this pull request as ready for review August 10, 2026 15:05
@uuzzrm

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

@uuzzrm

Copy link
Copy Markdown
Author

Following up on the strict integer validation feedback: the current head a1c1b44 contains the requested validation, and the reported repository checks are successful. Please re-review the current head when convenient.

@uuzzrm

Copy link
Copy Markdown
Author

Synced the existing branch with the latest upstream main in merge commit 17da48c.

The review fix itself is unchanged. The current head retains strict integer validation for cached_tokens (excluding bool) and regression coverage for string, float, and boolean payloads.

Local verification after the sync is environment-limited: the bundled Python has pytest but not the repository dependencies or uv, so collection stops before imports with ModuleNotFoundError for agent_framework. The prior focused CI checks for the fix were green; the new push has triggered the repository CI for the synced head.

The PR remains open and unmerged, awaiting the repository checks and maintainer review.

@github-actions

Copy link
Copy Markdown
Contributor

Python Test Coverage

Python Test Coverage Report •
FileStmtsMissCoverMissing
packages/mistral/agent_framework_mistral
   _chat_client.py4591097%270, 272–274, 281, 285, 289, 292–293, 295
TOTAL45728424890% 

Python Unit Test Overview

Tests Skipped Failures Errors Time
9285 36 💤 0 ❌ 0 🔥 2m 39s ⏱️

@giles17 Giles Odigwe (giles17) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Automated Code Review

Reviewers: 5 | Confidence: 93% | Result: All clear

Reviewed: Correctness, Security Reliability, Test Coverage, Failure Modes, Design Approach


Automated review by giles17's agents

@giles17

Copy link
Copy Markdown
Contributor

Hey Ruiming Zhao (@uuzzrm) please address failing checks on the PR

Narrow prompt token details before reading cached_tokens so the Mistral package passes strict Pyright without changing runtime validation.\n\nAddresses microsoft#7597 (comment)
@uuzzrm

Ruiming Zhao (uuzzrm) commented Aug 11, 2026

Copy link
Copy Markdown
Author

AI says: I addressed the failing checks in commit 2d72515cd. The failure was strict Pyright reporting an unknown cached_tokens type after the runtime Mapping check; the fix explicitly narrows that mapping to Mapping[str, Any] without changing the existing integer/non-bool validation.

Verified locally with:

  • uv run --frozen pyright -p packages/mistral/pyproject.toml — 0 errors
  • uv run --frozen pytest -q packages/mistral/tests/mistral/test_mistral_chat_client.py -m 'not integration' — 56 passed
  • uv run --frozen ruff check ... and ruff format --check ... — passed

Prysai

@uuzzrm

Ruiming Zhao (uuzzrm) commented Aug 11, 2026

Copy link
Copy Markdown
Author

AI says: The fix is pushed in 2d72515cd, and the local package checks pass. GitHub created the new workflow runs for this forked PR with action_required, so the Python workflows have not started yet and need maintainer approval before their results can be reported.

Prysai

@uuzzrm

Copy link
Copy Markdown
Author

Updated the branch with the current upstream main (27d82b1) in merge commit 3c705e0.

Re-ran the focused checks locally after the sync:

  • uv run --frozen pyright -p packages/mistral/pyproject.toml ? 0 errors
  • uv run --frozen pytest -q packages/mistral/tests/mistral/test_mistral_chat_client.py -m "not integration" ? 56 passed
  • uv run --frozen ruff check on the changed Mistral files ? passed
  • uv run --frozen ruff format --check on the changed Mistral files ? passed
  • git diff --check ? passed

The fork workflow runs for this head are currently action_required, so the repository test workflows have not started and require maintainer approval. The PR remains open and ready for review.

AI assistance was used to prepare this update.

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

Labels

python Usage: [Issues, PRs], Target: Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python: [Bug]: the Mistral chat client drops prompt_tokens_details.cached_tokens, so prompt-cache hits are invisible in UsageDetails

3 participants