Skip to content

feat: return reasoning content as reasoning_content field - #3

Open
A8Cl233395 wants to merge 2 commits into
yelixir-dev:mainfrom
A8Cl233395:feat/reasoning-content
Open

A8Cl233395 wants to merge 2 commits into
yelixir-dev:mainfrom
A8Cl233395:feat/reasoning-content

Conversation

@A8Cl233395

Copy link
Copy Markdown

When INCLUDE_REASONING is enabled, emit upstream reasoning deltas as the reasoning_content field instead of merging them into content: streaming chunks use delta.reasoning_content, non-streaming responses use message.reasoning_content, and the provider passthrough keeps reasoning_content (stripping it only when the flag is off). Reasoning content also counts toward the empty visible response guard.

@yelixir-dev

Copy link
Copy Markdown
Owner

Thanks for this — the direction is right and I want to take it. The reasoning split is genuinely useful for clients, and the image handling fixes a real defect (today we inline base64 into a text part, which no vision model can use).

Before merging I need three changes, all driven by one hard constraint in this project: the request body we send upstream must be byte-shaped like the one the CommandCode CLI sends. The bridge should not be distinguishable from the CLI on the wire. I checked the changes against the CLI bundle (command-code@1.53.0, dist/cli.mjs), specifically toWireMessages() — the function that builds params.messages for the upstream request.

1. The image part field is mimeType, not mediaType

The CLI emits:

"image" === t.type && o.push({
  type: "image",
  image: `data:${t.source.media_type};base64,${t.source.data}`,
  mimeType: t.source.media_type
})

So please rename CommandCodeImagePart.mediaType to mimeType, and always set it for data URIs (the CLI never omits it).

2. Only data URIs should become image parts

The CLI builds image parts exclusively from base64 sources, so a bare https://… URL in an image part is a shape the CLI never produces. Please either fetch the URL and inline it as a data URI, or fall back to the existing text representation for remote URLs.

3. Please split the commits by topic

The PR currently mixes two independent changes:

  • image handling (native image parts + tool-result images as a following user message) — a bug fix
  • reasoning_content as a separate field — a behavior change that is breaking for anyone running with INCLUDE_REASONING=1, since reasoning no longer arrives inside content

They need separate commits (and separate release-note lines) so users can see what changed and why.

Confirmed correct, no change needed

  • Assistant reasoning parts match the CLI exactly: "thinking" === t.type && e.push({ type: "reasoning", text: t.thinking }).
  • Emitting tool-result images as a following user message matches the CLI's own message layout (e.length>0 && t.push({role:"tool",content:e}); o.length>0 && t.push({role:"user",content:o})).
  • Stripping reasoning_content in the provider passthrough when the flag is off is a correct tightening.

I ran your branch locally: 15 test files, 232 tests, all passing.

Follow-up (not yours to fix)

The CLI drops image parts entirely when the target model has no image input modality (supportsVisionstripImages). Our model catalog does not carry modality data yet, so the bridge cannot gate this. I will track that separately.

Once 1–3 are addressed I will merge this.

yelixir-dev added a commit that referenced this pull request Sep 15, 2026
Bridge-only bugfix release against an unchanged CommandCode CLI 1.53.0.
Carries the Docker restart fixes from #4 and the removal of top_p and
stop from the upstream generate body. Records the release, the PR #3
review findings, and the PR #1 closure in the process log.
LitangDingzhen added 2 commits September 16, 2026 17:18
Convert OpenAI image_url parts into native {type:"image"} parts for
/alpha/generate so vision models receive real image input instead of a
text part full of base64. Image parts are built only from base64 data
URIs and always carry the CLI's mimeType field; remote URLs and
non-base64 data URIs fall back to the short [image_url: ...] /
[image: ...] text placeholders, so the bridge never emits an image part
shape the CommandCode CLI cannot produce. Tool-result images are
forwarded as a following user message because the upstream ModelMessage
schema rejects image parts inside tool-result output.
BREAKING CHANGE: with INCLUDE_REASONING enabled, upstream reasoning
deltas are no longer appended to content. Streaming chunks carry them in
delta.reasoning_content and non-streaming responses in
message.reasoning_content. When the flag is off, the provider passthrough
strips reasoning_content instead of leaving it in the payload. Reasoning
content still counts toward the empty-visible-response guard, and
assistant messages that carry reasoning_content are forwarded upstream as
leading {type:"reasoning"} parts, matching the CommandCode CLI.
@A8Cl233395
A8Cl233395 force-pushed the feat/reasoning-content branch from 41ac2cc to a22dbc5 Compare September 16, 2026 09:24
@A8Cl233395

Copy link
Copy Markdown
Author

All three points addressed:
(1) renamed to mimeType and always set
(2) image parts are built only from base64 data URIs — remote URLs and non-base64 data URIs fall back to the existing text placeholders
(3) split into two topic commits (d325ce8 image fix, a22dbc5 reasoning behavior change) and rebased onto main@7c27016 so the top_p/stop removal is preserved. INCLUDE_REASONING docs updated to match. npm run verify: 238 passed — the 4 admin-config/install-scripts failures are pre-existing on main on Windows, not from this branch.

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.

2 participants