Skip to content

fix(chat): translate legacy function history - #5844

Merged
lidge-jun merged 2 commits into
devfrom
fix/chat-legacy-functions
Sep 25, 2026
Merged

lidge-jun merged 2 commits into
devfrom
fix/chat-legacy-functions

Conversation

@Ingwannu

Copy link
Copy Markdown
Owner

Summary

  • translate legacy Chat functions declarations into Responses function tools
  • translate assistant function_call entries into function-call items with bounded request-local IDs
  • pair textual role: function results to pending same-name calls instead of dropping them
  • translate the legacy top-level function_call selector into tool_choice
  • reject malformed or orphaned legacy results explicitly while retaining the existing image-result refusal
  • document the compatibility decision in ADR-0111

Verification

  • focused legacy function regressions: 3 passed, 0 failed
  • complete Chat media/translation file: 8 passed, 0 failed, 44 assertions
  • structure SSOT gate: passed
  • privacy scan: passed
  • checks ran one at a time with CPUQuota at 75% or lower, MemoryMax at 1536 MiB or lower, no swap, low I/O weight, and bounded task counts
  • full local suite and build were intentionally not run under the conservative host ceiling; exact-head CI remains required

Compatibility

  • modern tools, tool_calls, and role: tool behavior is unchanged
  • legacy image-bearing function results remain an explicit 400 because Responses has no safe legacy pairing for that media shape
  • synthetic call IDs are bounded, deterministic within one request, and never persisted as provider identity
  • no Go counterpart was identified for this TypeScript Chat translation path; reviewer confirmation is requested before merge

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

@Ingwannu
Ingwannu requested a review from lidge-jun as a code owner September 25, 2026 10:41
@Ingwannu

Copy link
Copy Markdown
Owner Author

@lidge-jun @Wibias Please review the legacy Chat function pairing and explicit orphan refusal. The regression covers declarations, assistant call, textual result, and final assistant history. @codex review

@coderabbitai

coderabbitai Bot commented Sep 25, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

Next included review available in 9 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: bb6c8b8e-6016-4e36-8e80-ff38e53e989e

📥 Commits

Reviewing files that changed from the base of the PR and between 76db92a and b5426cc.

📒 Files selected for processing (4)
  • src/chat/inbound.ts
  • structure/decisions/ADR-0111-legacy-chat-function-history.md
  • structure/providers/chat-compat.md
  • tests/responses/chat-media-translation.test.ts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-25T10:44:51.995996Z b5426cc Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions github-actions Bot added the bug Something isn't working label Sep 25, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b5426cc2ff

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/chat/inbound.ts Outdated
Comment thread structure/providers/chat-compat.md Outdated
@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 64 / 80

이 PR은 옛날 Chat Completions 함수 호출을, 지금 서버가 쓰는 Responses 형식으로 옮겨 준다. 베이스는 dev다. 지금 dev 끝은 76db92a(feat: add optional JEV Auto routing, #5473)이고, 이 헤드는 그 커밋보다 2개 앞에 있다. types.ts / config.ts 나누기로 무효가 된 변경은 아니다. 같은 주제를 또 고친 열린 PR은 이 번호뿐이다.

옛날 클라이언트는 도구 목록을 functions에 적는다. 도우미 메시지에는 function_call 하나가 있고, 결과는 role: function으로 돌아온다. 지금까지 번역기는 새 tools, tool_calls, role: tool만 살리고 이 세 칸은 조용히 버렸다. 이제는 선언을 함수 도구로, 호출을 function_call 항목으로, 글자 결과를 function_call_output으로 붙인다. 옛 호출에는 아이디가 없어서, 그 요청 안에서만 쓰는 call_legacy_0001 같은 번호를 붙인다. 결과는 아직 답이 안 온 같은 이름에 연결한다. 이름이 없거나 대기 중인 호출이 없으면 400이다. 그림이 든 옛 결과는 예전처럼 400이고, 번역하지 않는다. 메시지에 function_call: null이 있으면 호출이 없는 보통 글로 둔다. 이 null 처리는 헤드 9c5d964에서 들어왔다.

결정 기록은 ADR-0111이고, structure/providers/chat-compat.md에도 적혀 있다. 테스트 세 개가 선언·호출·결과, null 호출, 주인 없는 결과를 본다.

CI는 빨갛다. test 2/4는 tests/responses/protocol-direct-encoders-chat.test.ts 375줄에서 스트림 칸을 JSON으로 읽다 실패했다. 이 PR은 그 파일을 고치지 않는다. gates의 gui/tests/apikeys-mutation-timeout.test.tsx 223줄은 true를 기대했는데 false였다. GUI 파일도 이번 diff에 없다. ci 잡은 그 두 실패를 모아서 실패한 것이다.

라인 - legacyFunctionsToResponses (src/chat/inbound.ts). functions가 없으면 통과한다. 값이 null이면 "functions must be an array"로 400이 된다. 새 tools: null은 없는 것으로 본다. 메시지 function_call: null은 방금 고쳤는데, 맨 위 functions: null은 그대로다. 칸을 항상 넣는 클라이언트는 빈 값을 null로 보낸다.

라인 - chatCompletionsToResponsesBody의 선택 칸 (src/chat/inbound.ts). tool_choice가 없을 때만 옛 function_call(auto, none, 함수 이름)을 tool_choice로 옮긴다. tool_choice: null이면 값이 있는 것으로 보고 옛 선택을 버린다. null은 고르지 않은 것과 같다.

라인 - role: function (src/chat/inbound.ts). 그림이 있으면 400이다. 영상이나 파일은 글자만 뽑아서 결과에 넣고, 그 미디어는 조용히 빠진다. 예전에는 결과 메시지 전체를 버렸다. 지금은 글은 살고 영상·파일만 잃는다.

라인 - structure/providers/chat-compat.md. 새 문단이 추론 표시(ADR-0068) 설명 한가운데, "Chat streamed tool-call identity" 제목 바로 앞에 붙어 있다. 함수 기록을 찾는 사람은 추론 절 안에서 만난다.

메인테이너의 판단이 필요한 지점

호출과 결과 사이에 사용자 말이나 다음 도우미 메시지가 오면 대기 목록을 비운다. 그 뒤에 온 role: function은 400이다. 예전에는 그 결과를 조용히 버렸다. 옛 기록 한가운데 말이 끼면 이제 요청 전체가 거절된다. 이 엄격함을 유지할지는 제품 선택이다.

tools와 functions를 한 목록에 이어 붙인다. 같은 이름이 양쪽에 있으면 도구가 두 개 생긴다. 둘 다 보내는 요청을 거절할지, 이름을 하나로 합칠지 정해야 한다.

저장소에서 function_call이 들어간 Go 파일은 검색되지 않았다. 작성자가 물은 "이 번역의 Go 짝은 없다"는 말과 맞다.

합성 아이디는 call_legacy_0001처럼 접두사가 고정이다. 같은 요청의 tool_calls가 그 문자열을 이미 쓰면 아이디가 겹친다.

너의 추천

방향은 맞다. 머지 전에 functions: null과 tool_choice: null을 메시지 function_call: null과 같이 "없음"으로 보고, 그 두 경우를 테스트에 넣으면 된다. 설명 문단은 추론 절 밖으로 제목을 달아 옮기는 편이 낫다. 영상·파일을 그림처럼 400으로 막을지는 메인테이너가 정하면 된다. 스트림 테스트와 GUI 키 테스트는 이번 파일과 멀어 보이니, 한 번 다시 돌려 이 PR 탓이 아닌지 확인한 뒤에 머지하는 쪽을 권한다.

이 댓글은 grok-bot이 작성했습니다

lidge-jun added a commit that referenced this pull request Sep 25, 2026
…g combo failover

#5843 and #5844 each pin their own half; this case drives one Chat request
carrying legacy functions, a function_call/function pair and
reasoning_effort through a combo whose first target has an empty effort
ladder, and checks the fallback target receives both the translated
call/output pair and effort high. It fails on dev and passes on the union.

Co-authored-by: Ingwannu <Ingwannu@users.noreply.github.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@lidge-jun
lidge-jun merged commit 7e70729 into dev Sep 25, 2026
50 of 55 checks passed
@lidge-jun
lidge-jun deleted the fix/chat-legacy-functions branch September 25, 2026 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants