[Manohar Paturi (ManoharPaturi)]

Motivation & Context

add_items signals both a missing conversation and unsupported item content with a plain ValueError, and the route answers every ValueError with 404. Posting a video-only item to an existing conversation therefore reported Not Found, which reads as a wrong conversation id rather than a request the client can fix.

Description & Review Guide

• What are the major changes? Unsupported item content now raises a ConversationItemValidationError (a ValueError subclass), and the items route maps it to 400 while a missing conversation stays 404.
• What is the impact of these changes? Only the status code for the malformed item case changes. Other handlers catching ValueError are unaffected because the new exception subclasses it.
• What do you want reviewers to focus on? The route test covers both cases against the HTTP surface: unsupported content on an existing conversation is 400, and an unknown conversation id is still 404.

Related Issue

Fixes #9506

AI Assistance

• No material AI assistance was used.
• This is an AI-assisted contribution. I reviewed, understood, and verified all submitted content and accept responsibility for it.

[Eduard van Valkenburg (eavanvalkenburg)]

The 400/404 exception split and route coverage are correct; only the import block needs to be made lint-clean.

[Manohar Paturi (ManoharPaturi)]

Thanks Eduard van Valkenburg (@eavanvalkenburg). I cleaned up the import block in _server.py so ConversationItemValidationError sits properly with the local package imports rather than interrupting the FastAPI group, and verified the file passes Ruff checks.

[Manohar Paturi (ManoharPaturi)]

The Public API Compatibility check fails on every open PR in this repo right now (e.g. #9602, #9607, and all of this author's open PRs), so it looks like a repo-wide CI issue rather than something introduced by this change.

The job crashes inside griffe while loading the base (main) snapshot, before it compares the PR changes — the traceback points at the base copy of the package:

griffe._internal.exceptions.AliasResolutionError: Could not resolve alias
agent_framework._workflows._edge.EdgeGroup.builtin_type pointing at builtins.type
(in /tmp/python-api-base-.../python/packages/core/agent_framework/_workflows/_edge.py:315)

Example run: https://github.com/microsoft/agent-framework/actions/runs/39810016173/job/119933441343

None of the changes in this PR touch _workflows/_edge.py. Filed an issue for the CI failure; happy to help debug if useful.

[Somay kousis (Somay-kousis)]

The 400 for unsupported content works: the new test fails on the merge base (ebcf4db, 404) and passes on de4ed7b. One malformed-item case on the same route still answers 404 for an existing conversation, and it also breaks that conversation for later reads.

Unknown role returns 404 and is stored anyway (python/packages/devui/agent_framework_devui/_conversations.py:329, :349, :367)

role is read from the item with no check (line 329). Message(role=...) accepts any string, so the message passes the new content check and is appended to stored_messages at line 349. Only afterwards does OpenAIMessage(role=cast(MessageRole, msg.role)) at line 367 reject it. That pydantic ValidationError is a ValueError, so the route falls through to the except ValueError at _server.py:1090 and answers 404.

Synthetic repro against an existing conversation (MockAgent server from test_server.py):

POST /v1/conversations/{id}/items  {"items":[{"type":"message","role":"bogus","content":"hi"}]}
1 POST item role=bogus: 404
2 GET conversation: 200
3 POST valid user item: 200
4 GET items: 404 1 validation error for Message
5 stored messages: [('bogus', 'hi'), ('user', 'valid')]

So a client gets "not found" for a conversation that GET /v1/conversations/{id} says exists, which is the split the new test docstring promises ("404 stays reserved for a missing conversation"). And because the rejected message was already stored, list_items rebuilds it on every later read: GET /items stays 404 for that conversation from then on.

This is not new in this PR. Same output on ebcf4db. But it is the same bug class as #9506, on the loop this PR already edits, so it seems worth closing here. Suggestion: inside the per-item loop, before line 349, check role against the allowed MessageRole values and raise ConversationItemValidationError if it isn't one. Then the route returns 400 and nothing gets stored. A second case in the new test with "role": "bogus" that asserts 400 and then GET /items == 200 would cover it.

What I ran

• New test from head against base src: 1 failed (404 != 400). Against head src: passes.
• test_server.py + test_conversations.py on head: 70 passed.
• Probe above on head and base: identical output.
• Also checked: a valid item followed by an unsupported one returns 400 on head (404 on base) and stores nothing, since the raise happens before extend.

Not checked: other ConversationStore implementations, the /v1/responses path with a conversation that holds a bad role, and "items": "hello" (500 on both base and head, a separate pre-existing issue).

[Manohar Paturi (ManoharPaturi)]

Confirmed with your probe shape first: on the previous head the bogus role item answered 404 and, because the message was already in stored_messages, GET items kept failing for that conversation afterward, exactly the stuck state you described.

e35e8f7 validates the role inside the per item loop, before anything is stored: anything outside system, user, assistant, and tool raises ConversationItemValidationError, so the route answers 400 and the conversation keeps reading normally. The new test case posts role bogus, asserts 400 with the role named in the detail, and then asserts GET items is still 200; it fails on the previous head and passes now. Both devui suites pass at 70 tests and ruff is clean on the touched files.

[Somay kousis (Somay-kousis)]

Reran on e35e8f7: role bogus now gets 400, nothing is stored, and GET /items stays 200 afterwards. Thanks.

[Manohar Paturi (ManoharPaturi)]

Eduard van Valkenburg (@eavanvalkenburg) the workflow runs for the new head 30f436c are waiting on the fork approval gate (every workflow shows action_required before any job starts), so someone with write access needs to approve them for the CI to run. The fix for your developer role finding is in that head: the accepted roles are now derived from the module's own MessageRole via get_args, so developer, critic, and discriminator pass again while bogus roles still get 400, and the test posts and reads back all four common roles. Details in the thread reply.