Skip to content

V3.1.2 beta.rc4#253

Open
leslielazzarino wants to merge 8 commits into
devfrom
v3.1.2-beta.rc4
Open

V3.1.2 beta.rc4#253
leslielazzarino wants to merge 8 commits into
devfrom
v3.1.2-beta.rc4

Conversation

@leslielazzarino

@leslielazzarino leslielazzarino commented Jul 1, 2026

Copy link
Copy Markdown

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:

  • I have tested the changes locally.
  • I have self-reviewed the code changes.
  • I have updated the documentation, if necessary.

Summary by CodeRabbit

  • New Features

    • Added an experimental OpenVINO KV cache precision option for lower memory use on long contexts.
    • Reasoning now shows live elapsed time while streaming.
  • Bug Fixes

    • Improved backend uninstall/reinstall reliability by handling read-only files and permission issues more robustly.
    • Better cleanup of running backend processes during stop and setup flows.
    • Remote images are now downscaled before storage, reducing resource usage.
  • Documentation

    • Updated setup instructions to use a one-time project setup command and hook installation step.

leslielazzarino and others added 8 commits July 1, 2026 19:26
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>
 - OVMS pinned to weekly build 2026.3.0.8022ddae3 (releaseTag 39717a69b → 8022ddae3).
  - Added a sync helper to read a backend's version/releaseTag from the shipped backend-versions.json.
  - Removed OVMS's hardcoded version/releaseTag defaults, sourcing them from the JSON instead, and added a
  guard against an unset version during download.
  - Fixed package author name/email (Intel® Corporation/TODO → Intel Corporation/webadmin@linux.intel.com).
  - Lowered the Phison preset contextSize from 64000 to 8192.

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>
@coderabbitai

coderabbitai Bot commented Jul 1, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This PR adds pre-commit tooling (config, hook installer script, npm scripts), hardens backend service reinstall/uninstall with permission restoration and process-tree termination for ComfyUI, LlamaCPP, and OpenVINO, introduces an OpenVINO INT4 KV cache precision setting with version marker persistence, adds a live chat reasoning streaming indicator, adds remote image downscaling before persistence, and updates a preset context size.

Changes

Developer Tooling and Pre-commit Setup

Layer / File(s) Summary
Pre-commit hook configuration
.pre-commit-config.yaml
Adds Ruff lint/format hooks for Python and local hooks for ESLint/Prettier in WebUI, all restaging changes without blocking commits.
Pre-commit hook installer script
WebUI/build/scripts/ensure-precommit-hook.mjs
New idempotent, best-effort Node script that detects git context, config presence, existing hook, selects pre-commit/uvx pre-commit, and installs the hook.
Setup wiring and docs
WebUI/package.json, readme.md
Adds setup and install-hooks npm scripts and updates readme install instructions accordingly.

Backend Service Reinstall and Process Termination Hardening

Layer / File(s) Summary
Permission restoration helper
WebUI/electron/subprocesses/tools.ts
Adds restoreTreeWritePermissions to recursively restore write permissions before removal (Linux/macOS only).
ComfyUI/LlamaCPP reinstall fixes
WebUI/electron/subprocesses/comfyUIBackendService.ts, WebUI/electron/subprocesses/llamaCppBackendService.ts
Applies the new helper before deletion and on EACCES retries during reinstall/uninstall.
OpenVINO reinstall, extraction, and termination
WebUI/electron/subprocesses/openVINOBackendService.ts
Adds Windows stray-process killing, retrying directory removal, stopAllModelServers(), and terminateProcessTree() used across all server stop paths and uninstall.

OpenVINO KV Cache Precision Setting

Layer / File(s) Summary
Bundled version lookup
WebUI/electron/remoteUpdates.ts, WebUI/external/backend-versions.json
Adds cached getBundledBackendVersionSync and updates the OpenVINO releaseTag.
Version marker and KV cache field
WebUI/electron/subprocesses/openVINOBackendService.ts
Persists installed version via installed-version.json, adds kvCachePrecision, requires version before download, and adds --kv_cache_precision launch arg.
Store toggle and settings sync
WebUI/src/assets/js/store/backendServices.ts, WebUI/src/env.d.ts
Adds persisted openvinoKvCacheU4 toggle, effectiveOvmsKvCachePrecision, and syncs it to the service on change/start.
Settings UI and i18n
WebUI/src/components/BackendOptions.vue, WebUI/src/assets/i18n/en-US.json
Adds an OpenVINO-only checkbox field and localization strings for the experimental setting.

Chat Reasoning Streaming Indicator

Layer / File(s) Summary
Reasoning progress tracking
WebUI/src/assets/js/store/openAiCompatibleChat.ts
Tracks reasoningInProgress/reasoningStartedAt via chunk-sequence detection instead of a time-gap heuristic.
Live reasoning display
WebUI/src/components/ChatReasoningDisplay.vue, WebUI/src/views/Chat.vue
Adds a live elapsed-time timer for the actively streaming reasoning block via new streaming/liveStartedAt props and an isReasoningStreaming helper.

Remote Image Downscaling

Layer / File(s) Summary
Shared downscale pipeline
WebUI/src/lib/utils.ts
Refactors file downscaling and adds downscaleDataUriTo1MP for base64 data URIs.
Home agent usage
WebUI/src/assets/js/store/homeAgent.ts
Downscales remote image data URIs before saving to media input.

Misc

Layer / File(s) Summary
Preset config
modes/base/presets/phison.json
Reduces contextSize from 64000 to 8192.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant BackendOptions as BackendOptions.vue
  participant BackendServicesStore as backendServices.ts store
  participant OpenVINOService as OpenVINOBackendService
  participant OVMS as OVMS process

  User->>BackendOptions: toggle INT4 KV cache checkbox
  BackendOptions->>BackendServicesStore: set openvinoKvCacheU4
  BackendServicesStore->>BackendServicesStore: compute effectiveOvmsKvCachePrecision
  BackendServicesStore->>OpenVINOService: updateServiceSettings(ovmsKvCachePrecision)
  OpenVINOService->>OpenVINOService: stop chat sub-servers if changed
  User->>BackendServicesStore: startService(openvino-backend)
  BackendServicesStore->>OpenVINOService: updateServiceSettings before start
  OpenVINOService->>OVMS: launch with --kv_cache_precision arg
Loading
sequenceDiagram
  participant ChatView as Chat.vue
  participant ChatStore as openAiCompatibleChat.ts
  participant ReasoningDisplay as ChatReasoningDisplay.vue

  ChatStore->>ChatStore: onChunk reasoning-delta -> reasoningInProgress=true
  ChatStore->>ChatStore: set reasoningStartedAt on new block
  ChatView->>ChatView: isReasoningStreaming(message, partIndex)
  ChatView->>ReasoningDisplay: pass streaming, liveStartedAt
  ReasoningDisplay->>ReasoningDisplay: tick interval, update elapsed time
  ChatStore->>ChatStore: onChunk text/tool -> reasoningInProgress=false
  ChatStore->>ChatStore: onFinish -> reasoningInProgress=false
  ChatView->>ReasoningDisplay: streaming=false, show final elapsed time
Loading

Possibly related PRs

  • TNG/AI-Playground#202: Both PRs touch the saveImageToMediaInput/media input flow in homeAgent.ts.
  • TNG/AI-Playground#210: Both PRs rework reasoning-stream/timing metadata and chunk-handling logic in openAiCompatibleChat.ts.
  • TNG/AI-Playground#222: Both PRs modify remoteUpdates.ts backend-versions.json parsing/lookup logic.

Suggested reviewers: mschuettlerTNG

🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is still the template placeholder text and provides no actual details about the changes, issue, or testing. Replace the placeholders with a real description, related issue, changes made, testing done, screenshots if needed, and checklist status.
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title is just a version label and does not describe the actual changes in the pull request. Use a concise, specific summary of the main change, such as the tooling and OpenVINO updates included here.
✅ Passed checks (2 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch v3.1.2-beta.rc4

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

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/homeAgent.ts (1)

1119-1139: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

mediaType/extension can mismatch the actually-persisted (downscaled) image bytes.

const dataUri = data:${img.mime};base64,${img.data_base64}
// Match the regular chat path: cap images at 1MP before persisting so
// oversized remote images aren't passed full-resolution to the model.
const downscaledUri = await downscaleDataUriTo1MP(dataUri) then ext/mediaType on the resulting FileUIPart are still derived from the pre-downscale img.mime (Line 1133, 1136). downscaleDataUriTo1MP re-encodes non-jpeg/webp source images (e.g. gif, bmp, tiff) as PNG, so when downscaling actually triggers on such a source, the persisted file's real bytes/mime no longer match the reported mediaType/filename extension.

Derive the extension/mediaType from the downscaled data URI's own mime header instead of the original img.mime.

🛠️ Proposed fix
           const dataUri = `data:${img.mime};base64,${img.data_base64}`
           // Match the regular chat path: cap images at 1MP before persisting so
           // oversized remote images aren't passed full-resolution to the model.
           const downscaledUri = await downscaleDataUriTo1MP(dataUri)
           const aipgUrl = await saveImageToMediaInput(downscaledUri)
-          const ext = img.mime === 'image/jpeg' ? 'jpg' : img.mime.replace('image/', '')
+          const finalMime = downscaledUri.match(/^data:([^;,]+)[;,]/)?.[1] ?? img.mime
+          const ext = finalMime === 'image/jpeg' ? 'jpg' : finalMime.replace('image/', '')
           parts.push({
             type: 'file',
-            mediaType: img.mime,
+            mediaType: finalMime,
             url: aipgUrl,
             filename: `${sourceLabel}-${Date.now()}-${i}.${ext}`,
           })
🤖 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 1119 - 1139, The
FileUIPart metadata in prepareRemoteFiles is still using img.mime to set
mediaType and the filename extension even after downscaleDataUriTo1MP may
re-encode the image. Update prepareRemoteFiles to derive both mediaType and ext
from the returned downscaledUri’s data URI header before calling
saveImageToMediaInput and pushing the FileUIPart, so the persisted bytes,
reported mime, and extension stay consistent.
🧹 Nitpick comments (3)
.pre-commit-config.yaml (2)

16-16: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Ruff version pinned twice (v0.15.20 and ruff@0.15.20); risk of drift.

The mirror rev and the local hook's uvx ruff@... pin the same version in two independent places. If one is bumped without the other, lint and format will silently run against different Ruff versions, producing inconsistent results.

Also applies to: 30-30

🤖 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 @.pre-commit-config.yaml at line 16, The Ruff version is pinned in two
places, so update the pre-commit hook configuration to use a single source of
truth for the version. In .pre-commit-config.yaml, keep the mirror hook `rev`
and the local `uvx ruff@...` pin aligned by refactoring the Ruff-related hook
entries together so they cannot drift independently. Make sure the configuration
for the Ruff hooks stays consistent across the `rev` and the `ruff@` version
reference.

15-22: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

id: ruff is now the legacy alias; current docs recommend ruff-check.

astral-sh/ruff-pre-commit's .pre-commit-hooks.yaml documents ruff-check as the current linter hook id and ruff as a "legacy alias" retained for compatibility. Functionally equivalent today, but worth switching to ruff-check to stay aligned with the actively-documented interface and avoid relying on a deprecated alias in a brand-new config.

♻️ Suggested update
-      - id: ruff
+      - id: ruff-check
         name: ruff lint (Python)
         args: [--fix]
         files: ^(service|home-agent)/.*\.py$
🤖 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 @.pre-commit-config.yaml around lines 15 - 22, The pre-commit hook in the
ruff configuration still uses the legacy `id: ruff` alias. Update the hook entry
in the config to use the current `ruff-check` hook id instead, keeping the
existing `name`, `args`, and `files` settings unchanged so the linter behavior
stays the same while matching the documented interface.
WebUI/src/assets/js/store/backendServices.ts (1)

419-435: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Route the sync failure through the errors store instead of console.warn.

A failed OVMS settings sync here is silently swallowed — the KV-cache toggle simply won't take effect and nothing surfaces it beyond a console line. Per coding guidelines, errors should flow through useErrors().report() (e.g. with surface: 'silent' to preserve today's no-toast behavior) rather than a bare console call, even though this mirrors the pre-existing llamaCppBuildVariant watch's console.warn.

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/store/backendServices.ts` around lines 419 - 435, The
OpenVINO KV cache sync watch is swallowing update failures with a bare console
warning, so route the catch path through the errors store instead. In the watch
on openvinoKvCacheU4, use useErrors().report() for the updateServiceSettings
failure, preserving silent behavior with an override like surface: 'silent' so
no toast appears. Keep the existing sync logic intact and replace the
console.warn fallback with the store-based error reporting.

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 @.pre-commit-config.yaml:
- Around line 10-13: The comment in the pre-commit config still implies
formatter hooks complete the commit in one pass, but the actual flow requires
rerunning the commit after `ruff-format` or `prettier` rewrites and restages
files. Update the comment near the formatter hook configuration to describe the
re-run workflow accurately, using the formatter hook section and the git
add/restage behavior as the reference point.

In `@WebUI/electron/subprocesses/openVINOBackendService.ts`:
- Around line 1594-1598: The Windows cleanup in killStrayOvmsProcessesWindows is
too broad because it uses taskkill on every ovms.exe process, which can
terminate unrelated OVMS servers. Update this logic in openVINOBackendService to
first discover running ovms.exe PIDs and inspect their ExecutablePath, then only
kill processes whose path is under this.ovmsDir. Keep the existing logging in
killStrayOvmsProcessesWindows but make the termination target-owned instances
only.
- Around line 2166-2168: The shutdown path in openVINOBackendService’s process
cleanup returns too early after proc.kill('SIGTERM') and waitForExit, which can
skip the Windows tree-kill cleanup. Update the same kill/exit flow so that on
Windows it still runs the taskkill /T /F branch before any early return, and
keep the logic centered around proc.kill and waitForExit to ensure child OVMS
worker processes are always terminated.

In `@WebUI/src/assets/js/store/openAiCompatibleChat.ts`:
- Around line 96-109: The reasoning streaming state is shared across all chats
because `reasoningInProgress` and `reasoningStartedAt` live in the store setup
scope and are written by the shared `customFetch` closure. Scope this state per
conversation key instead, similar to `turnErrors`, by storing a keyed reasoning
state map and reading/writing it from `customFetch` using
`requestConversationKey` or `activityScope.conversationKey`. Update the
UI-facing accessors so the active chat reads only its own conversation’s
reasoning state, preventing concurrent turns from overwriting each other.

---

Outside diff comments:
In `@WebUI/src/assets/js/store/homeAgent.ts`:
- Around line 1119-1139: The FileUIPart metadata in prepareRemoteFiles is still
using img.mime to set mediaType and the filename extension even after
downscaleDataUriTo1MP may re-encode the image. Update prepareRemoteFiles to
derive both mediaType and ext from the returned downscaledUri’s data URI header
before calling saveImageToMediaInput and pushing the FileUIPart, so the
persisted bytes, reported mime, and extension stay consistent.

---

Nitpick comments:
In @.pre-commit-config.yaml:
- Line 16: The Ruff version is pinned in two places, so update the pre-commit
hook configuration to use a single source of truth for the version. In
.pre-commit-config.yaml, keep the mirror hook `rev` and the local `uvx ruff@...`
pin aligned by refactoring the Ruff-related hook entries together so they cannot
drift independently. Make sure the configuration for the Ruff hooks stays
consistent across the `rev` and the `ruff@` version reference.
- Around line 15-22: The pre-commit hook in the ruff configuration still uses
the legacy `id: ruff` alias. Update the hook entry in the config to use the
current `ruff-check` hook id instead, keeping the existing `name`, `args`, and
`files` settings unchanged so the linter behavior stays the same while matching
the documented interface.

In `@WebUI/src/assets/js/store/backendServices.ts`:
- Around line 419-435: The OpenVINO KV cache sync watch is swallowing update
failures with a bare console warning, so route the catch path through the errors
store instead. In the watch on openvinoKvCacheU4, use useErrors().report() for
the updateServiceSettings failure, preserving silent behavior with an override
like surface: 'silent' so no toast appears. Keep the existing sync logic intact
and replace the console.warn fallback with the store-based error reporting.
🪄 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: 7d21ad8d-482d-42fe-acf9-f1a9b6a0a0cb

📥 Commits

Reviewing files that changed from the base of the PR and between a614f36 and 771c8b3.

📒 Files selected for processing (20)
  • .pre-commit-config.yaml
  • WebUI/build/scripts/ensure-precommit-hook.mjs
  • WebUI/electron/remoteUpdates.ts
  • WebUI/electron/subprocesses/comfyUIBackendService.ts
  • WebUI/electron/subprocesses/llamaCppBackendService.ts
  • WebUI/electron/subprocesses/openVINOBackendService.ts
  • WebUI/electron/subprocesses/tools.ts
  • WebUI/external/backend-versions.json
  • WebUI/package.json
  • WebUI/src/assets/i18n/en-US.json
  • WebUI/src/assets/js/store/backendServices.ts
  • WebUI/src/assets/js/store/homeAgent.ts
  • WebUI/src/assets/js/store/openAiCompatibleChat.ts
  • WebUI/src/components/BackendOptions.vue
  • WebUI/src/components/ChatReasoningDisplay.vue
  • WebUI/src/env.d.ts
  • WebUI/src/lib/utils.ts
  • WebUI/src/views/Chat.vue
  • modes/base/presets/phison.json
  • readme.md

Comment thread .pre-commit-config.yaml
Comment on lines +10 to +13
# Formatters (ruff-format, prettier) reformat in place and then `git add` what they
# changed, so they never abort the commit just because formatting was applied — the
# reformatted files are restaged and included in the same commit. The linters still
# fail on genuine, unfixable problems.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Sanity-check pre-commit's file-modification detection with the local ruff-format hook.
# (Run manually / locally; not runnable in this sandbox since pre-commit/uvx/ruff binaries
# and a git identity are not guaranteed to be available here.)
echo 'x=1' > /tmp/badly_formatted.py
git -C . add -f /tmp/badly_formatted.py 2>/dev/null || true
pre-commit run ruff-format --files /tmp/badly_formatted.py
echo "Exit code: $?"

Repository: TNG/AI-Playground

Length of output: 214


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the relevant config section and nearby hook definitions.
git ls-files .pre-commit-config.yaml
wc -l .pre-commit-config.yaml
sed -n '1,120p' .pre-commit-config.yaml

Repository: TNG/AI-Playground

Length of output: 2216


🌐 Web query:

pre-commit modifying files hook failed restage git add official docs formatting hook abort commit

💡 Result:

When a pre-commit hook modifies a file, the pre-commit framework intentionally fails the commit [1][2]. This design is deliberate to ensure that no changes are committed to your repository without being explicitly reviewed and staged by you [1][3]. If a hook modifies a file, it will cause the commit to abort [1][4]. To resolve this, you can follow these steps: 1. Inspect the changes made by the hook to ensure they are correct [1]. 2. Add the modified files to the staging area using git add or git add -u [1][3][4]. 3. Re-run your commit command [1][4]. Since the files are now in the index, the commit should proceed successfully [4][5]. The framework does not automatically stage these changes for you because it considers modifying the staging area an unsafe operation that could lead to unintended consequences, such as overwriting unstaged changes or committing broken code if a hook behaves unexpectedly [1][3][2]. If you find this workflow creates too much friction, you have a few options: - Manually stage the changes and re-run the commit [1][3]. - If you are confident in the hook, you can bypass the pre-commit checks using the --no-verify flag (e.g., git commit --no-verify -m "message"), though this is generally discouraged as it skips all configured hooks [4]. - If you prefer a different approach, you may consider other tools that integrate with git-commit differently, or accept that manual re-staging is the standard, safe behavior for the pre-commit framework [3][2].

Citations:


Formatter hooks still abort the first commit after rewriting files
Pre-commit still fails when ruff-format or prettier changes tracked files; git add only restages the output so the next git commit succeeds. Update the comment to reflect that this is a re-run workflow, not a one-shot commit.

🤖 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 @.pre-commit-config.yaml around lines 10 - 13, The comment in the pre-commit
config still implies formatter hooks complete the commit in one pass, but the
actual flow requires rerunning the commit after `ruff-format` or `prettier`
rewrites and restages files. Update the comment near the formatter hook
configuration to describe the re-run workflow accurately, using the formatter
hook section and the git add/restage behavior as the reference point.

Comment on lines +1594 to +1598
private async killStrayOvmsProcessesWindows(): Promise<void> {
if (process.platform !== 'win32') return
try {
await execAsync('taskkill /F /IM ovms.exe /T')
this.appLogger.info('Killed stray ovms.exe processes before extraction', this.name)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Avoid killing unrelated OVMS instances.

taskkill /IM ovms.exe terminates every ovms.exe on the machine, including servers not owned by this app. Filter by ExecutablePath under this.ovmsDir before killing PIDs.

🧰 Tools
🪛 ast-grep (0.44.0)

[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { ChildProcess, spawn } from 'node:child_process'
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(detect-child-process-typescript)

🤖 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/electron/subprocesses/openVINOBackendService.ts` around lines 1594 -
1598, The Windows cleanup in killStrayOvmsProcessesWindows is too broad because
it uses taskkill on every ovms.exe process, which can terminate unrelated OVMS
servers. Update this logic in openVINOBackendService to first discover running
ovms.exe PIDs and inspect their ExecutablePath, then only kill processes whose
path is under this.ovmsDir. Keep the existing logging in
killStrayOvmsProcessesWindows but make the termination target-owned instances
only.

Comment on lines +2166 to +2168
proc.kill('SIGTERM')
let exited = await waitForExit(2000)
if (exited) return

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify the Windows path cannot return before the taskkill /T /F branch.
ast-grep run --lang typescript --pattern $'proc.kill("SIGTERM")
let exited = await waitForExit(2000)
if (exited) return' WebUI/electron/subprocesses/openVINOBackendService.ts

Repository: TNG/AI-Playground

Length of output: 523


🏁 Script executed:

#!/bin/bash
set -euo pipefail

file='WebUI/electron/subprocesses/openVINOBackendService.ts'

# Show the relevant section with line numbers.
sed -n '2135,2195p' "$file" | cat -n

# Find the enclosing method and any other process-kill branches in this file.
rg -n "waitForExit|taskkill|SIGTERM|SIGKILL|process.platform|proc.kill" "$file"

Repository: TNG/AI-Playground

Length of output: 5075


Run taskkill /T /F before the early return on Windows.
proc.kill('SIGTERM') can let the parent exit before the Windows tree-kill branch runs, leaving OVMS worker processes alive and reinstall cleanup flaky.

🧰 Tools
🪛 ast-grep (0.44.0)

[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { ChildProcess, spawn } from 'node:child_process'
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(detect-child-process-typescript)

🤖 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/electron/subprocesses/openVINOBackendService.ts` around lines 2166 -
2168, The shutdown path in openVINOBackendService’s process cleanup returns too
early after proc.kill('SIGTERM') and waitForExit, which can skip the Windows
tree-kill cleanup. Update the same kill/exit flow so that on Windows it still
runs the taskkill /T /F branch before any early return, and keep the logic
centered around proc.kill and waitForExit to ensure child OVMS worker processes
are always terminated.

Comment on lines +96 to +109
// True while the model is actively emitting reasoning (i.e. the last content
// chunk was a reasoning delta and no text/tool chunk has followed). Driven
// straight off the chunk stream so the UI never has to infer "is reasoning
// still going?" from part positions or the per-delta `reasoningFinished`
// timestamp (which is bumped to "now" on every delta and so always looks
// recent). Cleared when the turn ends (see the `processing` safety-net watch).
const reasoningInProgress = ref(false)
// Wall-clock start of the reasoning block currently in progress. The part's
// own `reasoningStarted` metadata isn't attached to the live (still-
// streaming) UI part, so the chat view needs this to drive an increasing
// "Reasoned for X.Xs" timer; once the block finishes the view falls back to
// the part metadata. Updated to the block start whenever a new block begins.
const reasoningStartedAt = ref(0)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

reasoningInProgress/reasoningStartedAt are store-global, but streaming is per-conversation.

Both refs are declared once in the store's setup scope and captured by the single customFetch closure that every Chat instance shares (getOrCreateChat wires the same customFetch for every conversationKey). Nothing scopes these to requestConversationKey/conversations.activeKey.

This store supports genuinely concurrent turns: Home Agent channels (telegram/slack/discord/mock) each drain and call chatStore.generate() with their own conversationKey, and the desktop user can be actively chatting in a different conversation at the same time (see homeAgent.ts drainCommonQueue/handleChatMessage/handleAgenticMessage). Two concurrent onChunk invocations (one per turn) both read/write the same reasoningInProgress/reasoningStartedAt refs, so:

  • A side-channel turn's reasoning can flip reasoningInProgress to true while the desktop's active conversation is idle, making its last (already-finished) reasoning part re-render as "streaming" with a live timer sourced from the unrelated side-channel's reasoningStartedAt.
  • Conversely, the side-channel's onFinish/non-reasoning chunk can flip reasoningInProgress to false mid-stream for the desktop's genuinely-active reasoning block.

Since turnErrors in this same file already solves an analogous per-conversation problem with a Map<string, AppError>, the same pattern (e.g. a reactive<Record<string, { inProgress: boolean; startedAt: number }>> keyed by conversation) would fix this without changing the streaming/timing algorithm itself.

♻️ Suggested direction (sketch, not a drop-in diff)
-    const reasoningInProgress = ref(false)
-    const reasoningStartedAt = ref(0)
+    // Keyed by conversationKey so concurrent turns (desktop + Home Agent
+    // side-channels) don't clobber each other's reasoning indicator.
+    const reasoningStateByKey = reactive<Record<string, { inProgress: boolean; startedAt: number }>>({})
+    function reasoningStateFor(key: string) {
+      return reasoningStateByKey[key] ?? { inProgress: false, startedAt: 0 }
+    }

Then thread activityScope.conversationKey (already computed in customFetch) through the onChunk/onFinish writes instead of the bare refs, and expose reasoningStateFor/a computed keyed on conversations.activeKey for the UI to consume (Chat.vue would look up by the active key instead of reading a bare reasoningInProgress).

🤖 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/openAiCompatibleChat.ts` around lines 96 - 109, The
reasoning streaming state is shared across all chats because
`reasoningInProgress` and `reasoningStartedAt` live in the store setup scope and
are written by the shared `customFetch` closure. Scope this state per
conversation key instead, similar to `turnErrors`, by storing a keyed reasoning
state map and reading/writing it from `customFetch` using
`requestConversationKey` or `activityScope.conversationKey`. Update the
UI-facing accessors so the active chat reads only its own conversation’s
reasoning state, preventing concurrent turns from overwriting each other.

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