Skip to content

fix(local-asr): 修 PR #262 review 提的两个 bug#263

Merged
appergb merged 1 commit into
mainfrom
fix/local-asr-pr-review-fixes
May 5, 2026
Merged

fix(local-asr): 修 PR #262 review 提的两个 bug#263
appergb merged 1 commit into
mainfrom
fix/local-asr-pr-review-fixes

Conversation

@appergb
Copy link
Copy Markdown
Collaborator

@appergb appergb commented May 5, 2026

User description

这个 PR 做什么

跟进 PR #262PR Reviewer Guide 提的两个真 bug。

Bug 1 — 小文件 retry 污染

位置asr/local/download.rs::try_download_range_append

症状:小文件(≤32MB)路径用 append 模式打开 .partial。若上一次 attempt 写了 N 字节后断流,retry 拿到的还是完整 chunk → append → .partial 比应有大小多 N 字节,永久损坏且哨兵 .openless-asr-ready 写完后 qwen_load 仍会失败。

修法:每次 attempt 用 truncate(effective_start) + seek 重写,干净起点。小文件每个 chunk 就是整个文件,重写无成本。

Bug 2 — 多文件并发一个失败不 abort 其它 worker

位置asr/local/download.rs::run_download 主 join 循环

症状:原代码 "signaling others to stop" 只是 log,真 set cancel flag。某文件持久失败时,剩下的 worker 继续吃带宽直到全部完成才冒错误,浪费用户几分钟。

修法:错误时主动 cancel.store(true);外层加 self_aborted 标记区分 "用户主动 cancel" vs "我们因错误自 abort"——前者 emit Cancelled,后者 emit Failed,不会把错误误报成用户取消。

Test plan

  • cargo check 全绿
  • 故意配错 mirror 触发 Bug 2 → 期望 emit Failed 而非 Cancelled,剩余 worker 立即停
  • 小文件下载到一半 kill app → 重启再下 → 期望文件大小匹配 HF 报告值(不会膨胀)
  • 用户中途点取消 → 期望 emit Cancelled(不要被误判 Failed)

🤖 Generated with Claude Code


PR Type

Bug fix


Description

  • Fix retry corruption on small files by switching from append to truncate+seek

  • Make any download worker failure immediately abort all parallel workers and report the error


File Walkthrough

Relevant files
Bug fix
download.rs
Fix retry corruption and abort parallel download workers on error

openless-all/app/src-tauri/src/asr/local/download.rs

  • In try_download_range_append, open the partial file with .write(true)
    instead of .append(true), then truncate to effective_start and seek to
    that position before writing the chunk. This prevents file size
    inflation when a retry writes the full chunk again over a partially
    written file.
  • In run_download, when any per‑file worker fails, set the shared cancel
    flag and record self_aborted = true. The final result now correctly
    emits Failed rather than Cancelled, and other workers stop immediately
    instead of wasting bandwidth.
+22/-7   

(1) 小文件路径 try_download_range_append 的 retry 污染:
  原实现用 append 模式打开 .partial。若上一次 attempt 已写了部分字节后失败,
  retry 拿到的还是完整 chunk → append → 文件比应有大小多 N 字节,永久损坏。
  改成 truncate(effective_start) + seek 写入,每次 attempt 从干净起点重写。
  小文件(≤ 32MB)每个 chunk 是整个文件,重写无成本。

(2) 多文件并发一个失败不 abort 其它 worker:
  原代码只 log "signaling others to stop",没真 set cancel flag。剩下的
  workers 会继续吃带宽直到全部完成才把错误冒上来。
  现在错误时主动 cancel.store(true),再用 self_aborted 标记区分这是"我们
  因错误 abort"还是"用户主动 cancel"——前者 emit Failed,后者 emit Cancelled,
  避免错误被误报成用户取消。

跟踪 PR: #262 review by github-actions PR Reviewer Guide
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 5, 2026

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

262 - Partially compliant

Compliant requirements:

  • 小文件 retry 时改为按起点截断并重写,避免 .partial 膨胀损坏。
  • worker 失败时会主动设置 cancel flag,促使其它 worker 退出。
  • 将用户取消与自我中止区分开,错误场景不再误报为 Cancelled

Non-compliant requirements:

  • 无法仅通过代码审查确认运行时行为是否与预期完全一致。

Requires further human verification:

  • 需要在实际下载场景中验证:错误镜像触发时是否会立刻停止其它 worker,并正确上报 Failed
  • 需要在实际断点续传场景中验证:小文件中断后重试,.partial 大小是否始终与远端一致。
  • 需要在实际交互中验证:用户手动取消时是否始终上报 Cancelled,且不会被误判为失败。
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ No major issues detected

@appergb appergb merged commit c85b1bd into main May 5, 2026
2 checks passed
@appergb appergb deleted the fix/local-asr-pr-review-fixes branch May 5, 2026 05:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant