Skip to content

fix: mark internal memory functions as private#2817

Open
opieter-aws wants to merge 1 commit into
strands-agents:mainfrom
opieter-aws:opieter-aws/address-pr-review-comments
Open

fix: mark internal memory functions as private#2817
opieter-aws wants to merge 1 commit into
strands-agents:mainfrom
opieter-aws:opieter-aws/address-pr-review-comments

Conversation

@opieter-aws

@opieter-aws opieter-aws commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Description

Follow-up that closes two non-blocking consistency nits left on the memory PRs (#2811, #2797).
Both were flagged as "fine to merge as-is" by reviewers; this brings the code in line with the
repo's own privacy conventions now, while the surface is young and renaming is cheap — once
released, a module path or symbol can quietly become load-bearing for a downstream user.

1. The lone public symbol in an otherwise-private module (#2811).
resolve_extraction_config.py exposes a private type (_ResolvedExtractionConfig) and a private
function (_resolve_extraction_config), but the module's default-cadence constant was public.
It's referenced only within its own module and test, so it should be private too:
DEFAULT_EXTRACTION_TRIGGER_TURNS_DEFAULT_EXTRACTION_TRIGGER_TURNS. This matches the
prevailing _DEFAULT_* convention across the SDK (e.g. context_injector/plugin.py's
_DEFAULT_NAME, multiagent/graph.py's _DEFAULT_GRAPH_ID). The TS sibling still exports its
constant because it's used across files there; the Python one isn't, so privatizing is the
locally-correct call.

2. Internal module files that read as public paths (#2797).
The injection package underscored its functions but not its module files, so
from strands.injection import message_injection still worked and exposed the internals with no
namespace-level "private" signal. The repo convention is to underscore the module itself
(event_loop/_retry.py, models/_defaults.py, strands/_async.py). This renames the two
internal delivery modules to match:

  • injection/message_injection.pyinjection/_message_injection.py
  • injection/xml.pyinjection/_xml.py

The three importers (memory/memory_manager.py, vended_plugins/context_injector/plugin.py, and
the injection test) and the injection/__init__.py docstring reference are updated accordingly.
The package's exported __all__ surface is unchanged — only the internal module paths move.

Related Issues

Follow-up to #2811 and #2797.

Documentation PR

N/A — no public API or documented behavior changes.

Type of Change

Other (please describe): internal refactor for naming/privacy consistency — no behavior or public
API change.

Testing

How have you tested the change? Verify that the changes do not break functionality or introduce new warnings.

  • I ran hatch run prepare

hatch run prepare passes clean (3598 passed, 4 skipped). Also confirmed the privacy boundary
holds: from strands.injection import _message_injection, _xml imports, while
import strands.injection.message_injection now raises ModuleNotFoundError.

Checklist

  • I have read the CONTRIBUTING document
  • I have reviewed and understand every line of code in this PR, including any generated by AI tools, and I can explain why it works
  • My change is focused and reasonably small; I have split unrelated work into separate PRs
  • I have added any necessary tests that prove my fix is effective or my feature works
  • I have updated the documentation accordingly
  • I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@github-actions github-actions Bot added size/s area-community Related to community and contributor health chore Maintenance tasks, dependency updates, CI changes, refactoring with no user-facing impact labels Jun 16, 2026
@opieter-aws opieter-aws changed the title opieter aws/address pr review comments fix: mark internal memory functions as private Jun 16, 2026
@opieter-aws opieter-aws marked this pull request as ready for review June 16, 2026 02:42
@github-actions

Copy link
Copy Markdown
Contributor

Assessment: Approve

A tidy, low-risk consistency refactor that brings the memory/injection internals in line with the SDK's established privacy conventions (_DEFAULT_* constants, underscored internal module files like event_loop/_retry.py, models/_defaults.py). The reasoning in the description is sound — privatizing while the surface is young is the cheap, correct call.

Verification performed
  • Reference completeness: Repo-wide grep (.py, .md, .mdx, .toml) shows no stale references to DEFAULT_EXTRACTION_TRIGGER_TURNS, injection.message_injection, or injection.xml. All importers and docstrings updated.
  • Privacy boundary holds: from strands.injection import _message_injection, _xml works; import strands.injection.message_injection now raises ModuleNotFoundError.
  • Public surface unchanged: injection.__all__ still exports only InjectionConfig, InjectionContext, InjectionTrigger.
  • Tests green: the two affected test modules pass (37 passed locally).
  • No API-review needed: the change strictly reduces public surface with no behavior change.

Nicely scoped and easy to verify — thanks for closing out the nits while the cost is low.

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

Labels

area-community Related to community and contributor health chore Maintenance tasks, dependency updates, CI changes, refactoring with no user-facing impact size/s

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant