Skip to content

Commit 163cdea

Browse files
authored
Address model manager review feedback
1 parent 4b551e2 commit 163cdea

4 files changed

Lines changed: 43 additions & 7 deletions

File tree

CMakeLists.txt

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,9 @@ add_subdirectory(asr)
123123
add_subdirectory(translate)
124124
add_subdirectory(tts)
125125
add_subdirectory(pipeline)
126-
add_subdirectory(apps)
126+
if(VOX_BUILD_APPS OR VOX_BUILD_TESTS)
127+
add_subdirectory(apps)
128+
endif()
127129

128130
if(VOX_BUILD_TESTS)
129131
enable_testing()

apps/model_manager.cpp

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,15 @@ int run_download_command(const ManagedModel & model, const std::filesystem::path
7272
return 1;
7373
}
7474
const int result = std::system(model.download_command.c_str());
75-
std::filesystem::current_path(previous, ec);
75+
std::error_code restore_ec;
76+
std::filesystem::current_path(previous, restore_ec);
77+
if (restore_ec) {
78+
err << "Could not restore working directory " << previous << ": " << restore_ec.message() << "\n";
79+
if (result != 0) {
80+
err << "Download command failed for " << model.name << ".\n";
81+
}
82+
return 1;
83+
}
7684
if (result != 0) {
7785
err << "Download command failed for " << model.name << ".\n";
7886
return 1;
@@ -238,7 +246,7 @@ int run_model_command(
238246
installed_only = args.size() == 2;
239247
for (const ManagedModel & model : supported_models()) {
240248
const ManagedModelStatus status = inspect_model(model, project_root);
241-
if (installed_only && !status.complete) {
249+
if (installed_only && !status.installed && !status.has_partial_download) {
242250
continue;
243251
}
244252
out << model.name << "\t" << status_name(status) << "\t" << model.version << "\t";

apps/model_manager.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#pragma once
22

3+
#include <cstdint>
34
#include <filesystem>
45
#include <iosfwd>
56
#include <string>

tests/model_manager_test.cpp

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
#include "model_manager.h"
22

3+
#include <chrono>
34
#include <filesystem>
45
#include <fstream>
56
#include <iostream>
67
#include <sstream>
8+
#include <string>
79

810
namespace {
911

@@ -21,7 +23,8 @@ int main() {
2123
namespace fs = std::filesystem;
2224

2325
bool ok = true;
24-
const fs::path root = fs::temp_directory_path() / "vox_model_manager_test";
26+
const auto now = std::chrono::steady_clock::now().time_since_epoch().count();
27+
const fs::path root = fs::temp_directory_path() / ("vox_model_manager_test_" + std::to_string(now));
2528
std::error_code ec;
2629
fs::remove_all(root, ec);
2730
fs::create_directories(root / "models", ec);
@@ -43,16 +46,24 @@ int main() {
4346
status = vox::app::model::inspect_model(*model, root);
4447
ok = expect(status.installed && !status.complete, "empty model file should be incomplete") && ok;
4548

49+
std::ostringstream out;
50+
std::ostringstream err;
51+
int result = vox::app::model::run_model_command({"list", "--installed"}, root, out, err);
52+
ok = expect(result == 0, "list --installed should succeed for incomplete models") && ok;
53+
ok = expect(out.str().find("whisper-base") != std::string::npos, "installed list should include incomplete model") && ok;
54+
4655
{
4756
std::ofstream file(model_path, std::ios::binary);
4857
file << "not a real model, but enough to test file completeness";
4958
}
5059
status = vox::app::model::inspect_model(*model, root);
5160
ok = expect(status.installed && status.complete, "non-empty model file should be complete") && ok;
5261

53-
std::ostringstream out;
54-
std::ostringstream err;
55-
int result = vox::app::model::run_model_command({"verify", "whisper-base"}, root, out, err);
62+
out.str("");
63+
out.clear();
64+
err.str("");
65+
err.clear();
66+
result = vox::app::model::run_model_command({"verify", "whisper-base"}, root, out, err);
5667
ok = expect(result == 0, "verify should succeed for a complete local model") && ok;
5768
ok = expect(out.str().find("checksum: unavailable") != std::string::npos, "verify should show checksum status") && ok;
5869

@@ -72,6 +83,20 @@ int main() {
7283
ok = expect(result == 0, "remove should succeed") && ok;
7384
ok = expect(!fs::exists(model_path), "remove should delete model file") && ok;
7485

86+
{
87+
std::ofstream partial(model_path.string() + ".part", std::ios::binary);
88+
partial << "partial";
89+
}
90+
status = vox::app::model::inspect_model(*model, root);
91+
ok = expect(!status.installed && status.has_partial_download && !status.complete, "partial download should be incomplete") && ok;
92+
out.str("");
93+
out.clear();
94+
err.str("");
95+
err.clear();
96+
result = vox::app::model::run_model_command({"list", "--installed"}, root, out, err);
97+
ok = expect(result == 0, "list --installed should succeed for partial downloads") && ok;
98+
ok = expect(out.str().find("whisper-base") != std::string::npos, "installed list should include partial download") && ok;
99+
75100
fs::remove_all(root, ec);
76101
return ok ? 0 : 1;
77102
}

0 commit comments

Comments
 (0)