Pass a per-run framework context/state through the Session to every framework adapter - #552
Pass a per-run framework context/state through the Session to every framework adapter#552induwara-yaala wants to merge 9 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a first-class, per-run “framework context/state” dictionary that round-trips through Session under a reserved key (Session.Keys.FRAMEWORK_CONTEXT), is injected into each framework runner using the framework’s native mechanism, and is written back atomically only on successful completion (including streaming completion).
Changes:
- Introduces base
Runnerplumbing to load (deep-copy) and write-back a reservedframework_contextsession key, with picklability diagnostics. - Wires context injection/write-back across all framework adapters (OpenAI, LangGraph, Google ADK, Smolagents; CrewAI warns and ignores) and ensures the Langfuse traced LangGraph runner does not bypass the feature.
- Adds end-to-end docs, examples, and unit/regression tests covering load/write-back semantics, framework fidelity differences, and streaming disconnect behavior.
Reviewed changes
Copilot reviewed 40 out of 42 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| examples/cli/openai_context/README.md | New CLI demo documentation for OpenAI full round-trip context. |
| examples/cli/openai_context/pyproject.toml | New example project dependencies for OpenAI context demo. |
| examples/cli/openai_context/demo.py | OpenAI demo agent/tools + hooks seeding and displaying framework_context. |
| examples/cli/openai_context/demo_test.py | Example integration tests asserting cross-turn cart persistence. |
| examples/cli/openai_context/build.sh | Example uv build script (incl. local dist install path). |
| examples/cli/openai_context/.gitignore | Ignores example venv/dist artifacts. |
| examples/cli/langgraph_context/README.md | New CLI demo documentation for LangGraph declared-channel round-trip. |
| examples/cli/langgraph_context/pyproject.toml | New example project dependencies for LangGraph context demo. |
| examples/cli/langgraph_context/demo.py | LangGraph demo graph + hooks illustrating declared state channels. |
| examples/cli/langgraph_context/demo_test.py | Example integration tests asserting cross-turn cart persistence. |
| examples/cli/langgraph_context/build.sh | Example uv build script (incl. local dist install path). |
| examples/cli/langgraph_context/.gitignore | Ignores example venv/dist artifacts. |
| docs/specs/526-framework-runtime-context/spec.md | Implementation spec describing runner helpers + per-framework mapping and tests. |
| docs/specs/526-framework-runtime-context/design.md | Design requirements for reserved key, atomic write-back, and fidelity differences. |
| docs/docs/frameworks/smolagents.md | Documents smolagents filtered (pre-seeded keys only) round-trip behavior. |
| docs/docs/frameworks/openai.md | Documents OpenAI full round-trip behavior and links to new example. |
| docs/docs/frameworks/multi-framework.md | Documents cross-framework fidelity differences and portability guidance. |
| docs/docs/frameworks/langgraph.md | Documents LangGraph “declared channels only” round-trip behavior. |
| docs/docs/frameworks/google-adk.md | Documents ADK round-trip semantics (strip internal keys; tool-added keys survive). |
| docs/docs/frameworks/crewai.md | Documents CrewAI unsupported behavior (warn + ignore). |
| docs/docs/core-concepts/session.md | Adds framework_context reserved-key docs and constraints. |
| docs/docs/core-concepts/runner.md | Adds runner-level description + fidelity table for framework_context. |
| ak-py/tests/test_trace_langfuse_langgraph.py | Regression tests ensuring Langfuse traced LangGraph path preserves framework_context behavior. |
| ak-py/tests/test_tool_adk.py | Updates mocks to match ADK runner _setup_session_context return shape. |
| ak-py/tests/test_smolagents_runner.py | Adds unit tests for smolagents injection + filtered write-back + error path. |
| ak-py/tests/test_openai_runner.py | Adds unit + streaming tests for OpenAI injection/write-back and disconnect atomicity. |
| ak-py/tests/test_langgraph_runner.py | Adds unit + streaming tests for LangGraph input spread + write-back + disconnect atomicity. |
| ak-py/tests/test_framework_context.py | New base-runner plumbing tests: deep copy, merge, absent key behavior, picklability errors. |
| ak-py/tests/test_crewai_runner.py | Adds tests asserting CrewAI warning behavior and preservation of stored key. |
| ak-py/tests/test_adk_runner.py | Adds tests for ADK injection and full stripped state write-back + error/absent cases. |
| ak-py/src/agentkernel/trace/langfuse/langgraph.py | Refactors traced runner to delegate to base runner (so it doesn’t bypass framework_context). |
| ak-py/src/agentkernel/framework/smolagents/smolagents.py | Injects additional_args conditionally + filtered write-back from agent.state. |
| ak-py/src/agentkernel/framework/openai/openai.py | Injects context= into OpenAI Agents SDK calls + writes back mutated context on success. |
| ak-py/src/agentkernel/framework/langgraph/langgraph.py | Spreads caller keys into graph input + writes back declared channels (run + stream). |
| ak-py/src/agentkernel/framework/crewai/crewai.py | Warns and ignores non-empty framework_context (unsupported by CrewAI). |
| ak-py/src/agentkernel/framework/adk/adk.py | Seeds injected keys into ADK session state + reads back stripped state for write-back (run + stream). |
| ak-py/src/agentkernel/core/base.py | Adds reserved key enum + runner helpers for load/store/picklability diagnostics. |
| .github/test-config.yaml | Includes the new CLI examples in the e2e test inventory. |
| .agents/skills/ak-dev-new-framework-integration/SKILL.md | Updates contributor guidance to wire framework_context in new adapters. |
| .agents/skills/ak-dev-architecture/SKILL.md | Updates architecture skill to document reserved key + runner helper responsibilities. |
Comments suppressed due to low confidence (1)
ak-py/src/agentkernel/core/base.py:262
- _store_framework_context early-returns on
session is None, but the signature typessessionasSession. Align the annotation with the actual supported usage so callers/tests don’t need to ignore typing.
def _store_framework_context(self, session: Session, incoming: dict | None, produced: Mapping[str, Any] | None) -> None:
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
amithad
left a comment
There was a problem hiding this comment.
Overall: strong PR. The implementation matches spec.md requirement-for-requirement (reserved key, deep-copy load, shallow merge, write-back-on-success placement in all five adapters, streaming atomicity, CrewAI warn-and-skip, picklability fail-fast), the test coverage is close to the spec's own testing plan, and the docs/skills/examples surfaces are all updated. CI is fully green, including the two new e2e examples.
Spec documents (reviewed before the code):
- Every
path:linecitation, config/default claim, and behavior description indesign.mdandspec.mdwas verified against the base branch — all of them check out. - One process
[question]inline ondesign.mdabout staging (design.md+spec.md+ implementation in one PR, noplan.md).
Findings (no blockers): 4 suggestions, 1 question, 2 nits — see inline comments.
Not anchorable to the diff:
docs/docs/examples/overview.mdlists the CLI examples but was not updated for the two new examples (openai_context,langgraph_context) — the docs-sync guidance calls this pitfall out explicitly. (The page already misses some older examples, e.g.openai_structured, so this could also be a follow-up that trues up the whole inventory.)
Positives worth noting:
- The langfuse LangGraph traced runner was the only traced runner that re-implemented the native call (all others already delegate to
super().run()), so this PR closes the only path whereframework_contextwould have silently no-opped — and adds a regression test for exactly that. - The LangGraph adapter writes
messageslast, fixing an ordering bug present in the spec's own sketch (see inline nit). - Both examples demonstrate the correct per-framework write patterns (OpenAI via
RunContextWrapper.context, LangGraph via a declared state channel), and their e2e tests assert on a deterministic post-hook line instead of fuzzy LLM text.
…ng and stripping of non-caller keys
… logging method to static
induwara-yaala
left a comment
There was a problem hiding this comment.
Overall: Solid, well-tested implementation of per-run framework_context across all five adapters — matches its own spec.md closely, and the base Runner plumbing (deep-copy load / shallow-merge write-back / picklability guard / atomic-per-turn placement) is correct and well covered by test_framework_context.py and the per-runner test files. CI is green.
Most of what I found had already been raised by the existing Copilot review and by @amithad's review, and both rounds have since been addressed in later commits (7483a29b, cc4a2b06, 45799c58, ce85aa4e, b1f15952):
- Copilot's
session: Session→Session | Nonetyping and missing dict type-check on_load_framework_context— fixed (core/base.py:241,255-260). - @amithad's lost-update-trap tool example in
session.md— replaced with a read-only example plus an explicit warning callout. - @amithad's missing langfuse consumer-changes entry, the ADK
stream()test gap, themessages-ordering contradiction in thespec.mdsketch, and the missinglanggraph.mdexample link — all fixed. - @amithad's
[question]on whetherdesign.md/spec.mdskipped their own review-cycle process is still open — the PR's "Related Issues" section is still blank and there's no note that the design was pre-approved elsewhere; worth resolving before merge but it's a process question, not a code issue.
One new finding below (CLI input handling) that wasn't covered by the prior rounds.
Findings
- 1 suggestion (see inline) —
ak-py/src/agentkernel/cli/cli.py: unrelated, undocumented, untested scope creep that also drops a working feature (readline history/line-editing).
No blockers. No duplicated findings posted (see above for what's already covered by prior reviews).
| @@ -1,9 +1,8 @@ | |||
| import asyncio | |||
| import logging | |||
| import readline # Enables line editing and history features for input() in the CLI | |||
There was a problem hiding this comment.
[suggestion] Dropping import readline removes line-editing/history for every interactive CLI user, silently and out of scope for this PR.
- Why:
readlinehas a documented import-time side effect (# Enables line editing and history features for input() in the CLI) that patchesinput()process-wide. Removing it disables arrow-key history and line editing for allCLIusers, not just the newframework_contextdemos — and neitherdesign.mdnorspec.mdmentions a CLI change at all. - The replacement (
_ainput, a daemon-threadinput()wrapper bridged back viacall_soon_threadsafe) is a legitimate way to stop the interactive loop from blocking the event loop, but it's a nontrivial concurrency primitive with no test coverage anywhere inak-py/tests/(there's notest_cli.py), and no explanation in the PR of why the CLI needed to change for a session-key feature. - Suggestion: either scope this out to its own PR with its own rationale/tests, or if it's required to unblock something in
examples/cli/adk_context(or the other_contextdemos), say so explicitly in the PR description/spec and restorereadline(importing it doesn't require callinginput()from the main thread — the hook is process-wide) so history/editing isn't lost.
Description
Pass a per-run framework context/state through the Session to every framework adapter
Type of Change
Related Issues
Changes Made
Pass a per-run framework context/state through the Session to every framework adapter
Testing
Checklist