feat: add Cloudflare #37
Conversation
please test this, neko told me that i need to push here
Summary of ChangesHello @RumiDaNeko, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new backend component to support Text-to-Speech (TTS) services leveraging Cloudflare Workers AI. It establishes the necessary Go code to construct requests, communicate with Cloudflare's TTS models (specifically Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new backend for Cloudflare AI Text-to-Speech. The overall structure is a good start, but there are several critical and high-severity issues that need to be addressed before this can be merged. My review focuses on making the implementation robust, configurable, and integrated correctly with the existing application structure. Key issues include a potential panic from unsafe option unwrapping, use of http.DefaultClient without timeouts, improper error handling for response streaming, and problems with configuration handling (accountID) and integration. I've also included a suggestion to make the model selection dynamic as intended.
|
|
||
| // HandleSpeechCloudflare processes a TTS request using the Cloudflare Workers AI API. | ||
| // It requires the Cloudflare Account ID to be passed in. | ||
| func HandleSpeechCloudflare(c echo.Context, accountID string, options mo.Option[types.SpeechRequestOptions]) mo.Result[any] { |
There was a problem hiding this comment.
The function signature for HandleSpeechCloudflare takes an accountID parameter, which is inconsistent with other backend handlers and makes it difficult to integrate. Configuration like account IDs should typically be managed via environment variables for better portability and consistency. I'd recommend changing the function signature to remove the accountID parameter.
You can then add the following code at the beginning of the function (you'll need to import os):
accountID := os.Getenv("CLOUDFLARE_ACCOUNT_ID")
if accountID == "" {
return mo.Err[any](apierrors.NewErrInternal().WithDetail("CLOUDFLARE_ACCOUNT_ID environment variable not set").WithCaller())
}Also, remember to register this new backend in pkg/backend/backend.go so it can be used.
| func HandleSpeechCloudflare(c echo.Context, accountID string, options mo.Option[types.SpeechRequestOptions]) mo.Result[any] { | |
| func HandleSpeechCloudflare(c echo.Context, options mo.Option[types.SpeechRequestOptions]) mo.Result[any] { |
| // --- 6. Stream Successful Audio Response --- | ||
| // On success, Cloudflare returns the raw audio stream directly in the body. | ||
| // The Content-Type (e.g., "audio/mpeg") is correctly proxied. | ||
| return mo.Ok[any](c.Stream(http.StatusOK, res.Header.Get("Content-Type"), res.Body)) |
There was a problem hiding this comment.
The return value of c.Stream is an error. By wrapping it in mo.Ok[any](...), you are treating a potential error as a success case, which will hide streaming failures. The error should be checked and handled. Since headers will have already been sent at this point, you cannot send a JSON error response, but logging the error is important for debugging.
if err := c.Stream(http.StatusOK, res.Header.Get("Content-Type"), res.Body); err != nil {
slog.ErrorContext(c.Request().Context(), "failed to stream response", "err", err)
}
return mo.Ok[any](nil)| // - @cf/deepgram/aura-1 (supports 'text' param) | ||
| // | ||
| // We'll use @cf/myshell-ai/melotts as an example. | ||
| const modelName = "@cf/myshell-ai/melotts" |
There was a problem hiding this comment.
The modelName is hardcoded, which limits the flexibility of this backend. As the comment on the preceding lines suggests, this should be dynamic. You can use the opt.Model field from the request options and fall back to a default model if it's not provided.
| const modelName = "@cf/myshell-ai/melotts" | |
| modelName := opt.Model | |
| if modelName == "" { | |
| modelName = "@cf/myshell-ai/melotts" | |
| } |
|
appearently, gemini verifying it own code as wrong dang |
2 participants
please test this, neko told me that i need to push here