Skip to content

fix(engines): warn about under-provisioned hardware before the synth, not after - #1288

Merged
debpalash merged 3 commits into
mainfrom
fix/generate-vram-preflight
Jul 29, 2026
Merged

fix(engines): warn about under-provisioned hardware before the synth, not after#1288
debpalash merged 3 commits into
mainfrom
fix/generate-vram-preflight

Conversation

@debpalash

@debpalash debpalash commented Jul 28, 2026

Copy link
Copy Markdown
Owner

Closes #1240, #1246, #1248, #1277, #1283, #1284.

The pattern

Six reports, one story: a 4 GB or 6 GB card running an engine that declares a 6 GB floor, each user waiting out the full 300s compute budget to be told the job "was too heavy".

The routing layer knew the whole time — the timeout text even names the card and the figure ("RTX 2060 has 6.0 GB of VRAM and this engine wants about 6 GB"). The diagnosis was never the problem. The timing was.

Why nobody saw it

The caveat surfaced only on the engine-pick toast. That reaches users who change engines, and nobody whose engine was already selected — the default, or one persisted from a previous session. That is most users.

/generate does return X-OmniVoice-Routing, but a response header arrives when the job ends. Five minutes too late to be a warning, and the frontend never read it anyway.

The fix

Preflight at the chokepoint every synth path shares (api/generate.ts — the same argument as the in-flight count, and the same seven call sites).

  • Never awaited — a warning must not add latency to the request it warns about.
  • Never throws — an unreachable backend costs a warning, not a generate.
  • Once per engine+reason per session — informs instead of nags; keying on the reason means a genuinely different problem still gets through.
  • Advisory, not blocking — matches the routing layer's deliberate contract ([Bug] TTS generate ran for more than 372s of actual compute time and was abandoned — t #1226): the driver can page to system RAM, and short inputs fit where long ones don't.

Extracts routingNotice() as the single frontend mirror of the backend's routing_notice(). Two callers now ask "is this verdict worth interrupting for", and two inline copies would drift — invisibly, until someone on DirectML or an unavailable engine gets a hardware warning for a perfectly normal pick. That exact bug was Greptile's P1 on #1280.

Tests

8 new cases: fires on the already-selected engine, once-only, silent on healthy, silent on benign cpu_only (DirectML rule 5), fires on cpu_fallback, never throws when the backend is down, tolerates no active engine, fetches /engines once rather than per synth.

engines.generateCaveat added in all 21 locales with real prose.

Full frontend suite: 1602 passing.

Added a non-blocking, fire-and-forget “under-provisioned hardware” preflight warning before the /generate chokepoint (including streaming), wired through shared routing/caveat detection (routingNotice()) and guarded by per-session dedup with TTL-cached engine-list fetching and correct invalidation/seeding on engine selection (plus withTtsInflight to keep inflight tracking active for the full streamed response). This ensures users see advisory notices up front (with new localized engines.generateCaveat strings across all locales) instead of encountering synthesis timeouts/500s, while routing streaming through the same generate path and avoiding extra /engines calls. Risk to review: that the warning truly never delays or blocks generation, and that the dedup/cache/inflight lifecycles behave correctly across engine changes, transient engine-list failures, and stream consumption/release.

… not after

Six reports are the same story: #1240, #1246, #1248, #1277, #1283, #1284 —
4 GB and 6 GB cards running an engine that wants 6 GB, each one waiting out
the full 300s compute budget to be told the job "was too heavy". The routing
layer knew the whole time. The error text even names the card and the figure.

The caveat only ever surfaced on the engine-PICK toast, so it reached people
who changed engines and nobody whose engine was already selected — the
default, or one persisted from a previous session. That is most users.
/generate does return X-OmniVoice-Routing, but a response header arrives when
the job ends, five minutes too late to be a warning.

So the check moves to the chokepoint every synth path shares (api/generate.ts,
same argument as the in-flight count). Fire-and-forget: never awaited, so it
cannot add latency to the request it warns about; never throws, so an
unreachable backend costs a warning rather than a generate; once per
engine+reason per session, so it informs instead of nagging. Advisory, not
blocking — the driver can page to system RAM and short inputs fit where long
ones don't.

Extracts routingNotice() as the single frontend mirror of the backend's
routing_notice(). Two callers now need "is this verdict worth interrupting
for", and two inline copies would drift — invisibly, until someone on DirectML
or an unavailable engine gets a hardware warning for a normal pick.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Comment thread frontend/src/api/generate.ts
@greptile-apps

greptile-apps Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Adds generate-time hardware preflight warnings and completes the previous streaming fixes.

  • Routes streaming synthesis through the shared generation chokepoint.
  • Keeps synthesis marked in flight until the streaming response body finishes.
  • Deduplicates and caches routing warnings while invalidating stale engine data.
  • Adds localized warning text and regression coverage.

Important Files Changed

Filename Overview
frontend/src/api/generate.ts Adds the shared preflight invocation and balanced in-flight wrapper; both previous streaming issues are addressed.
frontend/src/utils/streamingTts.js Routes streaming through generateSpeech and holds an outer in-flight claim across body consumption.
frontend/src/utils/generatePreflight.js Implements non-blocking, non-throwing, deduplicated hardware-warning preflight with cache invalidation.
frontend/src/utils/routingNotice.js Centralizes which backend routing verdicts warrant user warnings.
frontend/src/utils/engineSelectToast.js Uses the shared routing predicate and invalidates preflight state after engine selection.
frontend/src/test/streamingPreflight.test.js Covers streaming preflight routing and in-flight lifetime through response-body consumption.
frontend/src/test/generatePreflight.test.js Covers warning eligibility, deduplication, caching, invalidation, and backend failure behavior.

Reviews (3): Last reviewed commit: "fix(engines): hold the in-flight claim f..." | Re-trigger Greptile

@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 2adce1ee-a2f6-4a39-b0ae-1d897d0deabb

📥 Commits

Reviewing files that changed from the base of the PR and between 762bc8a and ce990ce.

📒 Files selected for processing (3)
  • frontend/src/api/generate.ts
  • frontend/src/test/streamingPreflight.test.js
  • frontend/src/utils/streamingTts.js

📝 Walkthrough

Walkthrough

The frontend adds a cached, deduplicated engine provisioning warning before synthesis, shares routing-caveat handling with engine selection toasts, localizes the generation warning, and routes streaming synthesis through the shared generation entry point.

Changes

Engine caveat preflight

Layer / File(s) Summary
Shared routing notice and selection warnings
frontend/src/utils/routingNotice.js, frontend/src/utils/engineSelectToast.js
Routing caveats are normalized through routingNotice and used for CPU-fallback and selection warning toasts.
Generation preflight warning and localization
frontend/src/utils/generatePreflight.js, frontend/src/api/generate.ts, frontend/src/i18n/locales/*
Generation starts a non-blocking, cached preflight warning with session deduplication, failure recovery, and localized generateCaveat messages.
Streaming generation chokepoint and validation
frontend/src/utils/streamingTts.js, frontend/src/test/*Preflight.test.js
Streaming requests use generateSpeech, and tests cover preflight ordering, caching, warning conditions, retries, inflight tracking, and stream payloads.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 7 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description does not follow the required template and omits the Summary, Changes, Type, Testing, and Checklist sections. Rewrite it using the repository template and fill in the required sections with a concise summary, key changes, type, testing, and checklist items.
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.
✅ Passed checks (7 passed)
Check name Status Explanation
Title check ✅ Passed The title uses conventional-commit format with a scope and accurately describes the main change.
Linked Issues check ✅ Passed The changes warn before synthesis for under-provisioned hardware and match the issue's pre-synth OOM prevention goal.
Out of Scope Changes check ✅ Passed The added locales, tests, and streaming inflight changes all support the stated warning and synthesis objectives.
Cross-Platform Default Parity ✅ Passed Shared generateSpeech now preflights all synths with no platform branch or opt-in; streaming still uses the existing cross-platform supportsStreamingPreview gate.
I18n Completeness (21 Locales) ✅ Passed All translation keys used by the touched frontend code exist in all 21 locale files; no new hardcoded user-facing strings were introduced.
Local-First Guarantee ✅ Passed New preflight only queries the app’s local /engines API, is fire-and-forget, and swallows failures; no cloud, auth, or telemetry path was added.
Backward Compatibility ✅ Passed PASS: origin/main...HEAD is frontend-only; no backend/alembic files changed, and the new preflight/inflight code is read-only/non-blocking, so no DB or model reinstall risk.

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration.


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.

streamGenerateSpeech POSTed /generate via apiFetch directly — a second,
parallel door. Everything attached to "the one call every synth path shares"
therefore did not apply to it: the in-flight count that stops the updater
relaunching mid-synthesis, and the new under-provisioned-hardware preflight
(Greptile P1). A chokepoint with two doors is not a chokepoint.

Also fixes two cache defects in the preflight itself:

- An engine pick left the cached /engines response describing the PREVIOUS
  engine for up to 60s, so switching engines and generating immediately warned
  about the one you just left — or stayed silent about the one you just chose.
  notifyEngineSelected() now drops the cache, and hands over the caveat it just
  displayed so the preflight does not repeat the same sentence seconds later.
- A rejected listEngines() promise stayed cached for the full TTL, silencing
  the caveat for a minute after the backend came back. It is now evicted, but
  only if it is still the current entry, so a racing newer fetch survives.

6 new tests; 2 of the 3 streaming ones fail before this change.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Comment thread frontend/src/utils/streamingTts.js
… headers

generateSpeech releases its claim when the Response resolves — when the
HEADERS arrive — but a streaming synth generates audio for as long as the body
is read. Routing streaming through it gave it a count for the first time, then
dropped that count to zero for the entire synthesis, so the updater saw idle
and was free to relaunch mid-stream (Greptile P1).

streamGenerateSpeech now wraps the whole operation in withTtsInflight().
Nesting is harmless because the store tracks a count, not a boolean — the
inner claim just bumps it to 2 and back.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@debpalash
debpalash merged commit 07263ef into main Jul 29, 2026
16 checks passed
@debpalash
debpalash deleted the fix/generate-vram-preflight branch July 29, 2026 08:43
debpalash added a commit that referenced this pull request Jul 29, 2026
Conflict markers were committed in the previous merge — `git add` on the
directory staged both files as resolved while the markers were still in them.
Both belong to #1288 and are unchanged by this PR, so they take main's version
verbatim.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
debpalash added a commit that referenced this pull request Jul 29, 2026
* fix(engines): warn about under-provisioned hardware before the synth, not after

Six reports are the same story: #1240, #1246, #1248, #1277, #1283, #1284 —
4 GB and 6 GB cards running an engine that wants 6 GB, each one waiting out
the full 300s compute budget to be told the job "was too heavy". The routing
layer knew the whole time. The error text even names the card and the figure.

The caveat only ever surfaced on the engine-PICK toast, so it reached people
who changed engines and nobody whose engine was already selected — the
default, or one persisted from a previous session. That is most users.
/generate does return X-OmniVoice-Routing, but a response header arrives when
the job ends, five minutes too late to be a warning.

So the check moves to the chokepoint every synth path shares (api/generate.ts,
same argument as the in-flight count). Fire-and-forget: never awaited, so it
cannot add latency to the request it warns about; never throws, so an
unreachable backend costs a warning rather than a generate; once per
engine+reason per session, so it informs instead of nagging. Advisory, not
blocking — the driver can page to system RAM and short inputs fit where long
ones don't.

Extracts routingNotice() as the single frontend mirror of the backend's
routing_notice(). Two callers now need "is this verdict worth interrupting
for", and two inline copies would drift — invisibly, until someone on DirectML
or an unavailable engine gets a hardware warning for a normal pick.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(cuda): stop sending every RTX 40-series card to the CPU

The SM-arch gate required the device's exact tag in get_arch_list(). NVIDIA's
rules are not exact, and PyTorch depends on that: SASS is binary-compatible
UPWARD within a major version, so the official wheels ship sm_80/sm_86 and
deliberately no sm_89 — the 8.6 kernels already cover Ada. Exact matching
therefore declared sm_89 unsupported, check_device_compatibility() returned
False, and get_best_device() silently returned "cpu".

That is every RTX 4060/4070/4080/4090, not just the reporter's card (#1285) —
each one running TTS on the CPU on hardware that works fine, with a message
telling them their GPU was unsupported.

cuda_build_covers() now applies the real rules: sm_XY covers same-major
devices with minor >= Y; compute_XY PTX JITs forward to anything newer; an
a/f suffix (sm_90a) is architecture-specific and matches exactly. Unparseable
entries are skipped, and an empty arch list still degrades to "compatible" —
the pre-existing fail-open contract.

The remediation text also pointed at a NIGHTLY index for what is a stable
supported card; it now names the stable cu128 index.

12 tests: the Ada regression, Jetson Orin (8.7), downward-within-major and
cross-major rejection, PTX forward-JIT, arch-specific suffixes, and a genuine
sm_120-on-old-wheel mismatch so the gate is proven to still work.

Closes #1285

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(cuda): resolve app modules at call time, not import time

The tests/** review contract forbids module-level imports of app modules —
they go stale under sys.modules pollution from other suites, which is the live
cause of #1269's cross-suite failures. Binds core.device_caps per call.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix: restore generate.ts and generatePreflight.test.js from main

Conflict markers were committed in the previous merge — `git add` on the
directory staged both files as resolved while the markers were still in them.
Both belong to #1288 and are unchanged by this PR, so they take main's version
verbatim.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
debpalash added a commit that referenced this pull request Jul 29, 2026
)

* fix(engines): warn before a long CPU synth burns the whole budget

#1288 closed the under-provisioned-GPU gap but left the CPU one open, and I
missed it: a CPU-only host is a BENIGN routing verdict, so routingNotice()
correctly stays silent — yet #1299 and #1260 are exactly that shape, CPU hosts
that hit the 300s budget on long text with no warning at all. "Nothing is
misconfigured" and "this will finish in time" are different claims.

Threshold is the backend's own definition of past-short: generate_timeout_for()
gives the first 1200 characters the flat budget before extending it, so
ordinary sentences on a CPU laptop stay quiet and only the shape that actually
times out is flagged. Hardware caveats still take precedence — one toast, and
it names the real reason rather than generic advice.

5 tests; engines.cpuLongText translated in all 21 locales.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(engines): don't tell CPU-tuned engines to switch to themselves

Greptile P1. The advice names OmniVoice GGUF and Supertonic-3 as the CPU-tuned
alternatives — shown to someone already running one of them, it is advice to
switch to what they are using. Those two now get the same warning without the
self-referential clause; the engine set matches the backend's own timeout
message so the two can't disagree about who is CPU-tuned.

Also documents the preflight in docs/performance.md (docs-sync rule): both
warning shapes, why the threshold is 1200 characters (it is the figure the
budget itself uses), that they are advisory and once-per-engine-per-session,
and the CPU-tuned exception.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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.

[Bug] 500 Internal Server Error: CUDA error: out of memory CUDA kernel errors might be

1 participant