Skip to content

Commit 7277aa4

Browse files
fix(server): compare artifacts, not commit SHAs, when checking for model updates (#3073)
* fix(server): compare artifacts, not commit SHAs, when checking for updates check_for_model_updates flagged an update whenever the upstream commit SHA moved, so a README-only commit -- or any commit touching an unrelated variant in a shared multi-artifact repository -- re-raised "Update available" on every restart. download_from_registry already handles this via can_reuse_previous_hf_snapshot; the startup check never called it. - Compare the per-model artifact set (resolved checkpoints, expanded across GGUF shard families) between the cached ref and the latest commit before advertising a re-download. - Gate on Hugging Face: ModelScope snapshots are tree fingerprints with no commit pin, so they keep the snapshot-id comparison. - Every indeterminate path (missing file, fetch failure, empty set) falls back to flagging an update, so a real update is never hidden. Fixes #2542 Co-authored-by: Claude Opus 5 <noreply@anthropic.com> * fix(server): track update-check baselines separately, share artifact selection with pull - compare artifacts against the on-disk snapshot (resolved path/refs main), never the processed-at-pull sha, which may name a snapshot never materialized locally - select artifacts from the new revision's tree via the same helper pull uses, so added-in-directory and tokenizer/config-only changes are detected - verify auxiliary checkpoints under their own repository entries; a model is verified only once every repository it spans completes a determination - an indeterminate repository (no resolvable local baseline) assumes changed instead of silently skipping, so it can no longer block another repository's completed determination for the same model - any indeterminate artifact check consistently assumes changed - move the per-model repository count into registry_files::DeterminationTracker, guarded against a model being marked determined twice for the same repository - compare the union of the previous and current revision's selected files, not just the current one, so a file removed upstream (e.g. from a directory checkpoint or a GGUF shard family) isn't silently dropped out of comparison - apply the same union fix to pull's own snapshot-reuse check (download_from_registry), which shared the identical blind spot independently of this update-check path Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> * fix(server): keep same-repo auxiliary checkpoints in pull snapshot reuse * test(server): stale provenance snapshot must not flag a false update --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent fff3d7f commit 7277aa4

5 files changed

Lines changed: 1094 additions & 181 deletions

File tree

CMakeLists.txt

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2327,6 +2327,26 @@ if(BUILD_TESTING AND EXISTS "${_GGUF_SHARD_TEST_SRC}")
23272327
add_cpp_ci_test(GgufShardUtilsTest CI ON COMMAND test_gguf_shard_utils)
23282328
endif()
23292329

2330+
# Per-model artifact selection and snapshot baselines used to decide whether an
2331+
# upstream commit actually touches a downloaded model.
2332+
set(_HF_UPDATE_CHECK_TEST_SRC
2333+
"${CMAKE_CURRENT_SOURCE_DIR}/test/cpp/test_hf_update_check.cpp"
2334+
)
2335+
if(BUILD_TESTING AND EXISTS "${_HF_UPDATE_CHECK_TEST_SRC}")
2336+
add_executable(test_hf_update_check
2337+
test/cpp/test_hf_update_check.cpp
2338+
)
2339+
target_include_directories(test_hf_update_check PRIVATE
2340+
${CMAKE_CURRENT_SOURCE_DIR}/src/cpp/include
2341+
${CMAKE_CURRENT_BINARY_DIR}/include
2342+
)
2343+
target_link_libraries(test_hf_update_check PRIVATE lemonade-server-core)
2344+
add_dependencies(test_hf_update_check copy_resources)
2345+
2346+
include(CTest)
2347+
add_cpp_ci_test(HfUpdateCheckTest CI ON COMMAND test_hf_update_check)
2348+
endif()
2349+
23302350
# Model residency role/pool contract for router dependencies
23312351
set(_MODEL_RESIDENCY_TEST_SRC
23322352
"${CMAKE_CURRENT_SOURCE_DIR}/test/cpp/test_model_residency.cpp"
Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
#pragma once
2+
3+
#include <cstddef>
4+
#include <filesystem>
5+
#include <map>
6+
#include <string>
7+
#include <unordered_set>
8+
#include <vector>
9+
10+
namespace lemon {
11+
namespace registry_files {
12+
13+
// Shared by pull and the update check — a drift between them either hides an
14+
// update pull would download or flags one it would not. Throws like pull when
15+
// the variant matches nothing.
16+
std::vector<std::string> select_main_repo_files(
17+
const std::string& repo_id,
18+
const std::string& recipe,
19+
const std::string& variant,
20+
const std::vector<std::string>& repo_files);
21+
22+
// Union across two listings (e.g. previous and current revision), so a file
23+
// dropped from one side doesn't silently drop out of comparison too.
24+
std::vector<std::string> select_main_repo_files_union(
25+
const std::string& repo_id,
26+
const std::string& recipe,
27+
const std::string& variant,
28+
const std::vector<std::string>& repo_files_a,
29+
const std::vector<std::string>& repo_files_b);
30+
31+
// Keeps same-repository auxiliary checkpoints in the reuse comparison.
32+
std::vector<std::string> merge_reuse_comparison_files(
33+
const std::vector<std::string>& base_files,
34+
const std::string& repo_id,
35+
const std::string& recipe,
36+
const std::string& variant,
37+
const std::vector<std::string>& repo_files_a,
38+
const std::vector<std::string>& repo_files_b);
39+
40+
// Checkpoints without a "repo:variant" shape are skipped.
41+
std::map<std::string, std::vector<std::string>> group_aux_checkpoint_variants(
42+
const std::map<std::string, std::string>& checkpoints);
43+
44+
// Empty when the resolved path does not point inside snapshots/.
45+
std::string snapshot_id_from_resolved_path(
46+
const std::string& resolved_path,
47+
const std::filesystem::path& model_cache_path);
48+
49+
// Deliberately NOT the processed-at-pull sha recorded in
50+
// .lemonade_registry.json, which can name a snapshot that was never
51+
// materialized locally (pull keeps refs/main on an older snapshot when the
52+
// selected artifacts are unchanged).
53+
std::string active_local_snapshot(
54+
const std::string& resolved_path,
55+
const std::filesystem::path& model_cache_path);
56+
57+
// content_id identifies content across commits (LFS sha256 or git blob oid),
58+
// so revisions compare without downloading anything.
59+
struct HfFileMetadata {
60+
std::size_t size = 0;
61+
std::string content_id;
62+
std::string hash_algorithm;
63+
std::string hash_value;
64+
65+
bool has_content_id() const { return !content_id.empty(); }
66+
bool has_hash() const { return !hash_algorithm.empty() && !hash_value.empty(); }
67+
};
68+
69+
std::string hf_file_metadata_key(const std::string& repo_id, const std::string& filename);
70+
71+
// A selected file absent from tree_entries is omitted; callers must treat
72+
// that as "changed".
73+
std::map<std::string, HfFileMetadata> select_metadata(
74+
const std::string& repo_id,
75+
const std::vector<std::string>& selected_files,
76+
const std::map<std::string, HfFileMetadata>& tree_entries);
77+
78+
// False on any gap: a file missing from either revision's metadata (e.g.
79+
// added upstream inside a directory checkpoint), missing/partial on disk, or
80+
// a size mismatch.
81+
bool can_reuse_previous_hf_snapshot(
82+
const std::string& repo_id,
83+
const std::vector<std::string>& selected_files,
84+
const std::filesystem::path& previous_snapshot,
85+
const std::map<std::string, HfFileMetadata>& current_metadata,
86+
const std::map<std::string, HfFileMetadata>& previous_metadata);
87+
88+
// A model spanning several repositories (e.g. an auxiliary checkpoint outside
89+
// the main checkpoint's repository) becomes verified only once every
90+
// repository it spans has completed a determination. mark_determined() is
91+
// idempotent past zero so a caller re-marking the same model twice can't
92+
// corrupt another model's outstanding count.
93+
class DeterminationTracker {
94+
public:
95+
void add_pending(const std::string& model_name) {
96+
++pending_[model_name];
97+
}
98+
99+
// True exactly on the call that reaches zero outstanding.
100+
bool mark_determined(const std::string& model_name) {
101+
auto it = pending_.find(model_name);
102+
if (it == pending_.end() || it->second == 0) {
103+
return false;
104+
}
105+
if (--it->second == 0) {
106+
verified_.insert(model_name);
107+
return true;
108+
}
109+
return false;
110+
}
111+
112+
bool is_verified(const std::string& model_name) const {
113+
return verified_.count(model_name) != 0;
114+
}
115+
116+
std::size_t pending_count(const std::string& model_name) const {
117+
auto it = pending_.find(model_name);
118+
return it == pending_.end() ? 0 : it->second;
119+
}
120+
121+
private:
122+
std::map<std::string, std::size_t> pending_;
123+
std::unordered_set<std::string> verified_;
124+
};
125+
126+
} // namespace registry_files
127+
} // namespace lemon

0 commit comments

Comments
 (0)