3.1.2 beta test5#246
Open
leslielazzarino wants to merge 81 commits into
Open
Conversation
Signed-off-by: Leslie Lazzarino <leslie.lazzarino@tngtech.com>
Signed-off-by: Leslie Lazzarino <leslie.lazzarino@tngtech.com>
Signed-off-by: Leslie Lazzarino <leslie.lazzarino@tngtech.com>
Signed-off-by: Leslie Lazzarino <leslie.lazzarino@tngtech.com>
Signed-off-by: Leslie Lazzarino <leslie.lazzarino@tngtech.com>
Signed-off-by: Leslie Lazzarino <leslie.lazzarino@tngtech.com>
Signed-off-by: Leslie Lazzarino <leslie.lazzarino@tngtech.com>
Signed-off-by: Leslie Lazzarino <leslie.lazzarino@tngtech.com>
Signed-off-by: Leslie Lazzarino <leslie.lazzarino@tngtech.com>
Consolidate elevated Phison SSD-offload service commands into a single batch run to reduce UAC prompts. Add a Phison chat preset gated on the aiDAPTIV+ build that filters to large MoE models, introduce the largeMoe model flag, and support downloading/detecting sharded GGUF models.
Match split GGUF shard naming against the file basename so all shards are enumerated (previously only the first shard was counted, understating the total size and downloading an incomplete model). Also show "Chat Model" for ggufLLM/openvinoLLM downloads instead of "Undefined" in the download dialog.
Increase the Phison preset context size to 64000 and turn on RAG plus tools-enabled-by-default.
Signed-off-by: Leslie Lazzarino <leslie.lazzarino@tngtech.com>
Signed-off-by: Leslie Lazzarino <leslie.lazzarino@tngtech.com>
* linux poc gap fixes Signed-off-by: guptagunjan <gunjan.gupta@intel.com> * Address review on linux-poc gap fixes Signed-off-by: guptagunjan <gunjan.gupta@intel.com> --------- Signed-off-by: guptagunjan <gunjan.gupta@intel.com>
Signed-off-by: Leslie Lazzarino <leslie.lazzarino@tngtech.com>
Signed-off-by: Leslie Lazzarino <leslie.lazzarino@tngtech.com>
# Conflicts: # WebUI/build/scripts/fetch-external-resources.mts # WebUI/external/backend-versions.json
Signed-off-by: Leslie Lazzarino <leslie.lazzarino@tngtech.com>
Signed-off-by: Leslie Lazzarino <leslie.lazzarino@tngtech.com>
leslielazzarino
force-pushed
the
3.1.2-beta-test5
branch
from
980ff7d to
1fe5c49
Compare
|
Warning Review limit reached
More reviews will be available in 51 minutes and 58 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR updates resource fetching, ComfyUI git handling, process shutdown, hashing, backend setup wiring, UI text rendering, and async error handling across the app. ChangesCross-cutting stability and security changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Signed-off-by: Leslie Lazzarino <leslie.lazzarino@tngtech.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
WebUI/src/views/PromptArea.vue (1)
583-593: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winReport recorder failures through
useErrors, nottoast.error().This changed handler still emits a direct toast on failure, which bypasses the shared error pipeline.
As per coding guidelines, "All errors must flow through the
errorsstore (useErrors). Report errors viaerrors.report(err, overrides?)— never usetoast.error()directly or rely on bareconsole.error()."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@WebUI/src/views/PromptArea.vue` around lines 583 - 593, The recording failure path in handleRecordingClick still reports errors directly with toast.error, which bypasses the shared error pipeline. Update the failure handling in PromptArea.vue to use the useErrors store instead, wiring the recorder error through errors.report(err, overrides?) so the audioRecorder.startRecording flow and its error state are surfaced consistently through the centralized errors handling.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@home-agent/llm_proxy.py`:
- Around line 28-35: The helper _safe_response_content_type is still returning
the original raw_content_type after normalizing and whitelisting, which allows
parameters like charset to slip through. Update the allowed branch in
_safe_response_content_type so it returns the normalized content_type value
instead of raw_content_type, keeping the contract consistent with the helper
name and expected sanitization behavior.
In `@WebUI/electron/subprocesses/langchain.ts`:
- Around line 19-20: The local OpenAI API key fallback in the langchain
subprocess setup is still accepting empty strings, which can flow into
OpenAIEmbeddings as an invalid key. Update the LOCAL_COMPAT_OPENAI_API_KEY
initialization to treat empty or whitespace-only env values as missing and fall
back to a generated value instead, using the same logic where the constant is
defined so the OpenAIEmbeddings path always receives a valid non-empty key.
In `@WebUI/src/assets/js/store/homeAgent.ts`:
- Around line 267-273: The new rejection handlers in homeAgent.ts are bypassing
the app’s error pipeline by calling console.error directly; route these failures
through the useErrors store instead. Update the catch paths around
editKeyboardMessage and the other listed promise handlers to call
errors.report(e, { surface: 'silent' }) (or the appropriate overrides for
background flows), using the existing errors instance in homeAgent so all
failures go through the standard reporting path.
In `@WebUI/src/assets/js/store/setupWizard.ts`:
- Around line 588-590: The retry failure handler in initialize() is still
logging directly to console.error instead of using the shared errors store.
Update the catch path around initialize() so it reports the caught error through
errors.report(...) from useErrors, keeping the setupWizard retry failure
consistent with the existing error handling flow and avoiding any bare
console.error calls.
In `@WebUI/src/assets/js/tools/visualizeObjectDetections.ts`:
- Around line 254-256: The validation in visualizeObjectDetections() is logging
the same failure twice: remove the direct console.error in the empty-detections
guard and let the existing throw be handled by the downstream catch/reporting
flow. Keep the error path centralized through the normal errors store reporting
used in this module, so the failure is surfaced once via useErrors rather than a
bare console message plus catch logging.
In `@WebUI/src/components/BackendOptions.vue`:
- Around line 120-123: The setup/reinstall actions are not surfacing failures
through the shared error store, so user-facing errors can be lost behind
fire-and-forget calls or console logging. Update the handlers around
setupWizard.openHomeAgentSetup and the reinstall/setup functions in
BackendOptions.vue to await the async work inside try/catch, create or normalize
a typed AppError, and report it via errors.report(...) from useErrors. Remove
any bare console.error/toast-style handling in these paths and ensure every
failure flows through the errors store with appropriate overrides.
In `@WebUI/src/components/DemoSamplePrompts.vue`:
- Around line 81-83: The rejection from populateImageEditHistory() is only being
logged to the console, so it is not surfaced through the app’s centralized error
handling. Update DemoSamplePrompts.vue to route this catch path through
useErrors by obtaining the errors store and calling errors.report(e, overrides?)
instead of console.error, while preserving the existing fallback behavior around
the sample preset application. Use the existing populateImageEditHistory and
sample.presetName flow as the place to wire in the error reporting.
In `@WebUI/src/components/HomeAgentToggle.vue`:
- Around line 95-97: The HomeAgentToggle setup launch currently swallows
`openHomeAgentSetup()` failures by logging to `console.error`; route this
rejection through the app’s normal error pipeline instead. Update the
`setupWizard.openHomeAgentSetup()` call in `HomeAgentToggle.vue` to use
`useErrors` and call `errors.report(err, overrides?)` in the catch path,
preserving the existing setup-opening context so the failure is surfaced
consistently.
In `@WebUI/src/components/InstallationManagement.vue`:
- Around line 976-980: The llama standard version row is still using the
llama-specific color helper, which always returns amber and does not reflect
upgrade state. Update the rendering in InstallationManagement.vue so the llama
standard row uses the same upgrade/downgrade logic as
getVersionChangeClass(serviceName), or make
getLlamaStandardVersionChangeClass(component) delegate to that behavior, and
ensure upgrades render with text-green-500 instead of always text-amber-500.
In `@WebUI/src/components/MarkdownRenderer.vue`:
- Around line 45-47: The markdown fallback in MarkdownRenderer.vue still logs
the caught exception with console.error instead of routing it through the
required useErrors flow. Update the catch block in the MarkdownRenderer render
logic to obtain the errors store via useErrors and report the caught error with
errors.report(error, overrides?) before keeping the sanitizeMarkdown fallback,
so this failure path is handled consistently with the rest of the app.
In `@WebUI/src/components/ui/progress/Progress.vue`:
- Around line 13-15: The Progress component is still forwarding the raw
modelValue through delegatedProps, which can leave ProgressRoot state and ARIA
attributes inconsistent with the clamped display value. Update Progress.vue by
removing modelValue from the reactiveOmit call, keep progressValue as the
clamped computed source, and bind ProgressRoot to progressValue directly so the
root always receives the sanitized value.
In `@WebUI/src/views/PromptArea.vue`:
- Line 176: The mic control in PromptArea currently becomes unavailable whenever
speech-to-text is disabled, which prevents stopping an already running
recording. Update the disabled logic and click handling around the mic button in
PromptArea so the stop action remains reachable while
`audioRecorder.isRecording` is true, even if `speechToText.enabled` has been
turned off mid-session; preserve the existing transcribing guard, but allow the
control path to call `audioRecorder.stopRecording()` from the same button when a
recording is active.
---
Outside diff comments:
In `@WebUI/src/views/PromptArea.vue`:
- Around line 583-593: The recording failure path in handleRecordingClick still
reports errors directly with toast.error, which bypasses the shared error
pipeline. Update the failure handling in PromptArea.vue to use the useErrors
store instead, wiring the recorder error through errors.report(err, overrides?)
so the audioRecorder.startRecording flow and its error state are surfaced
consistently through the centralized errors handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 79d1b7da-2f44-493d-82c9-f170c4ac5184
⛔ Files ignored due to path filters (1)
WebUI/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (30)
.gitignore.python-versionWebUI/build/build-config.jsonWebUI/build/scripts/fetch-external-resources.mtsWebUI/electron/subprocesses/comfyUIBackendService.tsWebUI/electron/subprocesses/comfyUiRevision.tsWebUI/electron/subprocesses/langchain.tsWebUI/electron/subprocesses/llamaCppBackendService.tsWebUI/electron/subprocesses/mcpManager.tsWebUI/electron/subprocesses/service.tsWebUI/external/backend-versions.jsonWebUI/package.jsonWebUI/src/App.vueWebUI/src/assets/js/store/backendServices.tsWebUI/src/assets/js/store/dialogs.tsWebUI/src/assets/js/store/homeAgent.tsWebUI/src/assets/js/store/setupWizard.tsWebUI/src/assets/js/tools/visualizeObjectDetections.tsWebUI/src/components/AddLLMDialog.vueWebUI/src/components/BackendOptions.vueWebUI/src/components/DemoSamplePrompts.vueWebUI/src/components/HomeAgentToggle.vueWebUI/src/components/InstallationManagement.vueWebUI/src/components/LoadingBar.vueWebUI/src/components/MarkdownRenderer.vueWebUI/src/components/PhisonAidaptivOptions.vueWebUI/src/components/ProgressBar.vueWebUI/src/components/ui/progress/Progress.vueWebUI/src/views/PromptArea.vuehome-agent/llm_proxy.py
Comment on lines
+267
to
+273
| pending.adapter | ||
| .editKeyboardMessage( | ||
| pending.ref, | ||
| pending.adapter.formatMarkdown(`${pending.summaryMarkdown}\n\n${outcome}`), | ||
| pending.meta, | ||
| ) | ||
| .catch((e: unknown) => console.error('homeAgent: confirmation settle edit failed:', e)) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Route new catch-path failures through useErrors, not console.error.
These newly added rejection handlers bypass the app’s required error pipeline; please report via errors.report(...) (likely surface: 'silent' for these background flows).
As per coding guidelines, "All errors must flow through the errors store (useErrors). Report errors via errors.report(err, overrides?) — never use ... bare console.error()."
Also applies to: 643-655, 676-697, 1498-1498, 2176-2178, 2187-2189
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@WebUI/src/assets/js/store/homeAgent.ts` around lines 267 - 273, The new
rejection handlers in homeAgent.ts are bypassing the app’s error pipeline by
calling console.error directly; route these failures through the useErrors store
instead. Update the catch paths around editKeyboardMessage and the other listed
promise handlers to call errors.report(e, { surface: 'silent' }) (or the
appropriate overrides for background flows), using the existing errors instance
in homeAgent so all failures go through the standard reporting path.
Source: Coding guidelines
Comment on lines
+254
to
256
| if (detections.length === 0) { | ||
| console.error('[visualizeObjectDetections] No detections provided') | ||
| throw new Error('At least one detection is required') |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Avoid logging this validation failure twice.
This new console.error fires immediately before the throw, and the catch block below logs the same failure again. Drop the direct console logging here and let the normal reporting path handle it once. As per coding guidelines, "All errors must flow through the errors store (useErrors). Report errors via errors.report(err, overrides?) — never use toast.error() directly or rely on bare console.error()".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@WebUI/src/assets/js/tools/visualizeObjectDetections.ts` around lines 254 -
256, The validation in visualizeObjectDetections() is logging the same failure
twice: remove the direct console.error in the empty-detections guard and let the
existing throw be handled by the downstream catch/reporting flow. Keep the error
path centralized through the normal errors store reporting used in this module,
so the failure is surfaced once via useErrors rather than a bare console message
plus catch logging.
Source: Coding guidelines
Signed-off-by: Leslie Lazzarino <leslie.lazzarino@tngtech.com>
Signed-off-by: Leslie Lazzarino <leslie.lazzarino@tngtech.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
WebUI/src/assets/js/store/textToSpeech.ts (1)
104-110: 🎯 Functional Correctness | 🟠 MajorAvoid defaulting generic fallback voices to
af_heart
voiceis already omitted when empty, so keep the fallback voice blank unless the configured model is Kokoro-style. Otherwise/audio/speechrequests can fail on providers that don’t acceptaf_heart.Suggested fix
if (hasFallback()) { + const fallbackModel = fallback.value.model.trim() || 'tts-1' + const configuredVoice = fallback.value.voice.trim() + const resolvedVoice = + configuredVoice || (/kokoro/i.test(fallbackModel) ? 'af_heart' : '') return { baseURL: fallback.value.baseUrl.trim(), - model: fallback.value.model.trim() || 'tts-1', - voice: fallback.value.voice.trim() || 'af_heart', + model: fallbackModel, + voice: resolvedVoice, apiKey: fallback.value.apiKey, } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@WebUI/src/assets/js/store/textToSpeech.ts` around lines 104 - 110, The fallback mapping in textToSpeech.ts is forcing a generic default voice of af_heart in the hasFallback() branch, which should not happen for non-Kokoro providers. Update the logic in the fallback return object so voice stays blank when fallback.value.voice is empty, and only supply af_heart when the configured model is Kokoro-style or otherwise explicitly requires it. Keep the existing trimming behavior in the textToSpeech/store fallback handling and preserve the current omission behavior for empty voice values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@WebUI/src/assets/js/store/textToSpeech.ts`:
- Around line 104-110: The fallback mapping in textToSpeech.ts is forcing a
generic default voice of af_heart in the hasFallback() branch, which should not
happen for non-Kokoro providers. Update the logic in the fallback return object
so voice stays blank when fallback.value.voice is empty, and only supply
af_heart when the configured model is Kokoro-style or otherwise explicitly
requires it. Keep the existing trimming behavior in the textToSpeech/store
fallback handling and preserve the current omission behavior for empty voice
values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: aa4ed19b-707a-439b-a509-4a106b83db06
📒 Files selected for processing (3)
WebUI/electron/subprocesses/openVINOBackendService.tsWebUI/external/backend-versions.jsonWebUI/src/assets/js/store/textToSpeech.ts
Signed-off-by: Leslie Lazzarino <leslie.lazzarino@tngtech.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@WebUI/src/assets/js/store/comfyUiPresets.ts`:
- Around line 1126-1131: The image re-encode flow in the preset upload helper
can fall back to returning the original data URI from the canvas conversion
path, but the upload filename is always forced to .png, which can mismatch the
actual payload. Update the logic in the canvas/data-URI conversion helper and
the upload naming path so the generated filename extension matches the real
encoded format returned by the helper, or preserve the source extension on
fallback when re-encoding does not produce PNG bytes. Use the relevant code in
comfyUiPresets.ts around the image conversion and upload generation to keep the
extension and content consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9f7da07b-ec44-492e-a3a7-c3392c867465
⛔ Files ignored due to path filters (1)
comfyui-deps/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
WebUI/src/assets/js/store/comfyUiPresets.tscomfyui-deps/pyproject.toml
Comment on lines
+1126
to
+1131
| if (img.naturalWidth === 0 || img.naturalHeight === 0) return dataUri | ||
| const canvas = document.createElement('canvas') | ||
| canvas.width = img.naturalWidth | ||
| canvas.height = img.naturalHeight | ||
| const ctx = canvas.getContext('2d') | ||
| if (!ctx) return dataUri |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Keep upload extension consistent with the actual re-encode result.
On Line 1126 and Line 1131, the helper can return the original URI, but Line 1235 always forces .png. That can upload non-PNG bytes under a PNG filename on fallback paths.
Suggested fix
- // Always PNG now that the data URI is canvas-re-encoded above.
- const uploadImageName = `${uploadImageHash}.png`
+ const uploadImageExtension =
+ imageDataUri.match(/^data:image\/(png|jpeg|webp);base64,/)?.[1] ?? 'png'
+ const uploadImageName = `${uploadImageHash}.${uploadImageExtension}`Also applies to: 1234-1235
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@WebUI/src/assets/js/store/comfyUiPresets.ts` around lines 1126 - 1131, The
image re-encode flow in the preset upload helper can fall back to returning the
original data URI from the canvas conversion path, but the upload filename is
always forced to .png, which can mismatch the actual payload. Update the logic
in the canvas/data-URI conversion helper and the upload naming path so the
generated filename extension matches the real encoded format returned by the
helper, or preserve the source extension on fallback when re-encoding does not
produce PNG bytes. Use the relevant code in comfyUiPresets.ts around the image
conversion and upload generation to keep the extension and content consistent.
Signed-off-by: Leslie Lazzarino <leslie.lazzarino@tngtech.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description:
Please provide a brief description of the changes made in this pull request.
Related Issue:
If this pull request is related to any existing issue, please mention the issue number here.
Changes Made:
Please list down the specific changes made in this pull request.
Testing Done:
Please describe the testing that has been done to ensure the changes made in this pull request are functioning as expected.
Screenshots:
If applicable, please provide screenshots or GIFs or videos to visually demonstrate the changes made.
Checklist:
Summary by CodeRabbit