Skip to content

W.2: OpenAI chat decode module (finish + usage) shared by generate and stream (on #39; merges #29, #33) - #40

Open
cunninghamcard-bit wants to merge 8 commits into
exp/w1-openai-chat-encodefrom
exp/w2-openai-chat-decode
Open

cunninghamcard-bit wants to merge 8 commits into
exp/w1-openai-chat-encodefrom
exp/w2-openai-chat-decode

Conversation

@cunninghamcard-bit

Copy link
Copy Markdown
Owner

Unit W.2 (PHASES 1W; A9 W3). Branch cut from W.1 (#39, c4e8024); PR base is exp/w1-openai-chat-encode.

The branch also contains two merge commits (no force-push):

Both merged without conflicts.

Where the non-streaming finish mapping was

aimux-providers/src/openai/model.rs:331-338 (after the merges):

choice.finish_reason.as_deref().map(parse_finish_reason).unwrap_or(FinishReason { unified: Other, raw: None })

It passed parse_finish_reason as a function (.map(parse_finish_reason)), so a grep for parse_finish_reason( never found it. The "no finish reason gives Other" default was also written out twice: once there, and once in the stream path's finish (:748-751).

Change

New aimux-providers/src/openai/decode.rs, private module:

  • parse_finish_reason: moved from convert.rs, including A.1's function_call → ToolCalls. convert.rs keeps pub use super::decode::parse_finish_reason; so the public openai::convert::parse_finish_reason path still works.
  • finish_reason(Option<&str>): None gives Other with no raw value.
  • convert_usage: moved from model.rs, keeping D28 (cacheRead from nested cached_tokens only).

Call sites in model.rs:

generate stream
finish_reason :265 :628 (chunk), :676 (default)
convert_usage :268 :523

rg -n 'fn parse_finish_reason\|fn finish_reason\|fn convert_usage' aimux-providers/src/openai/ finds each function defined exactly once, all in decode.rs.

Tests

  • A.1 function_call_finish_reason_is_tool_calls and A.5's three cached-token tests moved with the code and pass as openai::decode::tests::*.
  • New: missing_finish_reason_is_other.
  • The OpenAI e2e (openai_upstream_e2e_test), factory (openai_factory_test, including W.1's encode test) and output round-trip tests all pass unchanged.
  • Fixture and cassette diff: zero.

Results (local linux x64)

W.1 (#39) W.2
cargo test --workspace 2896 / 0 / 7 2901 / 0 / 7 (+1 A.1, +3 A.5, +1 new)
fmt / clippy -D warnings ok ok
boundary script 0 0
gen_ts_types --check ok ok
Node ava (node 22) 87 / 1 baseline fail 87 passed / same 1 failure

Python was not run (no venv).

Matches AI SDK map-openai-finish-reason.ts:13-15; raw stays
"function_call". Unit test added.
Matches AI SDK @ai-sdk/openai@4.0.83 convert-openai-chat-usage.ts:28. Drops
UsageResponse.cached_tokens (top-level value stays in usage.raw). Moonshot
keeps its behaviour via compat ConvertUsage::MoonshotAI.
openai/decode.rs owns parse_finish_reason (moved from convert.rs, still
re-exported there), finish_reason(Option<&str>) (None -> Other, used by both
paths incl. the stream default) and convert_usage (moved from model.rs, D28
nested-only kept). A.1 and A.5 unit tests move with them.
…nvert re-export

convert.rs re-export of decode::parse_finish_reason had no users workspace-wide
(providers, ffi, bindings, tools).
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.

1 participant