fix: improve GPT compatibility and request logs

[RobinLin666]

Summary

• map GPT-5 and o-series chat token limits to max_completion_tokens while preserving legacy model behavior
• keep Anthropic-to-Responses tools non-strict, enforce the Responses output-token floor, and preserve cached-token usage
• replace noisy default HTTP logs with compact colored model, effort, token, cache, context, and total-time summaries

Validation

• bun run verify
• bun run build
• real Claude Code + gpt-5.6-sol MCP tool call completed successfully with full arguments

[voidsteed]

Thanks for this — the max_completion_tokens routing and the strict: false tool fix are real bugs caught correctly, and I want both.

I reviewed the branch locally (typecheck + bun test both pass, 153/153) and also had a second model review it independently. We converged on the same set of issues, plus it found a crash I'd missed. Details inline; summary of what blocks a merge:

Blocking

1. padColumn crashes the response body on non-string metadata — client-triggerable, corrupts the response and swallows the log line. This is the most serious one.
2. Requests whose body is never consumed produce zero log lines (HEAD / on this server is a live example). hono/logger logged unconditionally.
3. MIN_RESPONSES_OUTPUT_TOKENS silently raises a client-supplied max_tokens, undetectably.
4. MODEL_COLUMN_WIDTH = 14 renders claude-sonnet-5 and claude-sonnet-4.6 identically. On a proxy whose core job is model-name mapping, that's the one column that can't collide.

Non-blocking, worth a look: 2 MB response-body buffering to read one usage field; the consola.box/consola.info → debug demotions are a deliberate UX change that deserves a line in the PR description; isRecord returns true for arrays.

The branch also needs a rebase — it's based on v0.10.28 and conflicts with master in responses-bridge.ts, responses/types.ts, server.ts, create-responses.ts, and start.ts.

If you'd rather land the wins fast: split the max_completion_tokens + strict: false + cached-usage fixes into their own PR and I'll merge that quickly, leaving the logging rewrite to iterate separately. Happy either way.

[voidsteed]

Blocking — this crashes the response body for any client that sends a non-string model or effort.

value.slice is called with no type guard. Both ResponsesApiRequest["model"] and reasoning.effort are TypeScript-only types on an unvalidated c.req.json<T>() cast, so nothing stops a client sending:

{"model":"gpt-5.5","input":"hello","reasoning":{"effort":123}}

Reproduced against the real server:

status=400  bodyErr=value.slice is not a function  body=""  logs=0

The exception escapes through the response observer and errors the stream: the client gets an empty body instead of its response, and no log line is written either. A numeric model does the same.

Two things needed:

• Coerce/guard here (String(value), or reject non-strings in setRequestLogMetadata).
• Wrap the finishRequestLog call sites in try/catch. Observability code must never be able to break response delivery — even with the guard, that invariant is worth enforcing structurally.

[voidsteed]

Blocking — requests whose response body is never consumed emit no log line at all.

Every log write happens inside the observed stream's pull/cancel. If nobody reads the body, neither fires. !response.body covers the null case, but not "body exists and is dropped unread."

Concrete on this server: HEAD / returns 200 and logs nothing, because Hono discards the body. hono/logger logged both an incoming and an outgoing line without consuming anything.

Suggest logging from the middleware body after await next() for the non-streaming case, and reserving the stream-observer path for when you actually need end-of-stream usage numbers — with the finished flag still guarding against a double write.

[voidsteed]

Blocking — 14 chars makes the most important column ambiguous.

Measured against the model names this proxy actually maps to:

model	rendered
claude-sonnet-5	claude-sonnet…
claude-sonnet-4.6	claude-sonnet…
claude-opus-4.6	claude-opus-4…
claude-haiku-4.5	claude-haiku-…

Sonnet 5 and Sonnet 4.6 are indistinguishable, and the version suffix is exactly what someone reads this log to confirm — it's the output of translateModelName(), the mapping in CLAUDE.md that this proxy exists to perform.

18 fits every name in that table. Alternatively truncate from the middle to always preserve the suffix.

[voidsteed]

Non-blocking: this buffers up to 2 MB of every non-SSE response body into a string purely to read usage. /v1/models and non-streaming completions all get fully copied.

Since the only consumer is mergeResponseMetrics, consider capping this far lower for non-streaming JSON, or skipping observation entirely on routes that never carry usage.

[voidsteed]

Blocking — this silently raises a limit the client explicitly set, with no way to detect it.

max_tokens: 1 through /v1/messages becomes max_output_tokens: 16 upstream; every value 1–15 is rewritten, streaming and non-streaming alike. The client is never told its cap was overridden.

The modified test makes this the specification (max_tokens: 1 in → max_output_tokens: 16 asserted), which is what I'd push back on — it encodes the surprising behavior rather than the intended one.

This is the same class of problem as #5: the proxy pre-emptively rewriting a client value on a guess about what upstream will accept. If Copilot genuinely rejects max_output_tokens < 16, I'd rather let that rejection through, or clamp and log loudly. Silent widening is the one option that leaves the caller unable to reason about what it asked for.

What was the upstream error that motivated this? If you have the response body I'll take a look — there may be a narrower fix.

[voidsteed]

I've pushed the review fixes directly to this branch (maintainer edits were enabled) rather than leaving you to do the round-trip — your commit is rebased onto master with authorship intact, and my changes are a separate commit on top. Conflicts are resolved and the PR is mergeable again.

Rebase: the only real conflict was in responses-bridge.ts, where master had added max_tokens stop-reason handling in the same lines as your cached-token work. Resolved to keep both.

f372551 — what I changed and why:

1. padColumn crash (the one that worried me most). model and effort are only structurally typed by the c.req.json<T>() cast, so at runtime a client can send anything. {"reasoning":{"effort":123}} threw value.slice is not a function inside the response-body observer, which errored the stream — the client got an empty body instead of its response, and no log line either. Now coerces non-strings, and finishRequestLog is wrapped so logging can never break request handling regardless.

While testing that I found two related injection paths, both now closed by stripping control characters:

• a newline in model let a caller forge an extra log line
• an ESC sequence let a caller repaint the operator's terminal

2. Unread bodies. Non-streaming responses are logged when the handler returns rather than when the body drains. HEAD / is the concrete case — Hono discards the body, so the end-of-stream observer never fired and the request disappeared from the log. SSE still goes through the streaming observer, which is where incremental usage parsing is genuinely needed.

3. Model column 14 → 18. claude-sonnet-5 and claude-sonnet-4.6 both rendered claude-sonnet….

4. Removed MIN_RESPONSES_OUTPUT_TOKENS. This is the one place I overrode you without knowing your reason, so please push back if I've broken something real. My concern: silently raising a client's max_tokens leaves the caller unable to reason about the cap it set, and the modified test made that the spec. If Copilot does reject max_output_tokens < 16, I'd rather that rejection reach the client — but if you hit a concrete upstream error here, share the response body and I'll find a narrower fix.

Added 6 regression tests (tests/request-log.test.ts) covering each of the above; your two original tests are unchanged. bun run verify is green — typecheck, lint, 207 tests.

Not changed, still yours if you want them: the 2 MB non-SSE body buffering (much less pressing now that the buffered path reads an already-complete body), and isRecord returning true for arrays.

The max_completion_tokens routing and strict: false fixes are untouched — they were right as written, and they're the reason this PR is worth landing. Thanks for both.