Skip to content

关于模型设置-模型下载进度条的若干修复#293

Merged
appergb merged 2 commits into
Open-Less:mainfrom
Hongbro886:fix
May 7, 2026
Merged

关于模型设置-模型下载进度条的若干修复#293
appergb merged 2 commits into
Open-Less:mainfrom
Hongbro886:fix

Conversation

@Hongbro886
Copy link
Copy Markdown
Contributor

@Hongbro886 Hongbro886 commented May 6, 2026

User description

摘要

关于模型设置-模型下载进度条的若干修复

修复 / 新增 / 改进

修复下载模型区域点击后反应很慢的问题


PR Type

Bug fix, Enhancement


Description

  • Correct partial download byte count using chunk index

  • Improve cancel/retry progress state and optimistic UI

  • Remove cancelled phase styling and simplify progress visibility

  • Seed .partial.idx for sparse file consistency


Diagram Walkthrough

flowchart LR
  A["partial_actual_size()"] -- "reads .partial.idx --> chunk map" --> B["Correct downloadedBytes"]
  C["download_one()"] -- "writes empty .partial.idx" --> D["Ensure sparse partial consistency"]
  E["walk_files()"] -- "uses partial_actual_size, ignores .idx" --> F["Accurate initial progress"]
  G["handleDownload"] -- "seeds progress from local state" --> H["No zero jump on retry"]
  I["handleCancel"] -- "deletes progress, updates model.downloadedBytes" --> J["Instant partial bar"]
  K["ModelRow UI"] -- "removes cancelled phase handling" --> L["Cleaner progress display"]
Loading

File Walkthrough

Relevant files
Bug fix
download.rs
Compute real partial size and enforce idx consistency       

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

  • Added partial_actual_size to compute real downloaded bytes from
    .partial.idx chunk records
  • In download_one, ensure a .partial.idx file is created for sparse
    partial files to prevent misinterpretation
+59/-0   
models.rs
Correct downloaded size calculation in walk_files               

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

  • Ignore .partial.idx files when walking to sum downloaded sizes
  • Use partial_actual_size for .partial files instead of meta.len() to
    get true bytes
+10/-3   
Enhancement
LocalAsr.tsx
Fix cancel/retry progress state and remove cancelled UI phase

openless-all/app/src/pages/LocalAsr.tsx

  • Clear progress on cancel event and delete state key
  • Seed progress state before download to avoid zero-percentage flash
  • On cancel, optimistically update model downloadedBytes and remove
    progress entry
  • Remove cancelled phase from progress bar display logic and styling
+49/-7   

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 6, 2026

PR Reviewer Guide 🔍

(Review updated until commit 47b086e)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ No major issues detected

@appergb
Copy link
Copy Markdown
Collaborator

appergb commented May 7, 2026

@claude 审查一下,按照逻辑结构清楚分析一下,合并到主代码后可能会有哪些问题?是否可以合并?它做得如何?代码质量如何?是否破坏了前端 UI?是否结构拆分逻辑清楚?

@claude
Copy link
Copy Markdown

claude Bot commented May 7, 2026

Claude finished @appergb's task in 2m 39s —— View job


PR #293 代码审查报告

任务清单:

  • 初始化审查任务
  • 读取 download.rs 变更
  • 读取 models.rs 变更
  • 读取 LocalAsr.tsx 变更
  • 分析逻辑结构与潜在问题
  • 输出完整审查结论

总体评价

代码质量 中上,修复方向正确,逻辑链清晰。三个文件分工明确,后端负责真实字节计算,前端负责 UI 响应。存在 一个中等 UX 缺陷(非数据损坏),无安全风险,无 UI 破坏,可谨慎合并,建议同步修复下述问题后再合并或随后跟进。


逻辑结构分析

PR 的改动形成一条清晰的数据链:

download.rs                models.rs               LocalAsr.tsx
partial_actual_size()  →  walk_files() 调用  →  model.downloadedBytes  →  hasPartial / progress bar
(真实字节计算)            (跳过 .partial.idx)   (立即 started 状态)

三层分工合理,没有跨模块乱调,符合 CLAUDE.md 规定的"模块间只通过 types.rs 通信,cross-module wiring 在 coordinator"。


后端审查(download.rs + models.rs

partial_actual_size() 逻辑正确

函数设计清晰:

  • .partial.idx → 单流下载(dense),meta.len() 即真实字节 ✓
  • .partial.idx(空) → chunked 刚开始,0 字节 ✓
  • .partial.idx(有内容) → 按 idx 行数还原已完成 chunk 字节 ✓

HashSet 去重 chunk 索引,防止 idx 重复写入造成多算,思路稳健。

✅ 自动化 reviewer 的"空 idx 误判"批评是误判

自动化 review 说"Creating an empty .partial.idx whenever a download starts makes walk_files() treat every .partial as chunked"并认为 dense partial 会受影响。但代码中:

  • single_stream_downloadtotal_size == 0)根本不走 idx 路径
  • total_size <= CHUNK_SIZE 的单 chunk 路径也没有创建 .partial.idx
  • 只有真正的多 chunk 路径才 touch .partial.idx

因此自动化 reviewer 的第一条"Critical"批评并不成立

⚠️ 中等问题:partial_actual_size() 出错时静默返回 0

let content = match std::fs::read_to_string(&idx_path) {
    Ok(s) => s,
    Err(_) => return 0,  // idx 文件读失败 → 当作 0 字节
};

若 idx 文件因磁盘错误读取失败,函数返回 0 而非真实大小,导致 walk_files() 上报 0 字节已下载,UI 进度条归零。这是可接受的防御性设计(不 panic),但可以考虑记录一条 eprintln! 便于排查。这是次要问题,不阻断合并。

models.rs 修改干净

walk_files() 跳过 .partial.idx、对 .partial 用真实大小,代码改动最小且无副作用。


前端审查(LocalAsr.tsx

✅ 立即显示 started 进度条

handleDownload 开头立即写入 progress[modelId],修复了点击后反应慢的核心问题,逻辑正确。

⚠️ 中等 UX 问题:重新下载后初始字节从 0 开始而非从 partial 续传点开始

Fix this →

// 当前代码(第 204 行)
bytesDownloaded: prev[modelId]?.bytesDownloaded ?? 0,

取消下载后,progress[modelId] 被删除。用户再次点击下载时:

  • prev[modelId]undefined
  • bytesDownloaded → 0
  • 进度条从 0% 开始,直到第一个后端事件到来才跳到正确位置

而此时 model.downloadedBytes(经过乐观更新)才是真实的已下载字节。建议改为:

bytesDownloaded: prev[modelId]?.bytesDownloaded ?? (models.find(m => m.id === modelId)?.downloadedBytes ?? 0),

这样重新点击下载后进度条会从已下载位置开始。

handleCancel 乐观更新逻辑正确

捕获 lastBytes、立即更新 model.downloadedBytes、让 hasPartial 立刻生效,不等 200ms refresh——这个设计是对的,注释也写清楚了。

✅ 双重删除 progress 条目是安全的

handleCancel 和事件监听器都删除 progress[modelId],可能重复删除,但 JS 对不存在的 key 执行 delete 是安全的幂等操作,无问题。

✅ 错误路径处理正确

handleDownloadcatch 里将 started 状态翻到 failed,防止因 IPC 抛错导致进度条卡在 started 不消失。

✅ 移除 cancelled 样式是正确的

showProgress 移除 cancelled 相,进度条颜色移除灰色 cancelled 分支,与"取消后由 hasPartial 控制进度条显示"的整体设计一致。


是否破坏前端 UI?

否。 变更是收缩性的(移除一个 phase 的显示分支),没有引入新的视觉状态,现有 faileddownloadinghasPartial 路径均未受影响。


是否可以合并?

维度 结论
功能正确性 ✅ 核心修复逻辑正确
数据安全 ✅ 无数据丢失风险
前端 UI ✅ 无破坏
结构拆分 ✅ 分工清晰
主要 UX 缺陷 ⚠️ 重下时进度从 0 开始(可接受,首个 event 后自动修正)
测试覆盖 ⚠️ 无自动化测试

结论:可以合并,建议将 bytesDownloaded 初始化问题作为后续 follow-up 修复,或在本 PR 内一并处理后合并。自动化 reviewer 的"Incorrect size accounting"批评经分析是误判,不影响合并判断。

@appergb
Copy link
Copy Markdown
Collaborator

appergb commented May 7, 2026

@Hongbro886 解决 claude 提出的问题,然后再次准备完成后即可合并。

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 7, 2026

Persistent review updated to latest commit 47b086e

@appergb
Copy link
Copy Markdown
Collaborator

appergb commented May 7, 2026

@claude 需要严格审核他提交的内容和更改,同时为跨平台做好准备和分析。

@claude
Copy link
Copy Markdown

claude Bot commented May 7, 2026

Claude finished @appergb's task in 3m 46s —— View job


任务清单:

  • 读取变更文件(download.rsmodels.rsLocalAsr.tsx
  • 执行 git diff 分析实际改动(2 个 commit)
  • 严格审查代码逻辑与质量
  • 跨平台兼容性分析(macOS vs Windows)
  • 输出完整审查报告

PR #293 严格审查 + 跨平台兼容性分析

本次审查涵盖全部 2 个 commit(7a3866c47b086e),重点关注逻辑正确性、边界条件、以及 macOS / Windows 跨平台差异。


一、跨平台兼容性分析

🔴 严重:Windows NTFS 不创建真正的稀疏文件

Fix this →

download_one()f.set_len(total_size).partial 文件撑到最终大小:

// download.rs ~line 558
f.set_len(total_size)
    .with_context(|| format!("set_len partial failed: {}", partial.display()))?;
平台 行为 结果
macOS (APFS/HFS+) ftruncate 创建真正的 sparse 文件,holes 不占磁盘 ✅ 正常
Windows (NTFS) SetEndOfFile 立刻分配全部磁盘空间,不创建 sparse 文件 ❌ 下载 1.7B 模型时,Windows 会在第一个字节到达前就要求 1.7 GB 可用磁盘空间

Windows 要创建真正的 sparse 文件,必须在 SetEndOfFile 之前调用 DeviceIoControl(handle, FSCTL_SET_SPARSE, ...) 标记为稀疏文件。当前代码在 Windows 上的实际行为:

  1. 用户点击下载 → 立即尝试预分配完整模型大小的磁盘空间
  2. 磁盘不足时立刻报错,而不是"下了多少占多少"
  3. 即使成功,Windows 磁盘占用在下载期间一直是全量,不是真实已下字节

修复建议:使用条件编译,Windows 路径跳过稀疏文件预分配,改用顺序写入:

#[cfg(windows)]
{
    // Windows: 不预分配 sparse,直接创建空文件,每 chunk seek+write
    std::fs::OpenOptions::new().write(true).create(true).open(&partial)?;
}
#[cfg(not(windows))]
{
    let f = std::fs::OpenOptions::new().write(true).create(true).open(&partial)?;
    f.set_len(total_size)?;
}

或者引入 winapi crate 调用 FSCTL_SET_SPARSE

目前如果 Windows 上 engineAvailable = false,下载按钮被禁用,此问题暂时不影响用户,但一旦 Windows 引擎就绪,这将是一个 P0 bug。


🟡 中等:eprintln! 在生产构建里不可见

partial_actual_size() 使用 eprintln! 打印错误:

// download.rs ~line 471-476
eprintln!(
    "[local-asr] partial_actual_size: stat partial failed ({}): {}",
    partial.display(),
    e
);

eprintln! 输出到 stderr。在打包后的生产应用(macOS .app、Windows .exe)中,stderr 通常被丢弃,不会出现在 ~/Library/Logs/OpenLess/openless.log%LOCALAPPDATA%\OpenLess\Logs\openless.log。应统一使用 log::warn! 与其余代码保持一致:

log::warn!("[local-asr] partial_actual_size: stat partial failed ({}): {}", partial.display(), e);

Fix this →


🟢 Windows 并发写同一文件

chunk_with_retry_seek 的多个 task 同时以独立文件句柄写同一 .partial 文件的不同偏移。Windows 上 Rust 的 OpenOptions 默认使用 FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,允许多写入方,且每个句柄有独立的文件指针——并发 seek+write 安全,无问题。

🟢 文件重命名

tokio::fs::rename 在 Windows 内部使用 MoveFileExW 并带 MOVEFILE_REPLACE_EXISTING 标志,若目标存在会原子替换。此处 dest 在下载开始前已确认不存在(dest.exists() 跳过逻辑),安全。

🟢 路径分隔符

HF API 返回 / 分隔的相对路径,dir.join(&file.path) 在 Windows 上 Rust 的 PathBuf 会正确处理前向斜杠,无兼容问题。


二、代码逻辑严格审查

🟡 already_done_bytes 在续传时漏算其他文件的 partial 字节

// download.rs ~line 295-306
let already_done_bytes: u64 = info
    .files
    .iter()
    .map(|f| {
        let d = dir.join(&f.path);
        if d.exists() { f.size } else { 0 }  // ← 只统计已完成的文件
    })
    .sum();

此处只统计最终文件(非 .partial)。若上次下载已有 3 个文件中的 2 个完成了 50%,这些 partial 字节不在 already_done_bytes 中,也不在 in_flight_bytes(初始为 0)中。续传时,前几个 Progress 事件报告的 bytes_downloaded 会低于磁盘实际字节,进度条会出现短暂回退。

后端 Started 事件用 downloaded_bytes(model_id)(走 walk_filespartial_actual_size)是准确的,但后续 Progress 事件会低估——这是视觉抖动,不是数据损失。

🟡 .partial.idx 格式缺乏版本锁定,CHUNK_SIZE 变更时静默失效

// partial_actual_size
let start = (idx as u64).saturating_mul(CHUNK_SIZE);  // CHUNK_SIZE = 8MB

IDX 文件存储的是 chunk 索引(0-based 整数),字节偏移由 idx * CHUNK_SIZE 还原。若未来版本将 CHUNK_SIZE 从 8MB 改为 16MB,旧的 .partial.idx 文件会让 partial_actual_size 计算出错误字节数,且没有任何报错。建议在 IDX 文件头写入 CHUNK_SIZE 版本,或改为存储字节偏移而非 chunk 索引。

handleDownload 的乐观初始化(第二次 commit 修复)

第二个 commit 中的修复是正确的:

// LocalAsr.tsx ~line 199-201
const model = models.find(m => m.id === modelId);
const initialDownloaded =
  progress[modelId]?.bytesDownloaded ?? model?.downloadedBytes ?? 0;

取消后 progress[modelId] 已删除,progress[modelId]?.bytesDownloadedundefined,此时回退到 model?.downloadedBytes(cancel 时已乐观写入),避免了进度条从 0% 跳跃。这是上一次审查遗留问题的正确修复。

✅ 事件监听器与 handleCancel 的双重删除

两处都调用 delete next[modelId],JS 对不存在的 key 执行 delete 是幂等操作,无害。

partial_actual_size 的边界防御

if start >= total_size {
    continue;  // 防御 IDX 里的越界 chunk 索引
}

使用 HashSet 去重防止 IDX 重复写入造成多算,边界正确。

models.rs 的改动最小且无副作用

正确跳过 .partial.idx,对 .partial 调用 partial_actual_size,对完整文件用 meta.len(),逻辑清晰。


三、质量评估

维度 评分 说明
逻辑正确性 ✅ 中上 核心修复方向对,eprintln! 是一个疏漏
跨平台 ⚠️ 有待改进 Windows sparse 文件预分配是潜在 P0;现阶段因 engineAvailable 防护暂未触发
代码结构 ✅ 良好 三文件分工清晰,无跨模块乱调
可观测性 ⚠️ 有缺陷 eprintln! 在生产包不可见,影响排查
向后兼容 ⚠️ 潜在问题 CHUNK_SIZE 变更会静默破坏旧 partial
UI 破坏 ✅ 无 移除 cancelled 样式分支是收缩性改动,不影响其他路径

四、合并建议

建议在合并前修复:

  1. partial_actual_size 里的 eprintln! 改为 log::warn!(5 分钟改动)

建议后续跟进(不阻断合并):
2. Windows sparse 文件预分配——等 Windows 引擎就绪时必须处理
3. already_done_bytes 续传进度低估——视觉问题,不影响数据
4. .partial.idx 缺乏版本头——可在下一次大改时加

eprintln!log::warn! 修复后,可以合并。

@appergb
Copy link
Copy Markdown
Collaborator

appergb commented May 7, 2026

@claude 帮忙修复一下这些问题。

@claude
Copy link
Copy Markdown

claude Bot commented May 7, 2026

Claude finished @appergb's task in 6m 43s —— View job


审查问题修复完成(待 PR 作者应用)

任务清单:

  • 分析需要修复的问题
  • 读取源文件(download.rsmodels.rsLocalAsr.tsxCargo.toml
  • 修复 eprintln!log::warn!(合并前必须修复)
  • 修复 Windows 稀疏文件预分配(添加 FSCTL_SET_SPARSE 调用)
  • 修复 already_done_bytes 续传时漏算 partial 字节
  • 生成 patch(push 因 GitHub App 无 workflow 写权限被拒,详见下方说明)

推送说明

三处修改均已本地提交(commit 03fea4e,仅改动 download.rs),但推送失败,原因:
本次检出的 fix 分支历史包含若干旧的 CI 工作流变更提交(来自已合并的 PR),GitHub App 缺少 workflows 写权限,拒绝整体推送。

解决方法@Hongbro886 直接将下方 patch 应用到你的 PR 分支即可:

# 在 PR 分支根目录执行:
curl -s <此 comment 的 raw URL> | git am
# 或手动 cherry-pick 后 push

已修复内容(文件:openless-all/app/src-tauri/src/asr/local/download.rs

修复一:eprintln!log::warn!

partial_actual_size 里的两处 eprintln! 在打包后的生产应用中输出到 stderr 被丢弃,改为 log::warn! 后写入 openless.log,方便排查进度条无故归零。

-            eprintln!(
+            log::warn!(
                 "[local-asr] partial_actual_size: stat partial failed ({}): {}",
-            eprintln!(
+            log::warn!(
                 "[local-asr] partial_actual_size: read idx failed ({}): {}",

修复二:Windows NTFS 稀疏文件预分配

set_len 在 NTFS 上不创建稀疏文件(立即占满磁盘)。通过在 set_len 前调用 FSCTL_SET_SPARSE 标记文件,让 holes 真正不占磁盘。使用原始 FFI(不需要新增依赖),标记失败时静默降级。

partial_actual_size 函数之后、download_one 之前插入:

/// Windows NTFS: `set_len` 本身不创建 sparse 文件——会立即分配全量磁盘。
/// 需先通过 `FSCTL_SET_SPARSE` 把文件标记为 sparse,之后 `set_len` 的 holes 才真正不占磁盘。
/// 标记失败时静默降级:`set_len` 仍可继续,只是会占满磁盘空间。
#[cfg(target_os = "windows")]
fn mark_file_sparse(f: &std::fs::File) {
    use std::os::windows::io::AsRawHandle;

    const FSCTL_SET_SPARSE: u32 = 0x000900C4;

    extern "system" {
        fn DeviceIoControl(
            hDevice: *mut std::ffi::c_void,
            dwIoControlCode: u32,
            lpInBuffer: *const std::ffi::c_void,
            nInBufferSize: u32,
            lpOutBuffer: *mut std::ffi::c_void,
            nOutBufferSize: u32,
            lpBytesReturned: *mut u32,
            lpOverlapped: *const std::ffi::c_void,
        ) -> i32;
    }

    let mut bytes_returned: u32 = 0;
    let ok = unsafe {
        DeviceIoControl(
            f.as_raw_handle() as *mut std::ffi::c_void,
            FSCTL_SET_SPARSE,
            std::ptr::null(),
            0,
            std::ptr::null_mut(),
            0,
            &mut bytes_returned,
            std::ptr::null(),
        )
    };
    if ok == 0 {
        log::warn!("[local-asr] FSCTL_SET_SPARSE 失败,partial 文件将以非稀疏方式预分配(会占用完整磁盘空间)");
    }
}

在预分配块中添加调用(f.set_len 之前):

     // 3. 预先把 .partial 撑到最终大小(sparse 文件,holes = 零字节)
+    // macOS/Linux: ftruncate 直接创建 sparse 文件。
+    // Windows NTFS: 必须先调 FSCTL_SET_SPARSE 标记,再 set_len;否则 NTFS 立即分配全量磁盘。
     if !partial.exists() || std::fs::metadata(&partial).map(|m| m.len()).unwrap_or(0) != total_size {
         let f = std::fs::OpenOptions::new()
             .write(true)
             .create(true)
             .open(&partial)
             .with_context(|| format!("create partial failed: {}", partial.display()))?;
+        #[cfg(target_os = "windows")]
+        mark_file_sparse(&f);
         f.set_len(total_size)
             .with_context(|| format!("set_len partial failed: {}", partial.display()))?;
     }

修复三:already_done_bytes 续传时漏算 partial 字节

原代码只统计已完成文件,.partial 文件的已下字节不计入初始进度,导致续传开始时进度条短暂向后跳动。

     let already_done_bytes: u64 = info
         .files
         .iter()
         .map(|f| {
             let d = dir.join(&f.path);
             if d.exists() {
                 f.size
             } else {
-                0
+                let partial = d.with_extension("partial");
+                if partial.exists() {
+                    partial_actual_size(&partial)
+                } else {
+                    0
+                }
             }
         })
         .sum();

说明:以上三处修改合计 +52/-3 行,仅改动 download.rs,无需修改 Cargo.toml(Windows FFI 通过原始 extern "system" 调用,kernel32 在 Windows 目标上默认链接)。

@appergb appergb merged commit 32133b3 into Open-Less:main May 7, 2026
3 checks passed
@Hongbro886 Hongbro886 deleted the fix branch May 7, 2026 12:26
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.

2 participants