Skip to content

[WIP] Fix review feedback on PR #1637 regarding video generation#1638

Closed
Copilot wants to merge 1 commit into
devfrom
copilot/fix-review-feedback-pr-1637
Closed

[WIP] Fix review feedback on PR #1637 regarding video generation#1638
Copilot wants to merge 1 commit into
devfrom
copilot/fix-review-feedback-pr-1637

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 18, 2026

  • Create SDD issue docs for PR feat: implement OpenAI-compatible video generation features and settings #1637 review fixes (spec.md, plan.md, tasks.md)
  • Verify each requested fix against gen-video code and skip only no-longer-valid items
  • Fix persistence and provider model endpoint alignment for video generation
  • Fix AI SDK video polling/download security and cancellation behavior in both relevant paths
  • Fix renderer video block state handling, i18n fallback removal, and video block detection robustness
  • Localize remaining English strings in specified locale settings.json files
  • Speed up polling tests using fake timers/stubbed delay and add/adjust focused tests for new behavior
  • Run targeted validation for touched areas, collect a UI screenshot, then run required formatting/i18n/lint commands when possible
  • Run parallel validation (Code Review + CodeQL), address findings, and summarize fixed vs skipped review items
Original prompt

Please create a pull request for repository ThinkInAIXYZ/deepchat based on branch dev that fixes the review feedback on PR #1637 (head branch gen-video), focused on the actionable and still-valid issues below.

Context:

  • Existing PR: feat: implement OpenAI-compatible video generation features and settings #1637
  • Title: feat: implement OpenAI-compatible video generation features and settings
  • The fixes should be implemented against the feature work introduced there.
  • Keep changes minimal and targeted. Verify each finding against current code and skip any no-longer-valid item with a brief explanation in the PR body.

Fixes to implement:

  1. Persist videoGeneration session settings
  • File: src/main/presenter/agentRuntimePresenter/index.ts
  • Update both buildPersistedGenerationSettingsPatch and buildPersistedGenerationSettingsReplacement so videoGeneration is persisted the same way imageGeneration is.
  1. Prevent credential leakage when downloading generated video
  • File: src/main/presenter/llmProviderPresenter/aiSdk/runtime.ts
  • fetchBinary() currently forwards Authorization and provider defaultHeaders to arbitrary task.url values.
  • Only forward provider auth/default headers when the download URL is on the provider origin/host.
  • For third-party/presigned URLs, send a minimal request without provider credentials.
  • Preserve existing controller.signal and dispatcher behavior.
  • Apply to both relevant download locations.
  1. Make video polling cancellable by caller
  • File: src/main/presenter/llmProviderPresenter/aiSdk/runtime.ts
  • The local timeout AbortController must be composed with an optional caller AbortSignal so user cancellation stops background polling and download work.
  • Pass the composed signal through all relevant fetch/stream calls.
  • Ensure listeners/timeouts are cleaned up properly.
  • Apply to both analogous polling blocks.
  1. Keep persisted apiEndpoint aligned for video-generation models
  • File: src/main/presenter/llmProviderPresenter/providers/aiSdkProvider.ts
  • In fetchNewApiModels(), ensure models classified as video generation also get apiEndpoint set to video-generation (or corresponding enum), matching type and endpointType.
  1. Reset videoError when the video source changes or successfully loads
  • File: src/renderer/src/components/message/MessageBlockVideo.vue
  • Prevent stale error state after a previously failed source.
  • Reset on src change and/or successful load events while preserving existing error behavior.
  1. Show terminal state instead of endless loader
  • File: src/renderer/src/components/message/MessageBlockVideo.vue
  • When no media is present, do not always show a spinner.
  • Distinguish in-progress states from terminal failure/cancelled states and render an appropriate terminal failure view.
  1. Remove hardcoded English fallback text from video message renderer path
  • File: src/renderer/src/components/message/MessageBlockVideo.vue
  • Keep rendering key-based and i18n-driven.
  • Do not use hardcoded English user-facing fallback strings like Video or Request failed.
  1. Improve isVideoBlock detection
  • File: src/renderer/src/components/message/MessageItemAssistant.vue
  • Support legacy payloads in block.content in addition to block.image_data.
  • Preserve MIME/data URL checks.
  • When checking URLs, inspect pathname/filename and use strict extension matching rather than broad includes() matching.
  1. Localize newly added video-generation strings
  • Files:
    • src/renderer/src/i18n/da-DK/settings.json
    • src/renderer/src/i18n/fa-IR/settings.json
    • src/renderer/src/i18n/fr-FR/settings.json
    • src/renderer/src/i18n/he-IL/settings.json
    • src/renderer/src/i18n/ja-JP/settings.json
    • src/renderer/src/i18n/ko-KR/settings.json
    • src/renderer/src/i18n/pt-BR/settings.json
    • src/renderer/src/i18n/ru-RU/settings.json
  • Replace remaining English values in the new video-generation settings block with proper locale translations, preserving keys and formatting.
  1. Speed up polling tests
  • File: test/main/presenter/llmProviderPresenter/aiSdkRuntime.test.ts
  • Replace real waiting on VIDEO_GENERATION_POLL_INTERVAL_MS with fake timers or a stubbed delay so tests remain deterministic and fast.
  • Restore/cleanup timers appropriately.

Optional / lower priority (implement if still valid and low-risk):
11. Tighten schema validation for video parameter strings in src/shared/contracts/common.ts
12. Remove duplicated hardcoded doubao-seedance- prefix check in src/shared/videoGenerationSettings.ts

Validation expectations:

  • Run or update tests relevant to the touched areas.
  • Keep the PR scoped to fixing the review feedback.
  • Mention which review comments were fixed and which, if any, were skipped because they were no longer valid.

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: 根据 反馈的 comments 梳理出优化点

Assistant: 可以,这次 comments 可以整理成 4 类优化点:必须先修、建议尽快修、体验/一致性优化、可选重构

一、必须先修(建议优先处理)

这些是会导致功能错误、数据不一...

This pull request was created from Copilot chat.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants