feat: add tool calling, file upload, reasoning stream - #14
Conversation
There was a problem hiding this comment.
3 issues found across 6 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="src/stream-parser.ts">
<violation number="1" location="src/stream-parser.ts:148">
P2: Tool-call JSON extraction can fail after a stray closing brace because depth goes negative and startIdx is never set for the next JSON object.</violation>
<violation number="2" location="src/stream-parser.ts:291">
P2: Content-Type header removal is case-sensitive, leaving lowercase content-type intact and breaking multipart boundary handling</violation>
</file>
<file name="src/dify-provider.ts">
<violation number="1" location="src/dify-provider.ts:49">
P2: Custom fetch provided via DifyProviderSettings is never forwarded to the model config, so it is ignored during API calls.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
ad9d38f to
7c793e7
Compare
warmwind
left a comment
There was a problem hiding this comment.
PR Review: Real API Testing + Code Review
Real API Test Results (Agent Chat App)
| Feature | Status | Notes |
|---|---|---|
Streaming (streamText) |
✅ WORKS | Correct text output, proper text-start → text-delta → text-end → finish lifecycle |
Conversation continuity (chat-id) |
✅ WORKS | Bot correctly remembers previous messages across turns |
agent_thought handling |
✅ WORKS | Properly ignored to avoid duplicate content from agent_message |
Empty agent_message events |
✅ WORKS | Properly skipped (no empty deltas emitted) |
| Token usage extraction | ✅ WORKS | message_end correctly extracts from metadata.usage |
providerMetadata in stream events |
✅ WORKS | conversationId/messageId/taskId attached to all stream parts |
Build (npm run build) |
✅ PASSES | Clean TypeScript compilation |
| Blocking mode | Test app is Agent Chat type (doesn't support blocking) |
Issues Found
1. 🔴 2 Unit Tests Fail
npm run test → 2 failed | 9 passed (11)
should handle message_end and agent_message events(line 447): ExpectedoutputTokensto be0, got25. The PR changedmessage_endto usemetadata.usage.completion_tokensinstead of returning 0.should handle message_end with usage tokens from data field(line 490): ExpectedoutputTokensto be50, got25. The PR now usesmetadata.usageformessage_endanddata.total_tokensforworkflow_finished— test expectations need updating to match.
2. 🔴 Content-Type Header Casing Bug (stream-parser.ts:292)
const { "content-type": _, ...authHeaders } = headers();The headers() function in dify-provider.ts returns "Content-Type" (Pascal case), but the destructuring uses "content-type" (lowercase). JavaScript object destructuring is case-sensitive, so Content-Type: application/json will leak into the FormData upload request, breaking multipart boundary handling.
Fix: Change to "Content-Type" to match the headers function.
3. 🟡 fetch Not Forwarded (dify-provider.ts:49)
Pre-existing bug (not introduced by this PR): options.fetch from DifyProviderSettings is never passed to the model config, so custom fetch implementations are silently ignored.
4. 🟡 No Tests for New Code
stream-parser.ts adds 360 lines of new logic (ThinkTagParser, parseToolCalls, formatToolsPrompt, uploadFileToDify, extractFileAttachments) with zero test coverage.
5. 🟢 Minor: Hardcoded TOOL_SHORT_DESCRIPTIONS (stream-parser.ts:212-224)
Contains tool names specific to Claude Code (bash, glob, grep, edit, write, etc.) rather than generic Dify tools. Consider removing or making this configurable.
Verdict
Core streaming features work correctly against a real Dify API. Please fix the 2 failing tests and the Content-Type casing bug before merge.
Implement JSON tool call parsing and extraction from streaming responses Add file attachment upload via Dify /files/upload API with base64/Uint8Array support
… tags
ThinkTagParser was misclassifying <think>/</think> when they appeared inside
tool-call JSON (e.g. in arguments like {"prompt": "Use <think>..."}).
Track double-quoted string context and skip tag recognition while inside
strings so tool parameters containing <think> remain plain text.
- Add inString/escape state and reset in reset()
- Add tests for <think> in tool args and </think> inside JSON string
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="src/stream-parser.ts">
<violation number="1" location="src/stream-parser.ts:33">
P2: Quoted content inside `<think>` blocks is always emitted as text because the new inString logic runs before the state machine and hardcodes `type: "text"`, so reasoning content within quotes is misclassified and can leak.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Implement JSON tool call parsing and extraction from streaming responses
Add file attachment upload via Dify /files/upload API with base64/Uint8Array support
Parse ... tags for reasoning content in streams (reasoning-start/end events)
Summary by cubic
Adds tool calling, file uploads, reasoning streaming, and logger support to the Dify chat provider, with improved prompt building, stream parsing, and event handling. This enables richer interactions (tools + files), clearer reasoning/text streams, better debugging, accurate token usage, and robust file/header handling.
New Features
Migration
Written for commit 87b6d1e. Summary will update on new commits.