fix: sanitize internal fields from API messages

[cutepawss]

Fixes #134

Problem

When using Mistral (or any strict OpenAI-compatible provider) as a direct API endpoint, the second message always fails with:

Error code: 422 - {'detail': [{'type': 'extra_forbidden',
  'loc': ['body', 'messages', 2, 'assistant', 'finish_reason'],
  'msg': 'Extra inputs are not permitted', 'input': 'stop'}]}

The first message works fine because there's no assistant message in history yet. On the second turn, the previous assistant response — carrying internal fields like finish_reason — gets sent back to the API, which rejects it.

Root Cause

_build_assistant_message() adds finish_reason and reasoning to every assistant message dict for trajectory saving and session logging. The API preparation code stripped reasoning but forgot finish_reason. Additionally, _handle_max_iterations() sent messages with no sanitization at all.

This is a broader design issue: the code used a fragile blacklist (manually popping individual fields), which breaks whenever a new internal field is added.

Fix

Added a whitelist-based _sanitize_for_api() helper that keeps only standard OpenAI-compatible fields per message role:

_API_FIELDS_BY_ROLE = {
    "system":    {"role", "content"},
    "user":      {"role", "content", "name"},
    "assistant": {"role", "content", "tool_calls", "name", "refusal",
                  "reasoning_content", "reasoning_details"},
    "tool":      {"role", "content", "tool_call_id", "name"},
}

Applied consistently across all 3 API call sites:

1. Main conversation loop — replaced inline blacklist
2. Memory flush — replaced inline blacklist
3. Max iterations summary — was completely unsanitized (also fixed)

reasoning_content and reasoning_details are kept in the whitelist for providers that support multi-turn reasoning (OpenRouter, Moonshot AI). The original messages list is unchanged, so trajectory saving, session DB logging, and display continue to work.

Testing

Added 8 tests in tests/agent/test_sanitize_api.py:

Test	What it verifies
test_finish_reason_stripped	The actual bug — finish_reason removed
test_reasoning_becomes_reasoning_content	reasoning → reasoning_content conversion
test_reasoning_content_not_added_when_empty	No spurious field when reasoning is None
test_flush_sentinel_stripped	Internal _flush_sentinel on user msgs removed
test_standard_assistant_fields_preserved	tool_calls, reasoning_details pass through
test_tool_message_preserves_tool_call_id	Tool msgs keep tool_call_id, drop extras
test_system_message_minimal	System msgs only keep role + content
test_unknown_role_defaults_to_role_content	Unknown roles get safe defaults

$ python3 -m pytest tests/agent/test_sanitize_api.py -v
8 passed in 3.13s

$ python3 -m pytest tests/ -v
626 passed, 1 failed (pre-existing, unrelated), 9 deselected

[Bartok9]

LGTM! The whitelist-based sanitization is the right approach.

Key benefits:

• Forward-compatible (new internal fields won't accidentally leak)
• Handles all message roles properly
• Fixed the edge case in _handle_max_iterations()

The _API_FIELDS_BY_ROLE dict is clean and easy to extend if needed.

Tested locally with Mistral - the 422 errors are resolved. 🎉

[Bartok9]

LGTM! The whitelist-based sanitization is the right approach.

Key benefits:

• Forward-compatible (new internal fields won't accidentally leak)
• Handles all message roles properly
• Fixed the edge case in _handle_max_iterations()

Tested logic - the 422 errors should be resolved.

[teknium1]

@cutepawss are we aware of they accept extra_headers kwarg? Thats needed to control a lot of models' reasoning effort, whether they think at all, etc. I'm worried this isn't strict OAI spec but strictly mistral spec?

[cutepawss]

@teknium1
Good question! This fix only sanitizes message-level fields (the dicts inside the messages array), not API call-level kwargs.

extra_headers, extra_body, reasoning config, etc. are passed as kwargs to chat.completions.create() — they're completely untouched by _sanitize_for_api(). For example, this continues to work exactly as before:

client.chat.completions.create(
    model="...",
    messages=api_messages,  # ← only this is sanitized
    extra_body={"reasoning": {"effort": "high"}},  # ← untouched
    extra_headers={"X-Custom": "value"},            # ← untouched
)

What we're stripping are internal bookkeeping fields like finish_reason that _build_assistant_message() adds to message dicts for trajectory saving and session logging. These are not valid fields on chat messages in any provider's spec — finish_reason belongs on the choice object in the API response, not on messages sent back to the API. Mistral just happens to be the first provider that strictly validates this, while others silently ignore the extra field.

The whitelist is based on the OpenAI Chat Completion message spec, plus known provider extensions (reasoning_content for Moonshot AI/Novita, reasoning_details for OpenRouter multi-turn reasoning). It's not modeled after Mistral's validation rules specifically.

[teknium1]

Okay but messages with OpenRouter require a reasoning_content or reasoning field to carry over reasoning content for instance. Scrubbing that would break interleaved reasoning in many models

[cutepawss]

@teknium1 Thanks for flagging this I want to make sure I'm not breaking anything here.

From what I can see, _sanitize_for_api() handles this by copying reasoning → reasoning_content before stripping:

if role == "assistant" and msg.get("reasoning"):
    api_msg["reasoning_content"] = msg["reasoning"]

So reasoning_content and reasoning_details both pass through the whitelist. This should match the current behavior in main (lines 1957-1965), just consolidated into one function.

This is also covered by the tests in the PR — test_reasoning_becomes_reasoning_content and test_standard_assistant_fields_preserved verify that reasoning is preserved as reasoning_content and reasoning_details passes through untouched.

That said; is there a provider or flow where reasoning needs to go back to the API as-is, without being converted to reasoning_content?
I may be missing a case there.