fix(models): accept non-standard roles in AnthropicMessage so normalization can run - #249
Open
Navapon wants to merge 1 commit into
Open
fix(models): accept non-standard roles in AnthropicMessage so normalization can run#249Navapon wants to merge 1 commit into
Navapon wants to merge 1 commit into
Conversation
…zation can run Newer Claude Code clients inline role="system" messages (e.g. SessionStart hook context) in the messages array. AnthropicMessage.role was Literal["user", "assistant"], so Pydantic rejected these requests with 422 before normalize_message_roles() in converters_core.py could coerce them to "user". Widen role to str, matching the OpenAI path (ChatMessage.role is already str) so both APIs behave consistently: unknown roles pass validation and are normalized downstream before reaching the Kiro API. Fixes jwadow#190 Fixes jwadow#226
|
Thanks for the PR! 🎉 Before merge, we need a one-time CLA confirmation. Full CLA text: Please reply once with: You need to write once, all further messages from me can be ignored. |
Author
|
I have read the CLA and I accept its terms |
Navapon
marked this pull request as ready for review
July 17, 2026 03:13
|
@Navapon could you give a try to this gateway: https://github.com/ankitcharolia/kiro-gateway It supports all necessary endpoints for OpenAI and Anthropic. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Widens
AnthropicMessage.rolefromLiteral["user", "assistant"]tostrinkiro/models_anthropic.py, so requests containing inline non-standard roles (e.g.role: "system"sent by newer Claude Code clients for SessionStart hook context) are accepted and handled by the gateway's existing normalization pipeline instead of being rejected with:Fixes #190
Fixes #226
Why this approach
normalize_message_roles()inconverters_core.py(added for issue BUG: API error calling from codex App #64) coerces any non-user/assistant role touserbeforeensure_alternating_roles(). The strictLiteralon the Anthropic model rejects the request before that logic can run. This PR removes the gate rather than adding a new mechanism, handling the entire class of unknown-role issues ("system", "developer", future roles), not just one value.ChatMessage.roleisstr(kiro/models_openai.py:78), andtests/unit/test_routes_openai.py::test_validates_invalid_rolealready asserts unknown roles pass validation. This brings the Anthropic path into parity. Streaming and non-streaming share the same request model, so both modes are covered by one change.systemmessages; the 422s disappeared and conversations flow normally.Behavior change (intentional)
tests/unit/test_routes_anthropic.py::TestMessagesValidation::test_validates_invalid_rolepreviously asserted unknown role → 422. That assertion enforced the bug, so it is replaced bytest_accepts_non_standard_role(mirroring the existing OpenAI-side test of the same name and expectation).Test coverage
tests/unit/test_models_anthropic.py— newTestAnthropicMessageRoleAcceptance(5 tests): user/assistant regression guards,systemrole (primary repro), arbitrary unknown role, and a fullAnthropicMessagesRequestwith an inline system message mid-conversation (exact Claude Code shape).tests/unit/test_converters_anthropic.py— new end-to-end testtest_inline_system_role_message_is_normalized: converts a request with an inline system message viaanthropic_to_kiro()and asserts the Kiro history contains onlyuserInputMessage/assistantResponseMessageentries and the system content survives as a user message.tests/unit/test_routes_anthropic.py— route-level tests:test_accepts_non_standard_roleandtest_accepts_inline_system_role_message(no 422 at/v1/messages).pytest -q: 1698 passed (full unit + integration suite).Related PRs
I'm aware several open PRs address the same 422 (#191, #195, #197, #199, #205, #206, #207, #210, #234...). This one aims to be the minimal, test-covered variant that reuses the gateway's existing normalization rather than introducing new hoisting/merging logic — and it updates the route test that previously locked in the strict behavior. The merge-into-system approach discussed in #190 (comment by @W0n9) is a reasonable semantic refinement that could layer on top of this change later; happy to close this in favor of whichever direction the maintainer prefers.