Claude/fix tool calling deepseek#1554
Conversation
Co-authored-by: farzanshibu <69754165+farzanshibu@users.noreply.github.com>
…ion types - add ToolCall interface for DeepSeek function/tool payloads - type DeepSeek response and stream chunks to use ToolCall instead of any[] - keep only tools/tool_choice parameters (drop legacy functions/function_call) - add comprehensive chat completion tests: - legacy function fields are not forwarded - strict OpenAI finish_reason mapping for insufficient_system_resource - non-strict finish_reason passthrough - stream finish_reason mapping in strict mode
There was a problem hiding this comment.
Pull request overview
Updates provider transforms and request configs to improve type-safety and add DeepSeek tool-calling support, while also standardizing the realtime event parser import path and hardening logging.
Changes:
- Add
tools/tool_choicesupport and tool-call passthrough to the DeepSeek chat completion provider (plus extensive unit tests). - Adjust finish-reason / stop-reason typing and transforms across providers (notably Bedrock ↔ Anthropic stop reasons).
- Standardize
RealTimeLLMEventParserimport path casing and minor robustness tweaks in winky logging.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/services/winky/index.ts | Hardens handling of missing ai_model when calculating redaction, pricing, and metrics labels. |
| src/providers/utils.ts | Changes Anthropic stop-reason transform return type. |
| src/providers/deepseek/types.ts | Introduces DeepSeek tool-call typing. |
| src/providers/deepseek/chatComplete.ts | Adds tool-call support in request config and response/stream transforms. |
| src/providers/deepseek/tests/chatComplete.test.ts | Adds DeepSeek provider coverage (params, tool calls, finish reasons, streaming). |
| src/providers/bedrock/messages.ts | Adapts Bedrock Messages transforms to new stop-reason typing (currently via casts). |
| src/handlers/websocketUtils.ts | Updates realtime parser import path. |
| src/handlers/realtimeHandlerNode.ts | Updates realtime parser import path. |
| src/handlers/realtimeHandler.ts | Updates realtime parser import path. |
| package-lock.json | Updates lockfile (adds optional snappy native deps and modifies dev/optional flags). |
Comments suppressed due to low confidence (1)
src/providers/utils.ts:99
transformToAnthropicStopReasonnow declares a| nullreturn type, but the implementation never returnsnull(it always falls back toend_turn). This type change forces downstreamas any/as stringassertions and reduces type-safety. Either (a) revert the return type toANTHROPIC_STOP_REASONor (b) actually returnnullin the cases where callers expect it and update call sites accordingly.
export const transformToAnthropicStopReason = (
finishReason?: PROVIDER_FINISH_REASON
): ANTHROPIC_STOP_REASON | null => {
if (!finishReason) return ANTHROPIC_STOP_REASON.end_turn;
const transformedFinishReason = AnthropicFinishReasonMap.get(finishReason);
if (!transformedFinishReason) {
return ANTHROPIC_STOP_REASON.end_turn;
}
return transformedFinishReason;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
|
@farzanshibu - thanks for the PR! Can you address Copilot's comments? |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 9 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
|
@roh26it Please check |
This pull request introduces several important updates across the codebase, primarily focusing on improving type safety, adding support for DeepSeek tool calls, and correcting import paths. The most significant changes include enhancements to the DeepSeek provider to support tool calls, adjustments to type handling in Bedrock and DeepSeek response transforms, and standardization of import paths for the
RealTimeLLMEventParser. Additionally, there are minor improvements to ensure robustness in logging logic. #1555