Skip to content

feat: add default for memory extraction trigger#2811

Merged
opieter-aws merged 1 commit into
strands-agents:mainfrom
opieter-aws:opieter-aws/python-extraction-config-divergence
Jun 16, 2026
Merged

feat: add default for memory extraction trigger#2811
opieter-aws merged 1 commit into
strands-agents:mainfrom
opieter-aws:opieter-aws/python-extraction-config-divergence

Conversation

@opieter-aws

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

Copy link
Copy Markdown
Contributor

Description

Python's memory-extraction API diverged from the TypeScript SDK in three ways that made the
Python happy path noticeably heavier than its TS counterpart. In TS a store opts into automatic
extraction with extraction: true and gets sensible defaults; in Python the caller had to
construct an ExtractionConfig, pick a trigger explicitly (it was required), and choose an
extractor by hand. The shared documentation ("the simplest form turns it on with defaults",
"defaults to an interval of 5") only held for TS — there was no Python equivalent.

This brings Python in line. A store can now enable extraction with extraction=True, the
trigger is optional (defaulting to every 5 turns), and the extractor is selected automatically
from the store's write methods — matching TS's boolean | ExtractionConfig shape and its
resolveExtractionConfig behavior.

The change is opt-in and pay-for-play: existing ExtractionConfig(trigger=...) call sites keep
working unchanged. Defaulting is now centralized in a single internal _resolve_extraction_config
(mirroring TS's resolve-extraction-config.ts), so the MemoryManager and ExtractionCoordinator
no longer re-interpret raw config or re-apply defaults — the manager resolves once and passes
_ExtractionBindings downstream.

Public API Changes

A store's extraction field accepts a bool shorthand, and ExtractionConfig.trigger is now
optional:

# Before: extraction required an explicit ExtractionConfig with an explicit trigger
class MyStore:
    extraction = ExtractionConfig(trigger=IntervalTrigger(turns=5))

# After: bool shorthand enables extraction with defaults (every 5 turns;
# extractor chosen from the store's write methods)
class MyStore:
    extraction = True

# After: trigger is optional — omit it to default to every 5 turns
class MyStore:
    extraction = ExtractionConfig()  # interval trigger of 5, default extractor + filter

Default extractor is now resolved from the store's write methods, matching TS:

  • A store implementing only add → defaults to a ModelExtractor (client-side fact distillation,
    written via add).
  • A store implementing add_messages → defaults to server-side extraction (raw messages handed to
    add_messages, no model call).

An explicit extractor or filter still overrides the default. An explicit empty trigger list is
still rejected at construction.

Related Issues

Documentation PR

N/A

Type of Change

New feature

Testing

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

  • I ran hatch run prepare

Added a test_resolve_extraction_config suite (ported from the TS resolver tests) covering the
bool shorthand, trigger defaulting/normalization, capability-based extractor selection, and filter
defaulting. Existing coordinator and manager tests were updated to drive the resolved-config path;
the previously-raising "add-only store without an extractor" case now asserts the new
ModelExtractor default behavior.

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/m chore Maintenance tasks, dependency updates, CI changes, refactoring with no user-facing impact area-community Related to community and contributor health labels Jun 15, 2026
@opieter-aws opieter-aws changed the title opieter aws/python extraction config divergence feat: add default for memory extraction trigger Jun 15, 2026
@opieter-aws opieter-aws marked this pull request as ready for review June 15, 2026 21:07
Comment thread strands-py/tests/strands/memory/test_resolve_extraction_config.py
@github-actions

Copy link
Copy Markdown
Contributor

Issue: This widens a public contract — MemoryStoreConfig.extraction becomes ExtractionConfig | bool | None and ExtractionConfig.trigger becomes optional. Per team/API_BAR_RAISING.md, public-API changes go through bar-raising tracked via the api/needs-review label, which isn't on this PR.

Suggestion: This is largely mitigated since the change deliberately mirrors the already-shipped TS boolean | ExtractionConfig shape and resolveExtractionConfig behavior — the design was effectively validated there. Worth a maintainer confirming whether "match an existing cross-SDK API" qualifies as a minimal change (PR review sufficient) or still wants the label for tracking. Not blocking from my side.

@github-actions

Copy link
Copy Markdown
Contributor

Assessment: Approve (with minor comments)

Clean, faithful port of TS's resolveExtractionConfig that genuinely simplifies the Python happy path. Centralizing defaulting in _resolve_extraction_config and threading _ExtractionBindings downstream is the right call — it removes the duplicated _normalize_triggers and stops the manager/coordinator from re-interpreting raw config. Verified locally: 114 tests pass, ruff and mypy ./src clean.

Review notes
  • Correctness: Resolver logic matches the TS reference line-for-line, including the capability-based extractor default and the "leave an explicit empty trigger list for the manager to reject" handoff. The config.filter or DEFAULT_... defaulting is safe — MemoryMessageFilter has no __bool__/__len__, so an explicit empty filter is preserved (confirmed).
  • Tests: Resolver suite is a 1:1 port of the TS test cases; the per-field trigger assertions are justified (no value equality on IntervalTrigger) and documented. Manager/coordinator tests correctly updated to drive the resolved-config path.
  • API: Public contract widens (extraction: bool | ExtractionConfig, optional trigger) — flagged separately re: api/needs-review, mitigated by mirroring the shipped TS API.
  • Docs: PR's "updated docs"/"added example" boxes are unchecked and there's no memory-extraction user guide in site/; docstrings are thorough, but one in init_agent still describes the pre-PR ExtractionConfig-only shape.

Nice, well-scoped change — the cross-SDK parity and the thorough docstrings made this easy to follow.

@opieter-aws opieter-aws enabled auto-merge (squash) June 16, 2026 02:16
@opieter-aws opieter-aws merged commit 6f9cae0 into strands-agents:main Jun 16, 2026
37 of 46 checks passed
@opieter-aws opieter-aws deleted the opieter-aws/python-extraction-config-divergence branch June 16, 2026 02:16
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/m

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants