Skip to content

Improve FunASR download failure guidance - #392

Merged
buxuku merged 3 commits into
buxuku:mainfrom
LauraGPT:codex/smartsub-funasr-download-guidance
Jul 23, 2026
Merged

Improve FunASR download failure guidance#392
buxuku merged 3 commits into
buxuku:mainfrom
LauraGPT:codex/smartsub-funasr-download-guidance

Conversation

@LauraGPT

Copy link
Copy Markdown
Contributor

Summary

  • show a localized FunASR model download failure toast instead of only raw HTTP/errors
  • include the manual fallback path: copy the model page link, download the model folder, then use Import from folder
  • name the required files (model.int8.onnx, tokens.txt) so users can self-check SenseVoice/Paraformer folders before importing

Root cause

When HF/hf-mirror downloads are blocked, timed out, or return a low-level error, the UI already has the right manual import workflow but the failure toast does not connect the error to that workflow. Users can interpret the model path as deleted even when the model repository still exists.

Validation

  • corepack yarn test:engines -> engine unit tests: 658 passed, 0 failed
  • corepack yarn check:i18n -> zh/en key parity OK
  • git diff --check

Note: this checkout has both package-lock.json and yarn.lock; npm ci is blocked because the npm lock is out of sync with package.json, so I used the existing Yarn lockfile for validation.

@LauraGPT LauraGPT mentioned this pull request Jul 19, 2026
@LauraGPT

Copy link
Copy Markdown
Contributor Author

Current-head validation update for 498d6e654ce998284128aed3121ef9c5acd0389c:

  • Rechecked that this PR only changes the expected FunASR download guidance surface: FunasrModelSection.tsx, funasrDownloadError.ts, the EN/ZH resource locale files, and the focused engine-unit test script.
  • Verified the formatter/helper maps SenseVoice and Paraformer to the required manual-import files model.int8.onnx + tokens.txt, and keeps Silero VAD at silero_vad.onnx.
  • Verified both EN and ZH locale keys are present and interpolate {{files}}.
  • Verified the focused unit coverage still imports formatFunasrDownloadFailureToast and asserts the required-file guidance.
  • Ran git diff --check upstream-main...HEAD successfully.

I did not rerun corepack yarn test:engines or corepack yarn check:i18n in this pass because the current remote host does not have Node/corepack/yarn/npm available in PATH. The earlier PR-body validation remains the last full JS test run evidence; this pass was a static/head-integrity recheck to make review easier.

@buxuku

buxuku commented Jul 19, 2026

Copy link
Copy Markdown
Owner

@LauraGPT Pulled this locally for review. test:engines (658 passed) and check:i18n both pass here, and the hint copy matches the actual UI: the popover's copy button does resolve to the model page URL, the button label matches importFromFolder, and the file list matches requiredFiles in funasrModelCatalog.ts.

One behavior issue to fix before merge:

Cancelling a download now triggers the failure toast with the manual-import guidance. Clicking cancel aborts the downloader, the pending downloadFunasrModel call rejects with Download cancelled, and the IPC handler wraps that into { success: false, error: 'Error: Download cancelled' } — so a deliberate cancel now shows an 8s "download failed" toast telling the user to import manually. The old code had the same wart (it showed the raw error), but this PR makes it noticeably more misleading. Either special-case it in handleDownload the way anotherDownloadInProgress is handled, or better, return { success: false, canceled: true } from the main handler (same semantics as importModel) and early-return on it in the renderer.

Non-blocking:

The \n between the raw error and the hint won't render as a line break — sonner descriptions don't preserve newlines. Add whitespace-pre-line to the description classNames in ui/sonner.tsx, or just join with a space.

REQUIRED_FILES duplicates requiredFiles from main/helpers/funasrModelCatalog.ts. Fine since these specs are frozen, but add a comment pointing at the source of truth so they don't drift silently.

FunasrModelId is now defined in three places; FunasrModelSection.tsx could import the type from the new lib instead of keeping its own copy.

Qwen/FireRed panels have the same pre-existing cancel-toast issue — out of scope here, fine to clean up separately.

@LauraGPT

Copy link
Copy Markdown
Contributor Author

Thanks for the precise repro. Fixed the cancel path in the latest head 2165d79c.

What changed:

  • Added a FunASR-specific cancellation classifier for cancelled, canceled, and AbortError cases.
  • FunasrModelSection now returns silently for active user cancellation from both the IPC { success:false, error } path and thrown abort/cancel errors.
  • Real failures such as HTTP Error: 403 still show the manual-import fallback with the required file list.

Validation:

npm run test:engines
# engine unit tests: 662 passed, 0 failed

npm run check:i18n
# zh/en key parity OK, no fallback patterns

git diff --check
# passed

This should address the remaining behavior issue you flagged: clicking cancel no longer produces the manual-import failure guidance toast.

@buxuku

buxuku commented Jul 23, 2026

Copy link
Copy Markdown
Owner

@LauraGPT Pulled 2165d79 locally and re-ran against latest main: test:engines 707 passed, check:i18n OK, no new tsc errors in the touched files. The cancel path itself is fixed — cancel → abort → Error: Download cancelled out of the IPC handler, the classifier catches it and the misleading toast is gone. Thanks for the quick turnaround.

One thing still blocks merge: isFunasrDownloadCancelled matches more than the cancel flow can actually produce. The funasr download path has exactly one cancel message, Download cancelled (CANCELLED_MESSAGE in parallelDownloader.ts, plus downloadSingle in funasrModelDownloader.ts), but the regex also accepts bare abort/aborted — and that word shows up in genuine failures. When a server or CDN drops the connection mid-body, Node's response 'error' event fires with the message literally "aborted" (ECONNRESET underneath; easy to repro with a local http server that writes a partial body then destroys the socket). parallelDownloader forwards it through res.on('error'), the renderer receives Error: aborted, and the classifier now swallows it — the download button just snaps back with no toast at all. That's worse than main (which at least surfaced the raw error), and it hits precisely in the flaky/blocked-network scenario this PR is meant to cover. Proxy-style "request aborted" messages have the same problem.

Either fix works for me:

  • narrow the pattern to the real message, e.g. /\bDownload cancell?ed\b/i — your single-l canceled test case still passes, and keeping the DOMException AbortError branch is fine; or
  • the structured route from my last comment: have downloadFunasrModel return { success: false, canceled: true } (same shape as importModel) and early-return on the flag in the renderer, so nobody has to sniff error strings.

Minor: renderer/lib/funasrDownloadError.ts and scripts/test-engine-units.ts fail prettier --check (the Translate type should collapse to one line; the DOMException call needs wrapping). Looks like the local hook got bypassed — npx prettier --write on those two is enough.

The non-blocking items from last round (the \n join, the REQUIRED_FILES source-of-truth comment, the three copies of FunasrModelId) are still open. Fine to leave for a follow-up, just flagging so they don't get lost.

@LauraGPT

Copy link
Copy Markdown
Contributor Author

Fixed the remaining cancellation-classification blocker in head 840324ab.

  • String matching now only suppresses the downloader's actual Download cancelled / Download canceled messages.
  • A plain Error: aborted network failure is no longer classified as user cancellation, so the manual-import failure guidance remains visible.
  • Kept the structured DOMException / AbortError branch for real abort objects.
  • Applied Prettier to the two flagged files.

Red/green evidence:

before: engine unit tests: 662 passed, 1 failed
        expected false for Error: aborted; actual true
after:  engine unit tests: 663 passed, 0 failed
check:i18n: zh/en key parity OK, no fallback patterns
prettier --check: all matched files use Prettier code style
git diff --check: passed

@buxuku
buxuku merged commit 25b038f into buxuku:main Jul 23, 2026
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