feat(spec/kling): add Kling spec with typed params and restriction validation tests#5
feat(spec/kling): add Kling spec with typed params and restriction validation tests#5sunqirui1987 wants to merge 18 commits intogoplus:devfrom
Conversation
Summary of ChangesHello, 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 significantly expands the project's AI capabilities by integrating Kling AI for advanced image and video generation. It introduces a modular and extensible framework for interacting with AI services, complete with a concrete implementation for the Qiniu platform. The changes also refine the core asynchronous operation handling within the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
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 significant and well-structured implementation of the kling AI service, including the spec, a Qiniu provider, and a comprehensive set of examples. The code is well-organized, and the inclusion of documentation and tests is excellent.
My review focuses on improving the robustness and clarity of the new example code. I've pointed out several instances of incomplete error handling, which could lead to panics or unexpected behavior. I've also suggested improvements to make the examples more deterministic and easier to verify. Additionally, there are a couple of minor design and correctness suggestions for the example runner and utility code.
Overall, this is a great contribution. Addressing these points will make the examples more reliable and better showcase the new features.
Note: Security Review did not run due to the size of the PR.
| fmt.Println("Error:", err) | ||
| } |
There was a problem hiding this comment.
After printing the error, the function continues execution. This will likely lead to a panic later when svc is used while being nil. The function should return after handling the error. This pattern of incomplete error handling appears in several places in the new example files.
| fmt.Println("Error:", err) | |
| } | |
| fmt.Println("Error:", err) | |
| return | |
| } |
| op, _ := svc.Operation(xai.Model(kling.ModelKlingV1), xai.GenImage) | ||
| op.Params().Set(kling.ParamPrompt, "一只可爱的橘猫坐在窗台上看着夕阳,照片风格,高清画质") | ||
| op.Params().Set(kling.ParamAspectRatio, kling.Aspect16x9) | ||
| results, _ := xai.Call(ctx, svc, op, svc.Options(), nil) |
There was a problem hiding this comment.
The error returned from xai.Call is being ignored. While this is an example, it's a good practice to demonstrate proper error handling. Please check the error and handle it, for example by printing it and returning. This issue is present in multiple places across the new example files.
| results, _ := xai.Call(ctx, svc, op, svc.Options(), nil) | |
| results, err := xai.Call(ctx, svc, op, svc.Options(), nil) | |
| if err != nil { | |
| fmt.Println("Error:", err) | |
| return | |
| } |
examples/kling/main.go
Outdated
| } | ||
|
|
||
| func runSubprogram(subdir string, modelArgs []string) { | ||
| cwd, _ := os.Getwd() |
There was a problem hiding this comment.
The error returned from os.Getwd() is ignored. While it's unlikely to fail in most scenarios, it's a good practice to handle this error to make the example more robust.
| cwd, _ := os.Getwd() | |
| cwd, err := os.Getwd() | |
| if err != nil { | |
| fmt.Printf("Error getting current directory: %v\n", err) | |
| return | |
| } |
| var ( | ||
| mu sync.Mutex | ||
| results []string | ||
| ) |
There was a problem hiding this comment.
This package uses global variables mu and results to manage state. This makes the package stateful and can be brittle. Consider refactoring this to use a struct that holds the state (the results and the mutex), which can then be instantiated by the caller. This would make the design cleaner and safer for potential concurrent use.
| FirstFrame: "https://picsum.photos/1280/720", | ||
| EndFrame: "https://picsum.photos/1280/720", | ||
| RunningMan: "https://aitoken-public.qnaigc.com/example/generate-video/running-man.jpg", | ||
| MultiRef1: "https://picsum.photos/1280/720", | ||
| MultiRef2: "https://picsum.photos/1280/720", |
There was a problem hiding this comment.
Using picsum.photos for demo URLs (FirstFrame, EndFrame, MultiRef1, MultiRef2) will result in a different random image on each run. This can make the examples non-deterministic and hard to verify, especially for features like keyframing that rely on specific start and end images. It would be better to use static, permanent URLs for these assets, similar to how RunningMan and MotionImage are handled.
|
Good addition of Kling AI image/video generation support with solid test coverage and clear parameter validation. A few issues worth addressing before merging: the HTTP client logs the bearer token unconditionally by default, several example Run* functions continue execution after a nil-service error (leading to nil-pointer panics), unchecked type assertions in |
![xgopilot[bot]](https://avatars.githubusercontent.com/in/1924301?s=60&v=4){kind=small}
|
|
||
| curlCmd := c.buildCurlCommand(method, url, reqBodyBytes) | ||
| if c.logger != nil { | ||
| c.logger.Printf("[qiniu] curl command:\n%s", curlCmd) |
There was a problem hiding this comment.
Security: Bearer token leaked unconditionally into logs
buildCurlCommand embeds the raw bearer token in the curl string, and this block logs it on every request regardless of the debugLog flag:
if c.logger != nil {
c.logger.Printf("[qiniu] curl command:\n%s", curlCmd)
} else {
fmt.Println(curlCmd)
}The debugLog guard only wraps logDebug calls — it does not gate this block. Since NewClient defaults debugLog: true and logger: log.Default(), every production caller that doesn't explicitly pass WithDebugLog(false) will have its token written to the default Go logger.
The curl-print block should be gated on c.debugLog, and the token in the Authorization header should be redacted (e.g. Bearer ***) in log output.
spec/kling/operation.go
Outdated
| } | ||
|
|
||
| func (p *genImage) Call(ctx context.Context, svc xai.Service, opts xai.OptionBuilder) (xai.OperationResponse, error) { | ||
| s := svc.(*Service) |
There was a problem hiding this comment.
Unchecked type assertion — panic if non-*Service is passed
Both genImage.Call (here) and genVideo.Call use:
s := svc.(*Service)If any xai.Service implementation other than *kling.Service is passed — including a test double, proxy, or wrapper — this panics with no recovery. Use the comma-ok form and return a typed error:
s, ok := svc.(*Service)
if !ok {
return nil, fmt.Errorf("kling: expected *kling.Service, got %T", svc)
}
|
|
||
| // RunKlingV26 runs all kling-v2-6 demos. | ||
| func RunKlingV26() { | ||
| svc, err := shared.NewService() |
There was a problem hiding this comment.
Nil-service panic: error is printed but execution continues
This pattern appears in several Run* functions (RunKlingV26, RunKlingV21, RunKlingV15, RunKlingV2, RunKlingV21 images, etc.):
svc, err := shared.NewService()
if err != nil {
fmt.Println("Error:", err)
// ← missing return
}
// svc is nil here; the next line panics
op, _ := svc.Operation(...)Add return immediately after printing the error. By contrast, RunKlingV25Turbo, RunKlingV3, RunKlingV3Omni, and RunKlingVideoO1 use svc, _ := shared.NewService() — silently discarding the error entirely. Either pattern will panic when QINIU_API_KEY is set but the service fails to initialize.
| token: token, | ||
| maxRetries: 0, | ||
| baseRetryDelay: DefaultBaseRetryDelay, | ||
| debugLog: true, |
There was a problem hiding this comment.
debugLog defaults to true — debug output enabled in production by default
NewClient hardcodes debugLog: true. Any caller that does not explicitly pass WithDebugLog(false) ships with full request/response logging (and token leakage per the other finding). The safe default for a production client is debugLog: false; users should opt-in to debug output explicitly.
| ) | ||
|
|
||
| // Use mock or real backend | ||
| imgExec := qiniu.NewImageGenExecutor(imgBackend) |
There was a problem hiding this comment.
Tutorial references non-existent functions and paths
This Quick Start uses qiniu.NewImageGenExecutor(imgBackend) and qiniu.NewVideoGenExecutor(vidBackend), which do not exist. The actual exported constructors are qiniu.NewImageExecutor(client *Client) and qiniu.NewVideoExecutor(client *Client). The simpler convenience API introduced by this PR is just:
svc := qiniu.NewService("your-api-token")The "Run Examples" section below (around line 283) also references ./spec/kling/provider/qiniu/example/ which does not exist — the actual examples live under examples/kling/. The linked files (text2image.go, service.go, etc.) inside provider/qiniu/example/ are also absent. The Tutorial 4 Provider Integration block shows qiniu.ImageSubmitResult and qiniu.NewImageGenExecutor(backend), neither of which exists anywhere in the codebase. These need to be rewritten to match the actual API.
|
|
||
| ## Prerequisites | ||
|
|
||
| - Go 1.21+ |
There was a problem hiding this comment.
Prerequisites understates the required Go version
The tutorial says "Go 1.21+" but go.mod requires go 1.25.5. Update to match. Also note that go 1.25.5 is not a released Go version as of this writing (Go releases are 1.N or 1.N.P where P ≥ 0; the latest stable is 1.24.x). The go.mod directive may need to be corrected to a real release such as go 1.24.
| outputPath = filepath.Join(dir, "output", "results.txt") | ||
| } | ||
|
|
||
| f, err := os.OpenFile(outputPath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644) |
There was a problem hiding this comment.
Output file created world-readable; TOCTOU between stat and open
Two issues here:
-
Mode
0644makesresults.txtreadable by all users on the system. Since this file accumulates API response URLs (potentially time-limited signed CDN URLs),0600is more appropriate. -
There is a TOCTOU race: the directory is checked with
os.Stat, thenos.OpenFileis called separately. Between these two calls a symlink could redirect the write. Useos.MkdirAllto ensure the directory exists atomically before opening.
examples/kling/main.go
Outdated
| if filepath.Base(cwd) == "kling" { | ||
| runPath = subdir | ||
| } | ||
| cmd := exec.Command("go", append([]string{"run", "./" + runPath}, modelArgs...)...) |
There was a problem hiding this comment.
os.Args forwarded unsanitized to exec.Command
modelArgs comes directly from os.Args[1:] and is appended to the go run invocation. While there is no shell injection (no shell is invoked), flag-style arguments such as -toolexec=malicious or -overlay=attacker.json can influence how the Go toolchain subprocess executes. The model-name guard at lines 53–67 only prevents unknown model names in the dispatcher; it does not prevent extra arguments from reaching exec.Command. Consider rejecting any modelArgs element that starts with -.
| } | ||
|
|
||
| // GetInt returns the int value for the given param name, or 1 if missing/invalid. | ||
| func GetInt(p ParamsReader, name string) int { |
There was a problem hiding this comment.
GetInt returns 1 for missing/zero/negative values — silently coerces invalid input
The documented default of 1 is intentional for ParamN (image count), but this function is used for all integer params. If a caller explicitly passes 0 or a negative value, the result is also 1 with no warning or error — making invalid input indistinguishable from "not set". Consider returning 0 as the neutral default and letting callers apply model-specific defaults, or add a separate GetIntWithDefault that takes an explicit default argument.
|
|
||
| // validateParamsAgainstRestriction checks each set string param against its Restriction. | ||
| func validateParamsAgainstRestriction(schema xai.InputSchema, params *Params) error { | ||
| for name, val := range params.Export() { |
There was a problem hiding this comment.
Export() called on every Call() — unnecessary map copy on the hot path
validateParamsAgainstRestriction calls params.Export() which allocates and copies the entire params map on every API request. The function only reads values — it could iterate via params.Get(name) or an unexported accessor without the defensive copy. This is an avoidable allocation on each call.
| } | ||
|
|
||
| // IsImageModel returns true if the model supports image generation. | ||
| func IsImageModel(m string) bool { |
There was a problem hiding this comment.
IsImageModel exclusion list is fragile — new video models require manual additions
The exclusion uses individual strings.Contains checks for v2-5 through v2-9. A future video model like kling-v2-10 or kling-v2-5-image would either be missed or incorrectly excluded. Consider an allowlist approach (only the known image models return true) rather than a blocklist, or convert the exclusion to a numeric range check.
sunqirui1987
changed the title
sunqirui1987
changed the title
…examples - add `multi_shot`/`shot_type`/`multi_prompt` to kling-v3 schema and params build - pass multi-shot fields through qiniu v3 video request builder - update kling v3 and v3-omni video examples to use `shot_type=customize` - normalize `multi_prompt.index` to start from 1 and be consecutive - add/update tests and docs examples for multi_prompt usage
…OpenAI examples guide - rewrite the whole README for examples/openai - document all demos (text/image/video/multi-video/function-call/thinking) - explain unified block output format for response/candidate/block - clarify stream vs non-stream behavior and function-call special flow - add troubleshooting for auth/network/tool schema (parameters.type) errors - include defaults, base URL override, and verification command
…and examples OpenAI spec: - Split response.go into response_v1.go and response_v3.go by API version - Add response_v1_test.go and response_v3_test.go - Add NewV1WithQiniu() and qiniuV1Provider for Qiniu API image responses - Add WithDebugCurl option to log curl commands - Switch provider/qiniu to use NewV1WithQiniu Gemini spec: - Add backend, provider/qiniu, operation and schema updates - Add gemini.md documentation Examples: - Add examples/gemini (chat, image gen, tool use) - Update examples/openai shared blocks and service
Backend abstraction: - Add Backend interface in backend.go for pluggable transport - Service now uses Backend instead of direct genai.Models/Operations - Add NewWithBackend() and genAIBackend for default genai implementation - Operations delegate to backend; Actions() and model support per-backend Schema & operation: - Export NewInputSchema, NewInputSchemaEx, NewParams for external use - Add int/int8/int16 support in kindOf - Refactor syncResponse to NewSyncResponse; operations carry model param Provider & examples: - Add spec/gemini/provider/qiniu for Qiniu API - Add examples/gemini: chat-text, chat-image, chat-tool, image-generate, image-edit - Add gemini.md documentation
spec/gemini: - Add BackendService interface and Service.Backend() - Move gemini.md content to README.md, keep only links - Simplify provider/qiniu implementation examples/gemini: - NewService returns xai.Service so callers depend on interface - Rename svc to service consistently - Remove direct dependency on gemini package
- Add cmd/restrict_gen for restrict generation - Add spec/gemini/restrict_gen.go, spec/util/util.go - Update spec/gemini/schema, operation; spec/schema
spec/vidu: - Add vidu-q1 and vidu-q2 model support via xai.GenVideo - Backend abstraction for pluggable providers - Qiniu provider with Submit/GetTaskStatus - Param validation, async task handling, unified result encapsulation examples/vidu: - Runnable demos for q1/q2 text-to-video, reference-to-video - q2 image-to-video/pro and start-end-to-video/pro - CallSync + TaskID + GetTask polling example - Mock backend (default) and real Qiniu API mode
Spec & implementation: - spec/gemini/provider/qiniu/veo.md: API reference for Veo models - spec/gemini/provider/qiniu/video.go: video generation backend - spec/gemini/veo_opt.go: optional params (AspectRatio, Resolution, DurationSeconds, PersonGeneration, CompressionQuality) - spec/gemini/veo_validate.go: parameter validation Examples (examples/veo/): - veo_*_generate_preview.go: text-to-video for 2.0/3.0/3.1 - veo_image_to_video.go: image-to-video - veo_first_last_frame.go: first+last frame - veo_video_input.go: video as input (引用视频) - veo_reference_images.go: multi reference images (多参考图) - veo_callback.go: callback mode with PubsubTopic Models: veo-2.0-generate-001/exp/preview, veo-3.0/3.1-generate-preview, veo-3.0/3.1-fast-generate-preview
- Add video operations: text-to-video, image-to-video, remix - Add operation_video.go for video generation and polling - Add provider_video.go and Qiniu service_video integration - Add examples/sora with runnable demos (text-to-video, image-to-video, remix) - Refactor: move qiniu provider to provider/qiniu/, remove provider_qiniu.go - Update spec: message, options, params, schema for video support
- Add video operations: text-to-video, image-to-video, remix - Add operation_video.go for video generation and polling - Add provider_video.go and Qiniu service_video integration - Add examples/sora with runnable demos (text-to-video, image-to-video, remix) - Refactor: move qiniu provider to provider/qiniu/, remove provider_qiniu.go - Update spec: message, options, params, schema for video support
- Add spec/audio: Transcribe (ASR) and Synthesize (TTS) actions - Add Qiniu provider for voice/asr, voice/tts, voice/list APIs - Add OutputText and OutputAudio to spec/schema Generated interface - Add Transcribe and Synthesize actions to spec/operation - Add examples/audio: list-voices, asr, tts demos with mock/real modes - Update examples/README.md with audio quick start and directory structure
- unwrap audio/kling/vidu services in operation calls - add ApiKeySetter and thread-safe SetApiKey to Qiniu providers - expand Gemini image examples/docs and add wrapper tests
Add VideoSchema interface and VideoSchemaProvider to standardize video generation schema across providers (OpenAI Sora, Gemini Veo, Kling, Vidu). Changes: - spec/operation.go: Add VideoGenMode (text_to_video, image_to_video, multi_ref_to_video, start_end_to_video) with implication hierarchy and helper functions (ExpandVideoGenModes, NormalizeVideoGenModes) - Add video_schema.go for openai, gemini, kling, vidu implementing VideoSchema with SupportedModes, Fields, Restrict, FieldModes - Extract video param constants to params.go (openai, gemini) - Refactor openai/operation_video.go to use param constants, move restrictions/fields to video_schema.go - vidu: ParamReferenceImageURLs now accepts String|List; add Prompt required restriction
1 participant
Background
This PR introduces and refines
spec/klingcapabilities, including image/video parameter schemas, Qiniu provider request building, and supporting docs/examples. It also adds restriction validation tests to reduce regression risk.Key Changes
spec/klingimplementation across image, video, internal, and provider modules.ImageInput,VideoRef, andKeepOriginalSound) for better compile-time safety.BaseVideoParams.aspect_ratio,resolution,image_reference, and schema fallback behavior.Testing
go test ./spec/kling/...passedCommits Included
4494336ADd kling3cf6b2ckling ai UPDATEc722586xai kling