feat: propagate RunContextWrapper to session history read/write paths#2690
feat: propagate RunContextWrapper to session history read/write paths#2690HuxleyHu98 wants to merge 14 commits intoopenai:mainfrom
Conversation
HuxleyHu98
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 53f1c4779f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
HuxleyHu98
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 78e39d7b91
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 33cc084153
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 56794b32ad
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d06a2fe9d3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
HuxleyHu98
force-pushed
the
session-wrapper-history-paths
branch
from
4c26f31 to
6f96adb
Compare
|
I’ve addressed the review feedback and resolved the outdated Codex conversations. Local checks are passing on my side. Could a maintainer please enable/approve the PR checks when convenient? Thanks! |
HuxleyHu98
force-pushed
the
session-wrapper-history-paths
branch
from
168e211 to
3cee188
Compare
|
Local checks are all passing (format, lint, mypy, pyright — 0 errors, 166 tests passed). Could a maintainer please approve running the CI checks? Thanks! |
| async def get_items( | ||
| self, | ||
| limit: int | None = None, | ||
| wrapper: RunContextWrapper[Any] | None = None, |
There was a problem hiding this comment.
Can you make this consistent across the built-in implementations?
| wrapper: RunContextWrapper[Any] | None = None, | |
| *, | |
| wrapper: RunContextWrapper[Any] | None = None, |
HuxleyHu98
force-pushed
the
session-wrapper-history-paths
branch
from
4b70a92 to
bd454c0
Compare
Apply consistent kw-only enforcement for the wrapper parameter in get_items() and add_items() across all built-in and extension session implementations: - src/agents/memory/session.py (SessionABC + NullSession) - src/agents/memory/sqlite_session.py - src/agents/memory/openai_responses_compaction_session.py - src/agents/memory/openai_conversations_session.py - src/agents/extensions/memory/async_sqlite_session.py - src/agents/extensions/memory/advanced_sqlite_session.py - src/agents/extensions/memory/encrypt_session.py - src/agents/extensions/memory/redis_session.py - src/agents/extensions/memory/dapr_session.py - src/agents/extensions/memory/sqlalchemy_session.py - examples/memory/file_session.py
HuxleyHu98
force-pushed
the
session-wrapper-history-paths
branch
from
bd454c0 to
60a2807
Compare
|
Rebased onto the latest \ and reran local checks after the rebase. Current local status is all green on the touched surface:\n\n- ruff format --check ✅\n- ruff check ✅\n- mypy ✅\n- pyright ✅\n- uv run pytest tests/extensions/memory ✅\n\nCould a maintainer please enable/approve the PR CI checks when convenient? Thanks! |
|
@seratch Marked this PR ready for review again after addressing the previous comments. CI is kicking off now; would appreciate another look when you have a moment. |
|
Thanks for working on this. Overall, this should be good. That said, this PR changes lots of files, and those edits could conflict with our current priorities. Thus, let us hold off merging it even if it looks good to me. It may take a few weeks. It'd be appreciated if you could understand this. |
|
Thanks, totally understood — I appreciate the review and the context. Happy to leave this here for now and wait until the timing is better on your side. |
2 participants
Summary
This PR adds backward-compatible
RunContextWrapperpropagation to the main session history read/write paths.Specifically, it:
wrappersupport to:Session.get_items(...)Session.add_items(...)RunContextWrapperthrough:prepare_input_with_session(...)save_result_to_session(...)save_resumed_turn_items(...)wrapperWhy
Issue #2072 is about making runtime context available to session implementations.
The earlier approach in #2488 added optional parameters at the interface level, but it did not actually propagate context from the runner side. This patch focuses on the runner-side propagation path so session implementations that support it can actually receive
RunContextWrapperduring session history reads/writes.At the same time, this keeps older custom session implementations working unchanged.
Scope
This PR intentionally keeps the scope narrow.
Included:
get_itemsadd_itemsNot included:
pop_itemclear_sessionI kept those out on purpose to make the initial change small, testable, and compatibility-focused.
Compatibility
Backward compatibility is handled in the internal dispatch layer.
If a session implementation supports the optional
wrapperparameter, the runner passes the currentRunContextWrapper.If it does not, the runner falls back to the existing call shape, so older custom session implementations continue to work without modification.
Tests
Added coverage for:
wrapperget_items/add_itemsreceive the currentRunContextWrapperRunner.runRunner.run_syncRunner.run_streamedRan:
tests/test_session.pytests/test_agent_runner_streamed.pytests/test_soft_cancel.pytests/test_hitl_session_scenario.pytests/extensions/memory/test_encrypt_session.pytests/memory/test_openai_responses_compaction_session.pyResult:
Static checks:
pyright→ 0 errorsruff check→ passed