Skip to content

docs(microsoft-agent-framework): 10-API Class Deep Dives Vol. 5 — source-verified 1.19.0 additions - #347

Open
CodeHalwell wants to merge 20 commits into
mainfrom
claude/intelligent-goldberg-jq8spu
Open

CodeHalwell wants to merge 20 commits into
mainfrom
claude/intelligent-goldberg-jq8spu

Conversation

@CodeHalwell

Copy link
Copy Markdown
Owner

Summary

Adds Class Deep Dives Vol. 5 for the Microsoft Agent Framework Python SDK, source-verified against agent-framework==1.19.0 (latest PyPI release, up from 1.18.0 covered in Vol. 4).

New file: microsoft_agent_framework_python_class_deep_dives_v5.md

Covers 10 previously undocumented public APIs, chosen to span a broad range of the framework's functionality:

# Class / Type Area
1 WorkflowEvent[DataT] Workflow event bus — all lifecycle, diagnostic, bookkeeping, and data events; typed factory methods; request_info JSON serialization
2 AgentContext Agent middleware pipeline context; timing, message injection, and result-override examples
3 MiddlewareBundle Indivisible middleware groups; agent-hooks factory pattern
4 ConversationSplit / ConversationSplitter Built-in LAST_TURN/FULL eval strategies; custom splitter Protocol
5 VectorStoreHistoryProvider Vector-backed history with multi-dimensional scoping, embeddings, compaction, and semantic search tool
6 MemoryStore (ABC) / MemoryFileStore Abstract memory harness interface; filesystem implementation; custom in-memory testing stub
7 MemoryTopicRecord Topic memory file: constructor, markdown round-trip, dict serialization, search helpers
8 WorkflowRunResult list[WorkflowEvent] subclass; get_outputs(), get_final_state(), status_timeline(), pending request detection
9 FunctionInvocationContext Function middleware context; progressive tool exposure (add_tools/remove_tools); argument sanitisation
10 ChatOptions Cross-provider TypedDict; default options, per-run overrides, structured output, Unpack type-safe forwarding

Each section includes:

  • Full constructor signature with parameter table
  • All meaningful methods with signatures and notes
  • 2–4 self-contained runnable code examples verified against 1.19.0 source
  • Notes on experimental feature flags where applicable

Updated: microsoft_agent_framework_python_comprehensive_guide.md

  • Latest version badge bumped to 1.19.0
  • API reference callout updated to reference Vol. 1–5 (50 APIs total)

🤖 Generated with Claude Code

https://claude.ai/code/session_01J66Vy9idLUFLqJfgqkM5uq


Generated by Claude Code

Add Class Deep Dives Vol. 5, source-verified against agent-framework
1.19.0 (latest PyPI release). Covers 10 previously undocumented public
APIs spanning a broad range of the framework:

1. WorkflowEvent[DataT] — unified event bus: all lifecycle, diagnostic,
   bookkeeping, and data-plane events via typed factory methods, property
   accessors for request_info, and JSON serialization round-trip.

2. AgentContext — mutable context through the agent middleware pipeline;
   demonstrates timing middleware, message injection, and result override.

3. MiddlewareBundle — indivisible middleware group for features that only
   uphold their contract when installed together (e.g. agent-hooks).

4. ConversationSplit / ConversationSplitter — built-in LAST_TURN/FULL
   enum strategies plus the structural Protocol for custom splitters.

5. VectorStoreHistoryProvider — vector-backed conversation history with
   multi-dimensional scoping, optional embeddings, compaction, and a
   semantic search_history tool.

6. MemoryStore (ABC) / MemoryFileStore — abstract backing store interface
   for the memory harness; concrete filesystem implementation; custom
   in-memory implementation example for testing.

7. MemoryTopicRecord — topic memory file: constructor, markdown
   round-trip, to_dict/from_dict, and search helpers.

8. WorkflowRunResult — list[WorkflowEvent] subclass; get_outputs(),
   get_final_state(), status_timeline(), and pending request detection.

9. FunctionInvocationContext — function middleware context including
   progressive tool exposure (add_tools / remove_tools) with all-or-nothing
   batch semantics; argument sanitisation middleware example.

10. ChatOptions — TypedDict of cross-provider request parameters;
    default_options, per-run overrides, structured output, and
    Unpack[ChatOptions] type-safe forwarding.

Also updates the comprehensive guide to reference Vol. 5 and bumps
the latest version badge to 1.19.0.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J66Vy9idLUFLqJfgqkM5uq
Copilot AI lite review requested due to automatic review settings September 21, 2026 09:40
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 21, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-21T11:30:10.040709Z b8fbc0a New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6fd12205a3

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

agent = Agent(client=client, name="summarizer",
instructions="Summarize the user's text in one sentence.")

workflow = WorkflowBuilder().add_agent(agent).build()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Construct workflows through the supported builder API

WorkflowBuilder requires a start_executor= argument and does not expose add_agent(), so this first advertised runnable example fails while constructing the builder; the same unsupported pattern is repeated in the streaming, basic-result, and multi-agent examples. Use WorkflowBuilder(start_executor=agent) for a single agent and connect multiple agents with the documented edge/chain methods.

Useful? React with 👍 / 👎.

instructions="Write a haiku.")
workflow = WorkflowBuilder().add_agent(agent).build()

async for event in workflow.stream("Cherry blossoms fall"):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Stream through Workflow.run

Workflow does not provide a stream() method, so this example still raises AttributeError after its builder construction is corrected. The documented streaming API is workflow.run("Cherry blossoms fall", stream=True), whose returned stream can be used in the async for loop.

Useful? React with 👍 / 👎.

Comment on lines +180 to +183
resumed = await workflow.respond(
request_id=req.request_id,
response=user_answer,
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Resume pending workflows with Workflow.run

Workflow has no respond() method, making this HITL example and the later pending-request example unusable. Responses must be supplied through workflow.run(responses={req.request_id: user_answer}, ...), with checkpoint information when required by the paused run.

Useful? React with 👍 / 👎.

"reply with PASS or FAIL followed by a one-sentence reason."
),
)
evaluator = LocalEvaluator(judge_agent=judge)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Pass checks rather than an agent to LocalEvaluator

LocalEvaluator accepts positional EvalCheck callables and has no judge_agent keyword, so this built-in-split example fails at construction. To demonstrate an LLM judge, wrap the judge call in an EvalCheck (for example with @evaluator) and pass that check to LocalEvaluator; its returned EvalResults should then be read through items, not the later result.results loop.

Useful? React with 👍 / 👎.

Comment on lines +393 to +399
from agent_framework._harness._hooks import create_agent_hooks_middleware
from agent_framework.openai import OpenAIChatClient

acknowledge_experimental_feature(ExperimentalFeature.AGENT_HOOKS)

bundle = create_agent_hooks_middleware(
hooks_endpoint=os.environ["AGENT_HOOKS_ENDPOINT"],

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Use the actual agent-hooks factory contract

This factory is publicly exported from agent_framework/implemented in _agent_hooks, not _harness._hooks, and its contract takes one or more interceptor objects rather than a hooks_endpoint keyword. Consequently the example fails either at import or at invocation; replace it with an interceptor-based create_agent_hooks_middleware(interceptors, ...) example.

Useful? React with 👍 / 👎.

Comment on lines +751 to +753
provider = FileMemoryProvider(
memory_store=store,
memory_agent=Agent(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Pair MemoryFileStore with MemoryContextProvider

MemoryFileStore backs MemoryContextProvider, whereas FileMemoryProvider uses the separate AgentFileStore abstraction and does not accept memory_store or memory_agent. This example therefore raises a constructor error and also teaches the wrong memory model; instantiate MemoryContextProvider(store=store, ...) for the topic/index store shown here.

Useful? React with 👍 / 👎.

Comment on lines +1169 to +1170
async def process(self, context: FunctionInvocationContext, call_next):
for value in (context.arguments or {}).values():

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Handle validated BaseModel tool arguments

For normally validated tool calls, context.arguments may be a Pydantic BaseModel, which has no .values() method, so this guard raises AttributeError instead of inspecting or executing the tool. Normalize models with model_dump() while retaining the mapping path before iterating over argument values.

Useful? React with 👍 / 👎.

Comment on lines +1281 to +1283
1. **`Agent` constructor `default_options`** — applied on every run unless overridden.
2. **Per-run override** via `agent.run(..., **options)`.
3. **`Unpack[ChatOptions]` function signatures** for type-safe option forwarding.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Forward ChatOptions through the options parameter

Agent.run applies per-call chat settings through its options= mapping, not by expanding ChatOptions into top-level keywords. As written, this guidance and the examples using temperature=, model=, max_tokens=, or **override do not perform the documented option merge and may be rejected or forwarded as unrelated runtime kwargs; use agent.run(prompt, options=options) instead.

Useful? React with 👍 / 👎.

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.

Copilot review overview

🟡 Changes recommended

One or more issues must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 High severity · 12 Medium severity · 5 Low severity

Open (18)
What changed in this PR

Adds Vol. 5 of the Microsoft Agent Framework Python API deep dives and updates the comprehensive guide for version 1.19.0.

Changes:

  • Documents workflow, middleware, memory, evaluation, vector history, and chat option APIs.
  • Updates version metadata and links to include Vol. 5.
  • Adds runnable examples and 1.19.0 notes.
File Description
src/​content/​docs/​microsoft-agent-framework-guide/​python/​microsoft_agent_framework_python_comprehensive_guide.md Updated as part of this pull request.
src/​content/​docs/​microsoft-agent-framework-guide/​python/​microsoft_agent_framework_python_class_deep_dives_v5.md Updated as part of this pull request.

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

All bugs were in code examples in the Vol. 5 class deep dives doc:

- WorkflowBuilder: use start_executor= kwarg; replace .add_agent()/.add_agent()
  chain with .add_edge() since add_agent() does not exist
- Streaming: use workflow.run(..., stream=True) instead of workflow.stream()
- HITL resume: use workflow.run(responses={id: answer}, checkpoint_id=...)
  instead of the non-existent workflow.respond()
- LocalEvaluator: pass EvalCheck callables positionally; access .items not
  .results on EvalResults
- create_agent_hooks_middleware: fix import to agent_framework (not
  _harness._hooks) and use interceptors= arg, not hooks_endpoint=
- MemoryFileStore: pair with MemoryContextProvider(store=...) not
  FileMemoryProvider(memory_store=...) — MemoryFileStore backs
  MemoryContextProvider, not FileMemoryProvider
- FunctionInvocationContext.arguments: handle BaseModel via model_dump()
  before iterating values
- ChatOptions per-run: pass options={...} mapping, not bare **kwargs

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J66Vy9idLUFLqJfgqkM5uq
Copilot AI review requested due to automatic review settings September 21, 2026 09:51
Nine additional corrections to code examples and prose:

- include_status_events=True needed on workflow.run() for status/failed
  events to appear in the iterable list; without it they are only in
  status_timeline()
- result.usage_details not result.usage (AgentResponse has no .usage)
- AgentResponse(messages=...) not message= (wrong kwarg name)
- LocalEvaluator.evaluate() has no split= param; split_strategy belongs
  on EvalItem(split_strategy=ConversationSplit.LAST_TURN)
- VectorStoreHistoryProvider method table: show state=None default
- Structured output: use result.value (validated model), not
  result.text + model_validate_json()
- Fix mismatched quote in path-traversal note (``.."`  → `..`)
- typing.Unpack requires 3.11+; use typing_extensions for 3.10 compat
- Update stale "See also" link version string to 1.19.0

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J66Vy9idLUFLqJfgqkM5uq

Copy link
Copy Markdown
Owner Author

All 18 Copilot review findings addressed across two fix commits (daab4d0 and 5ec22819). Summary:

First commit (8 fixes — original Codex review)

Finding Fix
WorkflowBuilder().add_agent() doesn't exist WorkflowBuilder(start_executor=agent).build(); multi-agent chain uses .add_edge()
workflow.stream() doesn't exist workflow.run(..., stream=True)
workflow.respond() doesn't exist (×2) workflow.run(responses={id: answer}, checkpoint_id=...)
LocalEvaluator(judge_agent=...) wrong ctor LocalEvaluator(eval_check_fn) with an EvalCheck callable
create_agent_hooks_middleware wrong import + arg Import from agent_framework; use interceptors=[...]
FileMemoryProvider(memory_store=...) wrong provider MemoryContextProvider(store=store, ...)
context.arguments.values() fails for BaseModel model_dump() guard before iterating
agent.run(temperature=...) bare kwargs agent.run(..., options={"temperature": ...})

Second commit (9 fixes — Copilot review round 1)

Finding Fix
typing.Unpack requires 3.11+ from typing_extensions import Unpack
status/failed events not in result list by default Added include_status_events=True to workflow.run() with a note
result.usage → AttributeError result.usage_details
AgentResponse(message=...) wrong kwarg AgentResponse(messages=...)
evaluator.evaluate(split=...) no such param Moved split_strategy= onto EvalItem(split_strategy=ConversationSplit.LAST_TURN)
Method table shows state as required Changed to state=None
Structured output: result.text + JSON parse result.value (validated Pydantic model)
Mismatched quote in path-traversal note Fixed `.."` → `..`
Stale "See also" version string (1.18.0) Updated to 1.19.0

Generated by Claude Code

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5ec228193e

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +318 to +320
context.result = AgentResponse(
messages=Message.from_assistant(self._text),
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Pass a sequence to AgentResponse

AgentResponse.messages expects a sequence of Message objects, but this passes a single Message. The mocked response therefore cannot be consumed through normal sequence operations such as the .text accessor; wrap the assistant message in a list.

Useful? React with 👍 / 👎.

Comment on lines +403 to +406
# An interceptor is any callable matching the Interceptor protocol.
def my_interceptor(event_type: str, payload: dict) -> dict:
print(f"[Hook] event={event_type!r}")
return payload

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Implement the interceptor protocol

Although the earlier import and factory argument were corrected, the newly supplied plain function still does not implement the agent-hooks Interceptor protocol, which expects an object exposing intercept(context) and returning a verdict. The emitter cannot invoke this (event_type, payload) -> dict callable as documented; use an interceptor class such as the EgressGuard pattern already shown in Vol. 4.

Useful? React with 👍 / 👎.

Comment on lines +465 to +467
response_text = " ".join(
m.get("content", "") if isinstance(m, dict) else (m.text or "")
for m in item.response

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Treat EvalItem.response as text

After EvalItem applies its conversation split, item.response is already the normalized response string, not a list of message objects. This loop iterates over individual characters and then accesses m.text, so the advertised evaluator raises AttributeError; inspect item.response directly.

Useful? React with 👍 / 👎.

for m in item.response
)
passed = "paris" in response_text.lower()
return CheckResult(passed=passed, reason="Response mentions Paris" if passed else "Missing 'Paris'")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Supply the required check name

CheckResult requires a check_name in addition to passed and reason. Because factual_check is passed directly to LocalEvaluator rather than wrapped by a decorator that could provide metadata, this constructor call raises before the check can return; add a stable name such as check_name="factual_check".

Useful? React with 👍 / 👎.

Comment on lines +485 to +487
results = await evaluator.evaluate(items=[item])
for r in results.items:
print(r.passed, r.reason)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Read pass/fail data from item scores

The fresh correction to iterate over results.items is incomplete: each entry is an evaluation item result with fields such as status and scores, not a CheckResult with passed and reason. Once the check itself succeeds, this print still raises AttributeError; print r.status and inspect the entries in r.scores for per-check results and reasons.

Useful? React with 👍 / 👎.

Comment on lines +511 to +512
# Use exactly like a built-in split:
# evaluator.evaluate(items=items, split=split_before_tool_call)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Attach custom splitters to EvalItem

LocalEvaluator.evaluate() has no split parameter, as the corrected example immediately above also notes. Following this usage comment therefore raises an unexpected-keyword error; construct each EvalItem with split_strategy=split_before_tool_call instead.

Useful? React with 👍 / 👎.

Comment on lines +764 to +766
provider = MemoryContextProvider(
store=store,
memory_agent=Agent(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Remove the unsupported memory_agent argument

After switching this example from FileMemoryProvider to MemoryContextProvider, the unsupported memory_agent keyword remains. MemoryContextProvider accepts store and options such as consolidation_client, but not a separate agent, so this corrected example still fails during provider construction.

Useful? React with 👍 / 👎.

Seven additional API accuracy fixes to code examples:

- AgentResponse(messages=[...]) — wrap single Message in a list;
  messages= accepts Message | Sequence[Message] but list is safer
- create_agent_hooks_middleware: replace plain function stub with a
  proper Interceptor subclass (agent_hooks package); plain callables
  do not satisfy the Interceptor protocol
- factual_check EvalCheck: item.response is already a str (joined
  assistant text); was incorrectly iterating it as a list of messages
- CheckResult: add required check_name= field
- results.items loop: EvalItemResult has is_passed/scores, not
  passed/reason; print r.status, r.is_passed, and r.scores entries
- Custom ConversationSplitter comment: split_strategy goes on EvalItem,
  not as a split= kwarg on evaluate()
- MemoryContextProvider: remove unsupported memory_agent= kwarg;
  use consolidation_client= instead

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J66Vy9idLUFLqJfgqkM5uq

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 72edd2be37

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +483 to +486
conversation=[
{"role": "user", "content": "What is the capital of France?"},
{"role": "assistant", "content": "The capital of France is Paris."},
],

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Construct EvalItem conversations from Message objects

EvalItem.conversation expects list[Message], and ConversationSplit.LAST_TURN inspects message attributes such as role; these raw dictionaries are not normalized by EvalItem, so accessing item.response in factual_check fails before the check runs. Import Message and construct the user and assistant entries as Message instances, as the later FULL example does.

Useful? React with 👍 / 👎.

Comment on lines +400 to +402
# agent_hooks Interceptor objects come from the agent-hooks package.
# Install it: pip install agent-hooks
from agent_hooks import Interceptor, InterceptionContext, Verdict

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Install the distribution that provides agent_hooks

Fresh evidence after the earlier interceptor correction is this newly added install command: the agent_hooks import is supplied by the agent-hooks-sdk distribution (the verified Vol. 4 example uses pip install --pre agent-hooks-sdk), not agent-hooks. A reader following this advertised runnable example therefore cannot obtain the imported SDK; update both the command and package wording.

Useful? React with 👍 / 👎.

Comment on lines +1100 to +1105
for req_event in result.get_request_info_events():
print(f"Workflow is asking ({req_event.source_executor_id}): {req_event.data}")
answer = input("Your answer: ")
result = await workflow.run(
responses={req_event.request_id: answer},
checkpoint_id=checkpoint_id,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Resume concurrent requests with one response map

When a paused workflow has multiple pending requests, this resumes once per request while repeatedly loading the same checkpoint_id. Each invocation therefore starts from the original checkpoint rather than retaining responses supplied by earlier loop iterations, so the returned result can remain pending or lose prior answers. The corrected code should first collect every answer into one {request_id: answer} mapping and then call workflow.run() once (and repeat against a fresh result only for later HITL rounds).

Useful? React with 👍 / 👎.

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.

Copilot review overview

🟡 Changes recommended

One or more issues must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 High severity · 2 Medium severity

Open (3)
Resolved since last review (18)
Previously missed (5)

In code that hasn't changed since last review

Medium severity Enable status events for non-streaming results

src/​content/​docs/​microsoft-agent-framework-guide/​python/​microsoft_agent_framework_python_class_deep_dives_v5.md:116

This non-streaming call leaves include_status_events at its default False, so the status and failure events handled in the loop below are not present in result. Pass include_status_events=True (or read result.status_timeline()) so this example can observe those events.

Medium severity Resolve topics by slug or human-readable name

src/​content/​docs/​microsoft-agent-framework-guide/​python/​microsoft_agent_framework_python_class_deep_dives_v5.md:815

The store writes records under record.slug, but get_topic only looks up the caller's value as a dictionary key. A caller using the documented human-readable topic name will therefore get FileNotFoundError; resolve both the slug and record.topic (or use one consistent key).

Medium severity Filter workflow events by agent ID, not display name

src/​content/​docs/​microsoft-agent-framework-guide/​python/​microsoft_agent_framework_python_class_deep_dives_v5.md:1069

Agent.name is a display name; workflow event executor_id is the agent's stable id, which is auto-generated here because neither agent sets one. This filter will normally return no writer outputs. Compare with writer.id or assign id="writer" when constructing the agent.

Low severity Correct the stale feature table version reference

src/​content/​docs/​microsoft-agent-framework-guide/​python/​microsoft_agent_framework_python_class_deep_dives_v5.md:1429

The comprehensive guide's feature table is headed What's new in 1.14.0, not 1.18.0. This stale version reference makes the new volume's See also text inaccurate; either update the referenced table or describe the page without claiming it is an 1.18.0 table.

Low severity Update the Python guide index for the 1.19.0 release

src/​content/​docs/​microsoft-agent-framework-guide/​python/​microsoft_agent_framework_python_comprehensive_guide.md:8

This version bump is not reflected on the Python guide landing page: python/index.mdx still advertises core 1.18.0 and Vol. 4 as the latest deep-dive card. Update that index metadata/card with the 1.19.0 and Vol. 5 links so users can discover the release being advertised here.

This issue also appears on line 17 of the same file.

Copilot AI review requested due to automatic review settings September 21, 2026 10:07
Three more corrections:

- EvalItem.conversation expects list[Message], not raw dicts;
  replaced dict literals with Message('user', [...]) and
  Message('assistant', [...]) constructors; added Message import
- agent-hooks-sdk install comment: correct package name is
  agent-hooks-sdk (pip install --pre agent-hooks-sdk)
- HITL detecting-pending-requests example: collect all pending
  answers into one responses={} map before calling workflow.run()
  once; looping with one request per run() call restarts from the
  same checkpoint on each iteration, dropping earlier answers

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J66Vy9idLUFLqJfgqkM5uq
The SqlInjectionGuard example incorrectly presented a substring blocklist
as SQL injection protection. A blocklist only covers a tiny subset of
payloads and must never be the primary defence; real SQL injection
protection requires parameterized queries.

Changes:
- Rename SqlInjectionGuard → ToolInputPatternGuard
- Add prominent security note explaining the example is illustrative
  pattern-matching, not an injection guard
- Update the error message to not imply SQL injection semantics

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J66Vy9idLUFLqJfgqkM5uq

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.

Copilot review overview

🟡 Changes recommended

Unresolved moderate issues affect dependencies, examples, storage isolation, and workflow behavior.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 Medium severity

Open (1)
Resolved since last review (3)
Previously missed (4)

In code that hasn't changed since last review

Medium severity Use Message instances instead of dictionaries in the evaluation example

src/​content/​docs/​microsoft-agent-framework-guide/​python/​microsoft_agent_framework_python_class_deep_dives_v5.md:486

This claimed runnable example passes dictionaries where EvalItem and the splitter contract expect list[Message]; the splitters access message attributes such as role and contents. Use Message instances here, as the evaluation guide does, or this example fails when evaluated.

Medium severity Normalize topic and slug keys across custom store operations

src/​content/​docs/​microsoft-agent-framework-guide/​python/​microsoft_agent_framework_python_class_deep_dives_v5.md:825

The custom store writes records under record.slug, but get_topic() and delete_topic() look up the caller's topic key. For a normal record such as topic='Travel Plans' and slug='travel-plans', the provider cannot read or delete the record it just wrote; normalize both forms consistently (or key the stub by record.topic).

Medium severity Use owner-specific transcript directories or reject unsupported storage

src/​content/​docs/​microsoft-agent-framework-guide/​python/​microsoft_agent_framework_python_class_deep_dives_v5.md:852

This test store returns one fixed transcript directory for every owner. If MemoryContextProvider writes transcript archives through this method, sessions for different users share the same files and can collide; the existing in-memory backend instead raises NotImplementedError for unsupported transcript directories. Return an owner-specific path or explicitly reject transcript storage.

Low severity Clarify that store enables provider-side conversation storage

src/​content/​docs/​microsoft-agent-framework-guide/​python/​microsoft_agent_framework_python_class_deep_dives_v5.md:1298

The store option is provider-side conversation storage, not persistence of a request for fine-tuning. The model-provider reference describes it as “Provider-side conversation storage (OpenAI Responses API, Foundry)” (microsoft_agent_framework_python_model_providers.md:410); the current note could lead users to enable it for the wrong purpose.

Copilot AI review requested due to automatic review settings September 21, 2026 10:12

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e42991f4de

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

---

Latest: agent-framework 1.18.0 | Guide verified against: 1.14.0 | Python 3.10+
Latest: agent-framework 1.19.0 | Guide verified against: 1.14.0 | Python 3.10+

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Update the landing page for 1.19.0 and Vol. 5

This version bump leaves the Python landing page inconsistent: index.mdx still labels 1.18.0 as the stable release in its metadata, version card, and “What's shipped” section, and its reference cards stop at Vol. 4 (index.mdx lines 3, 22, and 117–125). Readers entering through the main overview therefore see a stale version and cannot discover the newly added Vol. 5 page; update that page and its revision history alongside this change.

Useful? React with 👍 / 👎.

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.

Copilot review overview

🔵 Needs a closer look

Unresolved documentation inaccuracies and metadata/discoverability corrections remain.

Review effort: Lite
Findings: None

Resolved since last review (1)
Previously missed (4)

In code that hasn't changed since last review

Medium severity Resolve topic names and slugs in get_topic

src/​content/​docs/​microsoft-agent-framework-guide/​python/​microsoft_agent_framework_python_class_deep_dives_v5.md:829

Although this section says get_topic accepts a topic name or slug, the stub stores records by slug and only performs owner_topics.get(topic). Calling it with the public record.topic (for example, "Travel Plans") raises FileNotFoundError, so the advertised custom implementation does not satisfy the interface. Resolve both forms in the example.

Low severity Document deprecated data events in event table

src/​content/​docs/​microsoft-agent-framework-guide/​python/​microsoft_agent_framework_python_class_deep_dives_v5.md:65

The event table omits the deprecated data event produced by WorkflowEvent.emit(), even though this section says it covers all event kinds and explicitly mentions emit() above. The comprehensive guide documents .emit() as producing type "data"; add that entry (and any other variants covered by the claim) so readers can interpret the complete event stream.

Low severity Correct ChatOptions.store fine-tuning description

src/​content/​docs/​microsoft-agent-framework-guide/​python/​microsoft_agent_framework_python_class_deep_dives_v5.md:1306

ChatOptions.store is not a fine-tuning switch. The provider-neutral options documentation describes it as provider-side request/conversation storage (for example, the Responses API or Foundry), so this note can lead users to believe enabling it persists data for fine-tuning when it does not.

Low severity Correct Vol. 5 API count inconsistency

src/​content/​docs/​microsoft-agent-framework-guide/​python/​microsoft_agent_framework_python_comprehensive_guide.md:17

Vol. 5 has 10 numbered sections but exposes 12 distinct types because sections 4 and 6 each document two separate types. Therefore “50 APIs (48 classes and 2 functions)” is internally inconsistent: 50 is a grouped-section count, not the count implied by the class breakdown. Either count each exported type or describe these as 50 grouped API sections.

- Bump version references from 1.18.0 to 1.19.0 in frontmatter
  description, Version card, and "What's shipped" section
- Add Vol. 5 (1.19.0) LinkCard to the Reference section after Vol. 4
- Add 2026-09-21 / 1.19.0 entry to the revision history table

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J66Vy9idLUFLqJfgqkM5uq
Copilot AI review requested due to automatic review settings September 21, 2026 10:17

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.

Copilot review overview

🟡 Changes recommended

One or more issues must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 3 Medium severity · 1 Low severity

Open (4)

- Fix VectorStoreHistoryProvider comment: both runs use the same session,
  not a "later session" (line 685)
- Fix MemoryTopicRecord custom-store stub: key write_topic by record.topic
  (not record.slug) so get_topic/delete_topic lookups match (line 832)
- Fix WorkflowRunResult executor filter: add id="writer" to Agent so that
  ev.executor_id == "writer" matches the assigned ID, not an auto-generated
  one (line 1066)
- Fix ChatOptions store field description: it controls provider-side
  conversation storage (OpenAI Responses API, Foundry), not fine-tuning
  persistence (line 1306)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J66Vy9idLUFLqJfgqkM5uq
Copilot AI review requested due to automatic review settings September 21, 2026 10:23
Each resumed workflow.run() saves a fresh checkpoint forming a
previous_checkpoint_id chain rather than updating in-place. Query
checkpoint_storage.get_latest(workflow_name=workflow.name) before
every iteration to obtain the current checkpoint_id.

Also pass checkpoint_storage to the initial workflow.run() call and
change the function signature to accept checkpoint_storage instead
of a raw checkpoint_id string.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J66Vy9idLUFLqJfgqkM5uq

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.

Copilot review overview

🔵 Needs a closer look

One or more issues must be addressed before approval.

Review effort: Lite
Findings: None

Resolved since last review (1)

Copilot AI review requested due to automatic review settings September 21, 2026 10:59

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 966e18399e

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

# Each resumed run saves a FRESH checkpoint forming a previous_checkpoint_id
# chain, so refresh the checkpoint_id via get_latest() before every iteration.
while result.get_final_state() == WorkflowRunState.IDLE_WITH_PENDING_REQUESTS:
latest = await checkpoint_storage.get_latest(workflow_name=workflow.name)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep HITL resumption on the current checkpoint chain

When two invocations of the same named workflow share checkpoint storage, get_latest(workflow_name=...) can return the other invocation's checkpoint because it selects globally by workflow name rather than by run chain. The fresh evidence is this new workflow-name-only lookup: the handler can then submit one user's request IDs and answers while restoring another user's state, potentially corrupting or exposing workflow data. Identify the checkpoint whose pending request IDs match the current result, then follow that checkpoint's descendant chain rather than selecting the globally latest checkpoint.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in commit 18a39eef. The root cause is shared storage across concurrent runs rather than anything in the get_latest() call itself — get_latest(workflow_name=...) is correct when the store holds only one run's checkpoints.

The fix:

  • Added a comment at the top of run_and_handle() explaining that checkpoint_storage must be scoped to a single run, and why.
  • Added a handle_request() usage pattern that creates a fresh MemoryFileStore(path=f"./checkpoints/{run_id}") per invocation (where run_id = uuid.uuid4().hex), so get_latest() only ever sees checkpoints from that specific run even in multi-user deployments.

Generated by Claude Code

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.

Copilot review overview

🔵 Needs a closer look

One or more issues must be addressed before approval.

Review effort: Lite
Findings: None

Previously missed (2)

In code that hasn't changed since last review

Low severity Document deprecated WorkflowEvent.emit and data events

src/​content/​docs/​microsoft-agent-framework-guide/​python/​microsoft_agent_framework_python_class_deep_dives_v5.md:71

This is presented as the complete event-type table, but it omits the still-public deprecated WorkflowEvent.emit() factory and its data event. The note only says it is deprecated, so readers cannot learn its signature or distinguish it from output/intermediate; add a data row and the corresponding emit entry, or explicitly state that deprecated APIs are excluded.

Low severity Document inherited skip_excluded constructor option

src/​content/​docs/​microsoft-agent-framework-guide/​python/​microsoft_agent_framework_python_class_deep_dives_v5.md:585

The constructor is presented as complete, but it omits the inherited skip_excluded option. The repository documents this as a shared history-provider flag (microsoft_agent_framework_python_sessions.md:141-146; microsoft_agent_framework_python_comprehensive_guide.md:2823-2830), so users cannot discover how to exclude compaction-marked messages with this provider. Add skip_excluded: bool = False to the signature and parameter table.

get_latest(workflow_name=...) selects the newest checkpoint for that
name in a given store; two concurrent invocations sharing the same
store can pick up each other's checkpoints.

Add a comment explaining the single-run-per-storage requirement, and
add a handle_request() usage pattern that creates a fresh MemoryFileStore
scoped to a uuid run_id for each invocation.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J66Vy9idLUFLqJfgqkM5uq
Copilot AI review requested due to automatic review settings September 21, 2026 11:04

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 18a39eef8c

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

import asyncio
import uuid
from agent_framework import WorkflowRunState
from agent_framework.checkpointing import MemoryFileStore

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Use FileCheckpointStorage for HITL checkpoints

This runnable example imports a nonexistent checkpoint backend: workflow checkpoints use the top-level FileCheckpointStorage, while MemoryFileStore is the unrelated long-term-memory store and is not exported from agent_framework.checkpointing. Consequently the example fails at import before handle_request() can run; import FileCheckpointStorage from agent_framework and construct it with the run-scoped directory (positionally or via storage_path=).

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in commit 820264fe. MemoryFileStore (the long-term memory backend) has been replaced with FileCheckpointStorage (the correct workflow checkpoint backend, re-exported from agent_framework). The import is now from agent_framework import WorkflowRunState, FileCheckpointStorage and the constructor call is FileCheckpointStorage(f"./checkpoints/{run_id}").


Generated by Claude Code

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.

Copilot review overview

🟡 Changes recommended

One or more issues must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 Medium severity

Open (1)

MemoryFileStore is the long-term-memory backend; workflow checkpoints
use FileCheckpointStorage (re-exported from agent_framework).

Replace the incorrect import and constructor call in the HITL
handle_request() usage pattern.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J66Vy9idLUFLqJfgqkM5uq
Copilot AI review requested due to automatic review settings September 21, 2026 11:10

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.

Copilot review overview

🔵 Needs a closer look

One or more issues must be addressed before approval.

Review effort: Lite
Findings: None

Resolved since last review (1)
Previously missed (2)

In code that hasn't changed since last review

Medium severity Splitter crashes on plain-text content items

src/​content/​docs/​microsoft-agent-framework-guide/​python/​microsoft_agent_framework_python_class_deep_dives_v5.md:522

This custom splitter will raise AttributeError as soon as it encounters a plain-text content item, because Message.contents commonly contains strings and strings have no .type. The repository's equivalent splitter guards with getattr(c, "type", None) (microsoft_agent_framework_python_evaluation.md:264-266); use the same guard so normal text before the first tool call is handled.

Low severity Scope experimental note to agent hooks factory

src/​content/​docs/​microsoft-agent-framework-guide/​python/​microsoft_agent_framework_python_class_deep_dives_v5.md:335

The experimental note is applied to MiddlewareBundle itself, but the preceding Vol. 4 reference presents MiddlewareBundle as a public root export and places the optional/experimental AGENT-HOOKS requirement on create_agent_hooks_middleware() instead (microsoft_agent_framework_python_class_deep_dives_v4.md:763-787). As written, the plain bundle example either unnecessarily requires an acknowledgement or contradicts the actual gate; scope the note to the factory usage.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 820264fef5

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +873 to +876
def get_index_text(self, session, *, source_id, line_limit, line_length, index_entries=None):
entries = index_entries or self.rebuild_index(session, source_id=source_id,
line_limit=line_limit, line_length=line_length)
return "\n".join(e.to_pointer_line(max_length=line_length) for e in entries)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Apply line_limit when rendering the memory index

When this testing store contains more topics than the configured MemoryContextProvider.index_line_limit, get_index_text() joins every entry because neither this method nor rebuild_index() applies line_limit. Tests using the advertised store therefore miss the production provider's index bound and can construct an oversized memory prompt; truncate the rendered entries to the requested limit.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in commit 57c0f47f. Both rebuild_index() and get_index_text() now respect line_limit: rebuild_index slices topics[:line_limit] before building the entry list, and get_index_text slices entries[:line_limit] before joining, matching the production MemoryContextProvider bound.


Generated by Claude Code

Comment on lines +818 to +820
def _safe(s: str) -> str:
"""URL-safe base64 encode a string for use as a path component (injective, no collisions)."""
return base64.urlsafe_b64encode(s.encode()).decode().rstrip('=')

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve empty transcript scope components

Fresh evidence after the earlier path-encoding fix is that URL-safe base64 maps an empty ID to an empty path component, which the filesystem discards. If empty owner/source IDs are used, scopes such as (source_id="", owner="a") and (source_id="a", owner="") both resolve beneath _tmp/YQ, allowing their transcript files to collide despite the claimed isolation; reject empty IDs or encode them with a non-empty tagged representation.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in commit 57c0f47f. _safe() now prepends a constant "s" before the base64 output, so _safe("") → "s" and _safe("x") → "sYA" etc. — every input including the empty string maps to a unique, non-empty path component, and the (source_id="", owner="x") / (source_id="x", owner="") collision is eliminated.


Generated by Claude Code

…index

_safe("") returned "" causing (source_id="", owner="x") and
(source_id="x", owner="") to resolve to the same filesystem path.
Prefix the base64 output with "s" so every input including "" maps
to a non-empty, unique path component.

rebuild_index() and get_index_text() both ignored the line_limit
parameter, diverging from the production MemoryContextProvider's
index bound. Slice topics and entries to [:line_limit] in both.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J66Vy9idLUFLqJfgqkM5uq
Copilot AI review requested due to automatic review settings September 21, 2026 11:15

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.

Copilot review overview

🔵 Needs a closer look

One or more issues must be addressed before approval.

Review effort: Lite
Findings: None

Previously missed (1)

In code that hasn't changed since last review

Low severity Reconcile section, API, and aggregate count discrepancies

src/​content/​docs/​microsoft-agent-framework-guide/​python/​microsoft_agent_framework_python_class_deep_dives_v5.md:13

This volume contains 10 numbered sections but 12 public types: ConversationSplit/ConversationSplitter and MemoryStore/MemoryFileStore each contribute two classes. That conflicts with the comprehensive guide's new total of 52 APIs (50 classes + 2 functions) and with the 10-API title/link/changelog labels; either describe this as 10 deep dives covering 12 APIs, or update the aggregate counts and labels consistently.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 57c0f47fb5

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +61 to +63
| `"warning"` | `WorkflowEvent.warning(msg)` | `str` | — |
| `"error"` | `WorkflowEvent.error(exc)` | `Exception` | — |
| `"output"` | emitted by `ctx.yield_output()` | DataT | `executor_id` |

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Include the deprecated data event in the taxonomy

For workflows or custom executors that still use the supported-but-deprecated WorkflowEvent.emit(), the emitted event has type "data", yet this event table and the factory-method table skip directly from diagnostic events to "output". A consumer implementing event handling from this advertised complete taxonomy can therefore silently discard emitted payloads; add the "data" row and the deprecated emit() factory while retaining the migration note.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in commit b8fbc0af. Added "data" to the event type table with WorkflowEvent.emit(data) (deprecated) and DataT data type, and added the emit row to the factory-method table with its signature and a note to prefer ctx.yield_output().


Generated by Claude Code

"""URL-safe base64 encode a string for use as a path component (injective, no collisions).

A constant 's' prefix ensures empty strings produce a non-empty component
("s"), preventing (source_id="", owner="x") from colliding with

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Bound encoded transcript path components

When a source or owner ID exceeds roughly 190 UTF-8 bytes, this unbounded base64 value expands beyond the common 255-byte filesystem component limit, so get_transcripts_directory() raises ENAMETOOLONG before the provider can persist a transcript. Fresh evidence after the earlier path-encoding correction is that replacing truncation with base64 removed collision risk but added no length handling; use a tagged digest representation for long values while retaining the current encoding for short ones.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in commit b8fbc0af. _safe() now uses a tagged two-scheme approach: short values get "s" + url-safe base64 (as before), and values whose encoded form would exceed 200 bytes get "d" + SHA-256 hex (64 chars, well within the 255-byte limit). The distinct "s"/"d" prefixes prevent any cross-scheme collision. A _MAX_COMPONENT = 200 constant documents the threshold.


Generated by Claude Code


## See also

- [Python Comprehensive Guide](/microsoft-agent-framework-guide/python/microsoft_agent_framework_python_comprehensive_guide/) — framework overview, verified against 1.19.0

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Correct the comprehensive guide verification claim

This link labels the comprehensive guide as verified against 1.19.0, but that guide's front matter and header explicitly state that its content is verified against 1.14.0 and only identify 1.19.0 as the latest PyPI release. Readers can therefore mistake older examples for source-verified 1.19 behavior; change this label to 1.14.0 or reverify and update the linked guide.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in commit b8fbc0af. The label now reads "content verified against 1.14.0 (latest release badge: 1.19.0)", accurately reflecting that the guide's examples were validated against 1.14.0 while the version badge was updated to track the latest PyPI release.


Generated by Claude Code

…ide version

WorkflowEvent taxonomy was missing the deprecated 'data' event type
produced by WorkflowEvent.emit(). Add the row to both the event-type
table and the factory-method table with a deprecation note.

_safe() had no length bound; IDs > ~190 UTF-8 bytes would produce a
base64 component exceeding the 255-byte filesystem limit. Now falls
back to 'd' + SHA-256 hex (64 chars) for long values, keeping 's' +
base64 for short ones. Distinct prefixes prevent cross-scheme collisions.

The See Also link to the comprehensive guide incorrectly claimed it was
verified against 1.19.0; the guide's own header states 1.14.0. Updated
the label to reflect both: content verified against 1.14.0, latest
release badge 1.19.0.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J66Vy9idLUFLqJfgqkM5uq
Copilot AI review requested due to automatic review settings September 21, 2026 11:23

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.

Copilot review overview

🔵 Needs a closer look

One or more issues must be addressed before approval.

Review effort: Lite
Findings: None

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b8fbc0af72

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

self._topics: dict[tuple[str, str], dict[str, MemoryTopicRecord]] = {} # topic → record
self._slug_idx: dict[tuple[str, str], dict[str, str]] = {} # slug → topic
self._states: dict[tuple[str, str], dict] = {}
self._tmp = Path("/tmp/in-memory-store-transcripts")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Isolate transcript storage per store instance

After the source/owner scoping fixes, every InMemoryMemoryStore instance still uses the same fixed /tmp root. When parallel tests or two providers create separate store instances with the same source, owner, and session ID, MemoryContextProvider writes both instances' transcripts into the same directory even though their in-memory topics and state are independent, allowing stale or concurrent transcript data to contaminate tests. Allocate a unique temporary root per instance and clean it up with the store.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in commit 201c05b3. __init__ now calls tempfile.mkdtemp(prefix="in-memory-store-") so every store instance gets a unique temporary root, eliminating transcript directory sharing between parallel test instances. A cleanup() method using shutil.rmtree is added for teardown.


Generated by Claude Code

All instances previously shared a single /tmp/in-memory-store-transcripts
root, so parallel tests with matching source_id/owner/session IDs would
write into the same transcript directory despite having independent
in-memory state.

Use tempfile.mkdtemp(prefix="in-memory-store-") in __init__ to give
every instance a unique root, and add a cleanup() method that calls
shutil.rmtree for teardown.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J66Vy9idLUFLqJfgqkM5uq
Copilot AI review requested due to automatic review settings September 21, 2026 11:31

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.

Copilot review overview

🔵 Needs a closer look

One or more issues must be addressed before approval.

Review effort: Lite
Findings: None

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants