Skip to content

Phase 2 Plan 02-04: Engine Compatibility Matrix API + UI - #99

Merged
debpalash merged 3 commits into
mainfrom
phase-2-plan-02-04-engine-compat-matrix
May 20, 2026
Merged

Phase 2 Plan 02-04: Engine Compatibility Matrix API + UI#99
debpalash merged 3 commits into
mainfrom
phase-2-plan-02-04-engine-compat-matrix

Conversation

@debpalash

@debpalash debpalash commented May 20, 2026

Copy link
Copy Markdown
Owner

Summary

Final Phase 2 wave-3 piece (depends on 02-01 SubprocessBackend + 02-03 IndexTTS migration, both merged). Surfaces the architectural shift made by Plans 02-01/02-03 to users via a new Settings → Engines compatibility matrix, plus a GET /engines/{id}/health endpoint that lets the matrix's "Test engine" button spawn-and-ping a SubprocessBackend on demand.

Closes ENGINE-06.

Requirements covered

  • ENGINE-06 — Engine Compatibility Matrix UI (install state, GPU compat, isolation mode, last error)
  • Mitigates T-02-12 (HF token leak via response body) and T-02-13 (loopback enforcement on the new health route) from the plan's threat model

What's in the PR

Backend (commit a5d1572)

  • New gpu_compat: tuple[str, ...] class attribute on TTSBackend, overridden per backend with reasonable defaults (cuda+mps+cpu for OmniVoice / VoxCPM2 / IndexTTS2, cpu-only for KittenTTS, mps+cpu for MLX-Audio, etc.).
  • New _HF_TOKEN_MASK_RE (hf_[A-Za-z0-9]{30,}) applied to reason and last_error fields inside list_backends() before serialization — closes T-02-12 without depending on the logging-level HFTokenRedactor.
  • New GET /engines/{engine_id}/health route — loopback-gated; calls SubprocessBackend.health_check() (spawn-and-ping) for subprocess engines, falls back to is_available() for in-process engines. Returns { id, ok, message, latency_ms }. Never 500s on a sick engine (exceptions land in the response body). Unknown id → 404.
  • _ENGINE_INSTANCES singleton cache so repeated health checks don't leak atexit hooks or spawn extra sidecars.

Frontend (commit ff712d3)

  • New frontend/src/components/EngineCompatibilityMatrix.jsx — semantic <table> with role-attrs. Columns: Engine / Install state / GPU compat / Isolation / Actions. "Test engine" button does not auto-spawn on mount; 5 s cooldown to prevent click-storms. Optional onSelect prop turns the matrix into a picker so Settings doesn't need a parallel table.
  • New frontend/src/components/EngineCompatibilityMatrix.css reusing existing chrome tokens.
  • frontend/src/api/engines.tsgetEngineHealth(id) client wraps the new backend route.
  • frontend/src/api/types.ts — extends EngineBackend with optional isolation_mode, last_error, install_hint, gpu_compat and adds EngineHealthResponse.
  • frontend/src/pages/Settings.jsx — drops the hand-rolled engines table inside EnginesTab and mounts <EngineCompatibilityMatrix family="tts" onSelect={...} /> instead. Cleans up dead imports (no new tab; the Engines tab already existed).

Endpoint shape

GET /engines/{engine_id}/health      (loopback only)
→ 200  { id, ok, message, latency_ms }
→ 404  unknown engine id
→ 403  non-loopback origin

GET /engines  (extended in Plan 02-01, plus `gpu_compat` in this PR)
→ 200  { tts: { active, backends: [
           { id, display_name, available, reason, install_hint,
             last_error, isolation_mode, gpu_compat }, ...
        ]},
         asr: {...}, llm: {...} }

Test plan

  • Backend: uv run pytest tests/backend/api/test_engines_route_shape.py -v11 passed (response shape, isolation_mode, gpu_compat, health subprocess + in-process paths, 404, 403, singleton cache, HF leak proofs on both /engines and /engines/{id}/health)
  • Backend full suite: uv run pytest tests/ -q --ignore=tests/manual402 passed, 10 skipped, 13 xfailed, 1 xpassed (up from 391+ baseline)
  • Frontend: cd frontend && bun run test65 passed (8 new tests in EngineCompatibilityMatrix.test.jsx)
  • Frontend lint: no new errors introduced; Settings.jsx pre-existing lint count went from 5 → 4
  • Frontend typecheck: bun run typecheck:ci clean
  • Manual smoke (reviewer): boot backend + frontend → Settings → Engines → matrix renders with one row per backend; click Test engine on a SubprocessBackend row → latency_ms appears next to button

Phase 2 Success Criterion mapping

  • ✓ Success criterion 5: "one engine in a broken state cannot prevent app boot" — the health route's defensive try/except plus list_backends()'s existing graceful degradation are now both covered by tests
  • ✓ Cross-platform parity: matrix renders identically on macOS/Windows/Linux (no platform-specific render paths); backend route reuses require_loopback which has been validated by Phase 1's loopback tests on all three OSes

Deviations from plan

  • The plan called for frontend/src/api/engines.js; this codebase already has frontend/src/api/engines.ts (TypeScript), so the health function was added there rather than creating a parallel .js file. Matches the existing pattern.
  • The plan called for EngineCompatibilityMatrix.test.jsx to run via bun test; this codebase's bun test runs Bun's built-in runner which lacks jsdom. The frontend test suite uses bun run test (alias for vitest run). SUMMARY documents this for Phase 6 CI.
  • list_backends() in asr_backend.py and llm_backend.py was NOT extended with gpu_compat / isolation_mode / last_error. The plan's must_haves focus on TTS (where the IndexTTS2 subprocess payoff lives); the frontend types those new fields as optional (?: ...) so the matrix still renders for ASR/LLM families on the simpler payload — only the Isolation and GPU compat columns degrade gracefully. A future plan can extend the other families in a one-line refactor.
  • INST-13 (dictation-widget Settings checkbox) is intentionally not in scope per the locked Phase 2 8-ID requirement scope (ENGINE-01..07 + BUG-01).

Files changed

 backend/api/routers/engines.py                              | +105 / -1
 backend/services/tts_backend.py                             |  +71 / -10
 frontend/src/api/engines.ts                                 |  +17 / -1
 frontend/src/api/types.ts                                   |  +20 / -1
 frontend/src/components/EngineCompatibilityMatrix.css       | +155 (new)
 frontend/src/components/EngineCompatibilityMatrix.jsx       | +269 (new)
 frontend/src/pages/Settings.jsx                             |  +25 / -107
 frontend/src/test/EngineCompatibilityMatrix.test.jsx        | +210 (new)
 tests/backend/api/__init__.py                               |   +0 (new)
 tests/backend/api/test_engines_route_shape.py               | +263 (new)
 tests/backend/services/test_tts_backend_registry.py         |   +3 / -1
 .planning/phases/.../02-04-SUMMARY.md                       | +152 (new)

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

  • New Features

    • Added Engine Compatibility Matrix UI in Settings to test engines and view GPU support across backends.
    • Added engine health check endpoint to verify backend availability and measure response latency.
  • Bug Fixes

    • Masked sensitive credentials in engine error messages to prevent accidental exposure.
  • Documentation

    • Added comprehensive Plan 02-04 documentation with implementation details and test guidance.
  • Tests

    • Expanded test coverage for engine health checks and compatibility matrix.

Review Change Stack

debpalash and others added 3 commits May 20, 2026 07:37
ENGINE-06 backend half. Adds the data + spawn-on-demand endpoint the new
Engine Compatibility Matrix UI will consume:

* `gpu_compat: tuple[str, ...]` class attribute on `TTSBackend`, overridden
  per backend with reasonable defaults (cuda+mps+cpu for OmniVoice/VoxCPM2;
  cpu-only for KittenTTS; mps+cpu for MLX-Audio; etc.). `list_backends()`
  serializes it as a list.
* `_HF_TOKEN_MASK_RE` (`hf_[A-Za-z0-9]{30,}`) scrubs the `reason` and
  `last_error` fields before they leave the registry — Phase 1's
  HFTokenRedactor logging filter does not run on FastAPI response bodies,
  so this closes T-02-12.
* `GET /engines/{engine_id}/health` — loopback-gated route that resolves
  the backend across tts/asr/llm registries, then either calls
  `SubprocessBackend.health_check()` (spawn-and-ping) for subprocess
  engines or falls back to `is_available()` for in-process engines.
  Returns `{ id, ok, message, latency_ms }`. Engine instances are cached
  per-class so repeated checks don't leak atexit hooks or spawn extra
  sidecars. The masked-redactor is reapplied on the way out.

Test coverage (tests/backend/api/test_engines_route_shape.py, 11 tests):
  * Response shape includes the new fields for every TTS entry
  * IndexTTS2 isolation_mode == "subprocess", OmniVoice == "in-process"
  * Health route round-trips with mocked SubprocessBackend success
  * Health route falls back to is_available for in-process backends
  * Unknown engine id → 404
  * Non-loopback origin → 403
  * Engine instance cache reuses the singleton across calls
  * HF tokens leaked into is_available() / health_check() are masked
    in both the /engines and /engines/{id}/health response bodies

Existing tts_backend_registry shape test updated to include `gpu_compat`.
Full suite: 402 passed, 0 failures (up from 391+ baseline).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ENGINE-06 frontend half. Mounts a new component on Settings → Engines
that surfaces, end-to-end, the data shape Plan 02-01 + Plan 02-03 added
to the backend registry:

* `frontend/src/components/EngineCompatibilityMatrix.jsx` (270 lines) —
  semantic <table> with role=row/cell so RTL queries work; one row per
  registered backend. Columns:
    - Engine name + install hint + Last error line
    - Install state badge (Available / Unavailable + inline reason)
    - GPU compat chips (CUDA / MPS / ROCm / CPU with colored variants)
    - Isolation mode badge (subprocess for IndexTTS, in-process for the
      rest — makes the Phase 2 architectural shift legible to users)
    - "Test engine" button → `/engines/{id}/health` round-trip; renders
      latency in ms inline next to the button; disabled while inflight;
      5 s cooldown to prevent click-storms.
  Mount does NOT auto-test any engine — per the plan's Open Question #2,
  spawning sidecars is gated on user action.
* `frontend/src/components/EngineCompatibilityMatrix.css` — minimal
  styling that reuses chrome tokens; chip colors per GPU target.
* `frontend/src/api/engines.ts` — `getEngineHealth(id)` client function
  wraps the new backend route through the shared apiJson helper.
* `frontend/src/api/types.ts` — extends EngineBackend with optional
  `isolation_mode`, `last_error`, `install_hint`, `gpu_compat` so the
  TypeScript surface tracks the backend wire shape, and adds
  EngineHealthResponse.
* `frontend/src/pages/Settings.jsx` — replaces the hand-rolled Engines
  table inside EnginesTab with `<EngineCompatibilityMatrix family="tts"
  onSelect={...} />`. selectEngine still wires up the picker; the
  matrix's onSelect prop renders the Use button per row when provided.
  Removes the now-unused FAMILY_META local map.

Test coverage (`frontend/src/test/EngineCompatibilityMatrix.test.jsx`,
8 tests via vitest):
  * Renders one row per backend with documented columns
  * isolation_mode badge: subprocess for IndexTTS2, in-process for
    OmniVoice / KittenTTS
  * GPU compat chips: omnivoice → cuda/mps/cpu; kittentts → cpu only
  * Unavailable rows render the failure reason inline
  * last_error line renders below status when populated; masked HF
    token sentinel survives verbatim
  * Test engine click fires getEngineHealth(id) and renders latency_ms
  * Test button disabled while inflight; second click is a no-op
  * Failure path (ok=false) renders a failure marker

Frontend suite: 65 passed (8 new). Lint: 0 new errors. typecheck:ci: clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Recap of Engine Compatibility Matrix delivery — backend route +
gpu_compat metadata + HF-token redaction, frontend EngineCompatibility-
Matrix component, full test counts, deviations, gpu_compat confidence
matrix, frontend test-runner command notes for Phase 6 CI.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented May 20, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

This PR implements the Phase 02-04 "Engine Compatibility Matrix API + UI" feature, adding GPU compatibility metadata to TTS backends, implementing an engine health-check endpoint with instance caching, creating a new UI component to display and test engines, and integrating it into Settings.

Changes

Engine Compatibility Matrix API + UI

Layer / File(s) Summary
Backend GPU compatibility metadata and HF-token redaction
backend/services/tts_backend.py, tests/backend/services/test_tts_backend_registry.py
Extends TTSBackend protocol with gpu_compat field (defaulting to ("cpu",)), assigns GPU compatibility tuples to nine backends (OmniVoice, VoxCPM2, MOSS-TTS-Nano, KittenTTS, MLX-Audio, CosyVoice, GPT-SoVITS, Sherpa-ONNX), and introduces regex-based HF-token masking that redacts hf_...-shaped secrets from reason and last_error in list_backends() response.
Engine health endpoint with instance caching
backend/api/routers/engines.py
Adds loopback-gated GET /engines/{engine_id}/health that resolves an engine ID across TTS/ASR/LLM registries, checks availability via subprocess health_check() or in-process is_available() using a per-class singleton cache, wraps exceptions into {ok, message} responses, masks HF tokens, and returns latency_ms alongside health status.
Frontend API types and health helper
frontend/src/api/types.ts, frontend/src/api/engines.ts
Extends EngineBackend type with optional install_hint, last_error, isolation_mode, and gpu_compat fields; introduces EngineHealthResponse interface; and provides getEngineHealth(engineId) API helper.
Engine Compatibility Matrix UI component
frontend/src/components/EngineCompatibilityMatrix.jsx, frontend/src/components/EngineCompatibilityMatrix.css
Implements new React component rendering a table of backend engines grouped by family, tracks loading/error states, normalizes engine entries, implements per-engine "Test engine" health checks with 5-second cooldown and in-flight guards, displays GPU chips, isolation mode badges, availability status, and last-error text, and optionally renders "Use" buttons via injected onSelect callback.
Settings page integration
frontend/src/pages/Settings.jsx
Refactors EnginesTab to render EngineCompatibilityMatrix with an onSelect callback that calls selectEngine and shows success/error toasts, removing the prior local engine-loading/switching/table UI and FAMILY_META constant.
Frontend component tests
frontend/src/test/EngineCompatibilityMatrix.test.jsx
Comprehensive Vitest + React Testing Library tests verifying row rendering, isolation mode and GPU compatibility badges, unavailability/last-error messaging, health check button behavior with cooldown/inflight guards, latency display, and failure indicators.
Backend API and registry tests
tests/backend/api/test_engines_route_shape.py
Tests for API contract validation (GET /engines response shape), health endpoint behavior (subprocess/in-process fallback, 404 for unknown IDs, 403 loopback-only enforcement, singleton caching), and HF-token leak prevention in both routes.
Phase 02-04 delivery summary
.planning/phases/02-engine-isolation-subprocessbackend-indextts-wav-export-dubbi/02-04-SUMMARY.md
Documents complete implementation, test coverage results, GPU compatibility defaults, test runner commands, HF-token redaction approach, scope notes, and manual smoke-test procedures.

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant EngineMatrix as Engine Matrix UI
  participant APIList as apiListEngines
  participant Settings as Settings Page
  participant APIHealth as /engines/{id}/health
  participant Backend as Backend Health Check
  
  User->>Settings: Open Settings → Engines tab
  Settings->>EngineMatrix: render with onSelect callback
  EngineMatrix->>APIList: fetch all engines on mount
  APIList-->>EngineMatrix: {tts: [...], asr: [...], llm: [...]}
  EngineMatrix->>EngineMatrix: normalize entries, display rows
  
  User->>EngineMatrix: click "Test engine" button
  EngineMatrix->>EngineMatrix: check cooldown and in-flight guard
  EngineMatrix->>APIHealth: getEngineHealth(engine_id)
  APIHealth->>Backend: resolve engine, call health_check() or is_available()
  Backend-->>APIHealth: {ok, message, latency_ms}
  APIHealth-->>EngineMatrix: {id, ok, message, latency_ms}
  EngineMatrix->>EngineMatrix: cache result, update row with latency/failure
  
  User->>EngineMatrix: click "Use" for available engine
  EngineMatrix->>Settings: onSelect(family, backend_id)
  Settings->>Settings: call selectEngine, show toast
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • debpalash/OmniVoice-Studio#98: Main PR's new /engines/{id}/health endpoint and expanded list_backends() response shape (including isolation_mode and TTS-backend metadata like gpu_compat and HF-token redaction) directly depend on the IndexTTS-2 SubprocessBackend integration and registry wiring added by this PR.

  • debpalash/OmniVoice-Studio#97: The main PR builds directly on this PR's SubprocessBackend/tts_backend.list_backends() contract by adding gpu_compat and HF-token redaction to the same list_backends fields (last_error, isolation_mode), then wiring UI and the new health route to call the subprocess-style health check behavior.

  • debpalash/OmniVoice-Studio#39: Both PRs modify backend/services/tts_backend.py around the TTS backend registry—specifically the CosyVoiceBackend integration—where the main PR adds gpu_compat metadata to existing backends while this PR introduces the CosyVoice 3 backend and registers it.

Poem

🐰 In Silicon Fields, a Matrix Sprouts

Engines lined up in neat, glowing rows,
GPU's colors shimmer and glow,
Secrets masked where Hugging Face goes,
Health checks hop-skip at each deft mouse throw,
A Settings page where compatibility flows! 🎯

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 52.63% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: implementing the Engine Compatibility Matrix API and UI as part of Phase 2 Plan 02-04, which is the core objective of this PR.
Description check ✅ Passed The description comprehensively covers all template sections: Summary, Changes (detailed backend/frontend components and endpoint shapes), Type (✨ New feature), Testing (specific test results provided), and Checklist with relevant items addressed. The description exceeds template expectations with threat model mitigations, success criterion mapping, and deviation notes.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 phase-2-plan-02-04-engine-compat-matrix

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 and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 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 `@backend/api/routers/engines.py`:
- Around line 164-176: The current _get_engine_instance performs an
unsynchronized check-then-set on the global _ENGINE_INSTANCES causing races;
make it thread-safe by adding a global threading.Lock (e.g.
_ENGINE_INSTANCES_LOCK) and wrap the get/create/update sequence in a with
_ENGINE_INSTANCES_LOCK: block so that _ENGINE_INSTANCES.get(cls), inst = cls(),
and _ENGINE_INSTANCES[cls] = inst are executed atomically inside
_get_engine_instance; reference symbols: _get_engine_instance,
_ENGINE_INSTANCES, cls, inst, and the new _ENGINE_INSTANCES_LOCK.

In `@frontend/src/components/EngineCompatibilityMatrix.jsx`:
- Around line 321-329: The engine "Use" button calls onSelect(activeFamily,
b.id) but never refreshes the matrix state, causing the active badge to remain
stale; update the click handler in EngineCompatibilityMatrix.jsx (the Button
within the onSelect block) to await the result of onSelect (or handle its
promise), then call the component's existing refresh/fetch function (e.g.,
refreshMatrix, fetchEngines, or a provided onRefresh/onSwitchComplete prop) or
update local state to re-fetch engines/families so the UI re-renders with the
new active engine; ensure the handler handles success/failure and only refreshes
after a successful switch.
- Around line 133-169: The click guard in testHealth reads stale healthByEngine
from the closure and can dispatch duplicate probes; make the guard atomic by
using the functional state updater on setHealthByEngine inside testHealth to
inspect prev[id] and set inflight/lastClickAt in one synchronous update. Example
approach: declare a local shouldRun = false, call setHealthByEngine(prev => { if
(prev[id]?.inflight || (prev[id]?.lastClickAt && now - prev[id].lastClickAt <
TEST_COOLDOWN_MS)) return prev; shouldRun = true; return { ...prev, [id]: {
inflight: true, lastClickAt: now } }; }); then if (!shouldRun) return; proceed
to call apiGetEngineHealth and update state in the try/catch as before; update
references to healthByEngine checks to use the functional updater pattern in
testHealth.
🪄 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 Plus

Run ID: 5b47652d-1c72-4ce4-ac5b-0cbfa9af3699

📥 Commits

Reviewing files that changed from the base of the PR and between c3695e1 and 78f1219.

📒 Files selected for processing (12)
  • .planning/phases/02-engine-isolation-subprocessbackend-indextts-wav-export-dubbi/02-04-SUMMARY.md
  • backend/api/routers/engines.py
  • backend/services/tts_backend.py
  • frontend/src/api/engines.ts
  • frontend/src/api/types.ts
  • frontend/src/components/EngineCompatibilityMatrix.css
  • frontend/src/components/EngineCompatibilityMatrix.jsx
  • frontend/src/pages/Settings.jsx
  • frontend/src/test/EngineCompatibilityMatrix.test.jsx
  • tests/backend/api/__init__.py
  • tests/backend/api/test_engines_route_shape.py
  • tests/backend/services/test_tts_backend_registry.py

Comment on lines +164 to +176
def _get_engine_instance(cls):
"""Return a cached singleton instance of ``cls``.

SubprocessBackend's ``__init__`` registers an atexit shutdown hook,
so re-instantiating per request would leak handler entries (and on
real engines, additional sidecar processes the first time the lock
is acquired). One instance per process is the right move.
"""
inst = _ENGINE_INSTANCES.get(cls)
if inst is None:
inst = cls()
_ENGINE_INSTANCES[cls] = inst
return inst

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Make _ENGINE_INSTANCES initialization thread-safe.

Line 172 and Line 174 implement an unsynchronized check-then-set. Concurrent health requests can construct the same backend class more than once, undermining the singleton guarantee and potentially duplicating sidecar/atexit side effects.

🔧 Suggested fix
+from threading import Lock
 from time import perf_counter
@@
 _ENGINE_INSTANCES: dict[type, object] = {}
+_ENGINE_INSTANCES_LOCK = Lock()
@@
 def _get_engine_instance(cls):
@@
-    inst = _ENGINE_INSTANCES.get(cls)
-    if inst is None:
-        inst = cls()
-        _ENGINE_INSTANCES[cls] = inst
-    return inst
+    with _ENGINE_INSTANCES_LOCK:
+        inst = _ENGINE_INSTANCES.get(cls)
+        if inst is None:
+            inst = cls()
+            _ENGINE_INSTANCES[cls] = inst
+        return inst
🤖 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 `@backend/api/routers/engines.py` around lines 164 - 176, The current
_get_engine_instance performs an unsynchronized check-then-set on the global
_ENGINE_INSTANCES causing races; make it thread-safe by adding a global
threading.Lock (e.g. _ENGINE_INSTANCES_LOCK) and wrap the get/create/update
sequence in a with _ENGINE_INSTANCES_LOCK: block so that
_ENGINE_INSTANCES.get(cls), inst = cls(), and _ENGINE_INSTANCES[cls] = inst are
executed atomically inside _get_engine_instance; reference symbols:
_get_engine_instance, _ENGINE_INSTANCES, cls, inst, and the new
_ENGINE_INSTANCES_LOCK.

Comment on lines +133 to +169
const testHealth = useCallback(async (id) => {
const now = Date.now();
const cur = healthByEngine[id];
if (cur?.inflight) return;
if (cur?.lastClickAt && now - cur.lastClickAt < TEST_COOLDOWN_MS) {
// Click-storm cooldown — silently ignore.
return;
}
setHealthByEngine((prev) => ({
...prev,
[id]: { inflight: true, lastClickAt: now },
}));
try {
const result = await apiGetEngineHealth(id);
setHealthByEngine((prev) => ({
...prev,
[id]: {
inflight: false,
ok: !!result.ok,
message: result.message || '',
latency_ms: Math.round(result.latency_ms || 0),
lastClickAt: now,
},
}));
} catch (e) {
setHealthByEngine((prev) => ({
...prev,
[id]: {
inflight: false,
ok: false,
message: e?.message || String(e),
latency_ms: 0,
lastClickAt: now,
},
}));
}
}, [apiGetEngineHealth, healthByEngine]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Make health-click guard atomic to prevent duplicate probes.

Line 135–140 reads stale healthByEngine from closure, so rapid double-clicks before re-render can still dispatch multiple apiGetEngineHealth calls.

Suggested fix
+  const healthGuardRef = React.useRef({});
+
   const testHealth = useCallback(async (id) => {
     const now = Date.now();
-    const cur = healthByEngine[id];
+    const cur = healthGuardRef.current[id];
     if (cur?.inflight) return;
     if (cur?.lastClickAt && now - cur.lastClickAt < TEST_COOLDOWN_MS) {
       // Click-storm cooldown — silently ignore.
       return;
     }
+    healthGuardRef.current[id] = { inflight: true, lastClickAt: now };
     setHealthByEngine((prev) => ({
       ...prev,
       [id]: { inflight: true, lastClickAt: now },
     }));
     try {
       const result = await apiGetEngineHealth(id);
+      healthGuardRef.current[id] = { inflight: false, lastClickAt: now };
       setHealthByEngine((prev) => ({
         ...prev,
         [id]: {
@@
     } catch (e) {
+      healthGuardRef.current[id] = { inflight: false, lastClickAt: now };
       setHealthByEngine((prev) => ({
         ...prev,
         [id]: {
@@
-  }, [apiGetEngineHealth, healthByEngine]);
+  }, [apiGetEngineHealth]);
🤖 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 `@frontend/src/components/EngineCompatibilityMatrix.jsx` around lines 133 -
169, The click guard in testHealth reads stale healthByEngine from the closure
and can dispatch duplicate probes; make the guard atomic by using the functional
state updater on setHealthByEngine inside testHealth to inspect prev[id] and set
inflight/lastClickAt in one synchronous update. Example approach: declare a
local shouldRun = false, call setHealthByEngine(prev => { if (prev[id]?.inflight
|| (prev[id]?.lastClickAt && now - prev[id].lastClickAt < TEST_COOLDOWN_MS))
return prev; shouldRun = true; return { ...prev, [id]: { inflight: true,
lastClickAt: now } }; }); then if (!shouldRun) return; proceed to call
apiGetEngineHealth and update state in the try/catch as before; update
references to healthByEngine checks to use the functional updater pattern in
testHealth.

Comment on lines +321 to +329
{onSelect && b.available && !isActive && (
<Button
size="sm"
variant="subtle"
onClick={() => onSelect(activeFamily, b.id)}
aria-label={`Use ${b.display_name}`}
>
Use
</Button>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Refresh matrix after successful engine switch to avoid stale active state.

Line 321–329 triggers onSelect but does not refresh backend data, so the active badge may remain outdated until manual refresh.

Suggested fix
-                  {onSelect && b.available && !isActive && (
+                  {onSelect && b.available && !isActive && (
                     <Button
                       size="sm"
                       variant="subtle"
-                      onClick={() => onSelect(activeFamily, b.id)}
+                      onClick={async () => {
+                        await onSelect(activeFamily, b.id);
+                        await reload();
+                      }}
                       aria-label={`Use ${b.display_name}`}
                     >
                       Use
                     </Button>
                   )}
🤖 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 `@frontend/src/components/EngineCompatibilityMatrix.jsx` around lines 321 -
329, The engine "Use" button calls onSelect(activeFamily, b.id) but never
refreshes the matrix state, causing the active badge to remain stale; update the
click handler in EngineCompatibilityMatrix.jsx (the Button within the onSelect
block) to await the result of onSelect (or handle its promise), then call the
component's existing refresh/fetch function (e.g., refreshMatrix, fetchEngines,
or a provided onRefresh/onSwitchComplete prop) or update local state to re-fetch
engines/families so the UI re-renders with the new active engine; ensure the
handler handles success/failure and only refreshes after a successful switch.

@debpalash
debpalash merged commit 84fffa5 into main May 20, 2026
8 checks passed
@debpalash
debpalash deleted the phase-2-plan-02-04-engine-compat-matrix branch May 20, 2026 02:22
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.

1 participant