Skip to content

Python: [BREAKING] Extract caching from SkillsProvider into a CachingSkillsSource decorator#6847

Merged
giles17 merged 2 commits into
microsoft:mainfrom
giles17:caching-skills-source
Jul 1, 2026
Merged

Python: [BREAKING] Extract caching from SkillsProvider into a CachingSkillsSource decorator#6847
giles17 merged 2 commits into
microsoft:mainfrom
giles17:caching-skills-source

Conversation

@giles17

@giles17 giles17 commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Motivation & Context

SkillsProvider currently bakes caching into itself: the _cached_context
field, the _get_or_create_context() method, and a disable_caching branch in
before_run. This mixes context construction with caching and prevents caching
from being a composable layer in the skills-source pipeline (alongside
Aggregating/Filtering/Deduplicating/DelegatingSkillsSource).

This change extracts caching into a dedicated CachingSkillsSource decorator so
caching becomes a swappable pipeline layer rather than logic welded into the
provider. It mirrors the .NET work in #6768 to keep the two implementations
aligned, and is a prerequisite for #6711 (adding context to skill-source
retrieval).

Description & Review Guide

  • What are the major changes?

    • New CachingSkillsSource(DelegatingSkillsSource) decorator (experimental,
      SKILLS feature) that caches the inner source's skills list. It shares a
      single in-flight fetch across concurrent callers (via an asyncio.Lock with
      a double-checked cache) and only stores the result on success, so a failed
      fetch is not cached and the next call retries.
    • SkillsProvider now wraps its resolved source in a CachingSkillsSource by
      default and rebuilds instructions/tools from the (source-cached) skills each
      run. The provider's internal cache (_cached_context,
      _get_or_create_context) is removed; before_run is now branch-free.
    • disable_caching=True (on SkillsProvider(...) and from_paths(...)) now
      skips wrapping the source instead of toggling an internal flag — preserving
      the existing public kwarg and its meaning. It is the Python equivalent of
      .NET's builder DisableCaching().
    • CachingSkillsSource is exported from agent_framework.
  • What is the impact of these changes?

    • Caching is now composable: callers can place CachingSkillsSource explicitly
      in a pipeline, and disable_caching=True opts out of the default wrap.
    • Behavioral nuance: only the skills list is cached now (not the built
      instructions/tools, which are cheap, deterministic functions of the skills
      and are rebuilt each run). The expensive discovery in get_skills() is still
      cached. This is a [BREAKING — experimental] change to skills internals.
  • What do you want reviewers to focus on?

Related Issue

Fixes #6767

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, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.

…illsSource decorator

Adds a composable CachingSkillsSource(DelegatingSkillsSource) decorator that caches the inner source's skills list, and rewires SkillsProvider to wrap its resolved source in it by default (skipped when disable_caching=True). Removes the provider's baked-in caching (_cached_context field and _get_or_create_context). Mirrors .NET microsoft#6768.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 30, 2026 22:05
@giles17 giles17 added documentation Usage: [Issues, PRs], Target: documentation in the code base and learn docs python Usage: [Issues, PRs], Target: Python breaking change Usage: [PRs], Target: all PRs that introduce changes that are not backward compatible labels Jun 30, 2026

@github-actions github-actions Bot 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: 90% | Result: All clear

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


Automated review by giles17's agents

@github-actions

github-actions Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Python Test Coverage

Python Test Coverage Report •
FileStmtsMissCoverMissing
packages/core/agent_framework
   _skills.py10453896%314, 587, 1107, 1122, 1124–1125, 1492–1493, 1737, 1766, 2303, 2766–2767, 2864, 2872, 2877, 2880, 2885, 2905, 2917, 2922, 3018, 3026, 3031, 3034, 3039, 3059, 3068, 3073, 3334–3335, 3745, 3972–3973, 4000–4001, 4008–4009
TOTAL43145514488% 

Python Unit Test Overview

Tests Skipped Failures Errors Time
8524 37 💤 0 ❌ 0 🔥 2m 9s ⏱️

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

This PR refactors Python skills caching by moving it out of SkillsProvider and into a composable CachingSkillsSource decorator, aligning the Python skills-source pipeline with the decorator-based approach used elsewhere (and mirroring the related .NET work).

Changes:

  • Added CachingSkillsSource (a DelegatingSkillsSource decorator) that caches the inner source’s skills list and shares a single in-flight fetch across concurrent callers.
  • Updated SkillsProvider to wrap its resolved source in CachingSkillsSource by default (unless disable_caching=True) and to rebuild instructions/tools from (possibly cached) skills on each run.
  • Updated tests and documentation to reflect the new caching behavior and exported CachingSkillsSource from agent_framework.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
python/packages/core/tests/core/test_skills.py Adds focused unit tests for CachingSkillsSource caching/retry/concurrency behavior and updates provider caching tests to reflect the new decorator-based approach.
python/packages/core/AGENTS.md Documents the composable SkillsSource decorator pipeline and clarifies that caching now lives in the source pipeline (default-wrapped by SkillsProvider unless opted out).
python/packages/core/agent_framework/_skills.py Implements CachingSkillsSource and removes provider-internal context caching in favor of source-pipeline caching.
python/packages/core/agent_framework/init.py Exports CachingSkillsSource as part of the public package surface.

@giles17 giles17 marked this pull request as ready for review June 30, 2026 22:09

@github-actions github-actions Bot 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: 78% | Result: All clear

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


Automated review by giles17's agents

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@SergeyMenshykh

Copy link
Copy Markdown
Contributor

LGTM. This PR also add cache isolation key selector https://github.com/microsoft/agent-framework/pull/6797/changes#diff-a80018eeb1587fa83185680bb40b0a428a32fb71d302eed0224f7ade8d3ebfeb for .NET cache source. Same needed to be done on python as well. Not necessary in this PR.

@giles17 giles17 added this pull request to the merge queue Jul 1, 2026
Merged via the queue into microsoft:main with commit c416766 Jul 1, 2026
38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking change Usage: [PRs], Target: all PRs that introduce changes that are not backward compatible documentation Usage: [Issues, PRs], Target: documentation in the code base and learn docs python Usage: [Issues, PRs], Target: Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python: Extract caching from SkillsProvider into a CachingSkillsSource decorator

4 participants