Skip to content

Python: Fix store=False not overriding client default#4569

Open
TaoChenOSU wants to merge 4 commits intomicrosoft:mainfrom
TaoChenOSU:taochen/python-4488
Open

Python: Fix store=False not overriding client default#4569
TaoChenOSU wants to merge 4 commits intomicrosoft:mainfrom
TaoChenOSU:taochen/python-4488

Conversation

@TaoChenOSU
Copy link
Contributor

@TaoChenOSU TaoChenOSU commented Mar 9, 2026

Motivation and Context

Closes #4488

Description

Fixes the bug where runtime store=False option doesn't take precedence over client default option.

Contribution Checklist

  • The code builds clean without any errors or warnings
  • The PR follows the Contribution Guidelines
  • All unit tests pass, and I have added new tests where possible
  • Is this a breaking change? If yes, add "[BREAKING]" prefix to the title of the PR.

@TaoChenOSU TaoChenOSU self-assigned this Mar 9, 2026
Copilot AI review requested due to automatic review settings March 9, 2026 20:59
@TaoChenOSU TaoChenOSU added the agents Issues related to single agents label Mar 9, 2026
@github-actions github-actions bot changed the title Fix store=False not overriding client default Python: Fix store=False not overriding client default Mar 9, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes a bug in the Python agent runtime where store=False provided via agent/runtime options does not correctly take precedence over client defaults (e.g., STORES_BY_DEFAULT=True), impacting whether local history storage is injected.

Changes:

  • Merge agent default_options with runtime options in _prepare_run_context so runtime options take precedence.
  • Simplify the InMemoryHistoryProvider auto-injection condition using store + STORES_BY_DEFAULT.
  • Update/expand unit tests around STORES_BY_DEFAULT / store behaviors and refresh python/uv.lock dependency pins.

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.

File Description
python/packages/core/agent_framework/_agents.py Merges default + runtime options earlier and adjusts store/STORES_BY_DEFAULT logic for local history injection.
python/packages/core/tests/core/test_agents.py Adds/adjusts tests for storage injection behavior and reformats some existing tests.
python/uv.lock Updates locked dependency versions (filelock, fonttools, kiwisolver, posthog, prek, setuptools).
Comments suppressed due to low confidence (1)

python/packages/core/tests/core/test_agents.py:1589

  • The regression this PR targets (agent-level default_options={"store": False} should override a client with STORES_BY_DEFAULT=True) isn’t directly covered by the added tests. Current coverage only asserts runtime options={"store": False}. Please add a test where client.STORES_BY_DEFAULT=True, the agent is constructed with default_options={"store": False}, run() is called without passing options, and InMemoryHistoryProvider is injected.
async def test_stores_by_default_with_store_false_injects_inmemory(
    client: SupportsChatGetResponse,
) -> None:
    """Client with STORES_BY_DEFAULT=True but store=False should still inject InMemoryHistoryProvider."""
    from agent_framework._sessions import InMemoryHistoryProvider

    client.STORES_BY_DEFAULT = True  # type: ignore[attr-defined]

    agent = Agent(client=client)
    session = agent.create_session()

    await agent.run("Hello", session=session, options={"store": False})

Copilot AI review requested due to automatic review settings March 9, 2026 22:51
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.

@markwallace-microsoft
Copy link
Member

Python Test Coverage

Python Test Coverage Report •
FileStmtsMissCoverMissing
packages/core/agent_framework
   _agents.py3524288%457, 461, 525, 943, 981, 999, 1098–1102, 1176, 1313, 1331, 1333, 1348, 1354, 1393, 1395, 1404–1407, 1410–1411, 1416, 1418, 1424–1425, 1437, 1439–1440, 1448–1449, 1454–1456, 1464–1465, 1467, 1472, 1474
TOTAL22685257888% 

Python Unit Test Overview

Tests Skipped Failures Errors Time
4947 20 💤 0 ❌ 0 🔥 1m 20s ⏱️

@TaoChenOSU TaoChenOSU enabled auto-merge March 9, 2026 23:36
@TaoChenOSU TaoChenOSU added this pull request to the merge queue Mar 9, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 9, 2026
@moonbox3 moonbox3 added this pull request to the merge queue Mar 9, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 10, 2026
@TaoChenOSU TaoChenOSU added this pull request to the merge queue Mar 10, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 10, 2026
@moonbox3 moonbox3 added this pull request to the merge queue Mar 10, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 10, 2026
@moonbox3 moonbox3 added this pull request to the merge queue Mar 10, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Issues related to single agents python

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Python: [Bug]: STORES_BY_DEFAULT seems to cause agents have neither local nor remote storage

5 participants