Skip to content

Commit 374f285

Browse files
bmehta001Copilot
andauthored
Cherry-pick [SDKv1] Self-heal catalog cache on BYOM cache-miss (#760) (#772)
The SDKs cache the catalog in memory with a 6-hour TTL. When a user manually drops a model directory (BYOM) into the cache while the SDK is running, the SDK''s in-memory map is stale: Core returns the BYOM id from `get_cached_models` and `get_model_list` IPCs, but the SDK filters it out for getModel/getModelVariant because its `modelIdToModelVariant` map is still stale. ## Self-heal on cache-miss Force-refresh path in the four user-facing entry points: - `get_cached_models` / `get_loaded_models`: when any id in the fresh list returned by Core does not resolve against the local map, force a catalog refresh and re-resolve only the unresolved ids. - `get_model(alias)` / `get_model_variant(id)`: on miss, force a refresh and retry the lookup once. A `UpdateModels(ct)` warm-up call was added to the C# `get_model` / `get_model_variant` paths for consistency with the other SDKs (not strictly required for BYOM). ## Catalog refresh is now incremental Previously every catalog refresh was clear-and-rebuild, which orphaned every `Model` / `ModelVariant` wrapper a caller was holding. After this PR, refresh preserves wrapper identity for ids that survive: - C# / JS / Python: `Catalog.UpdateModels` reuses existing `ModelVariant` wrappers and calls `RefreshInfo(info)` to surface fresh metadata in place. `Model` wrappers are reused per alias and `RefreshVariants` swaps the backing variant list by reference (no torn reads). Evicted ids are dropped from the maps. - Rust: `apply_model_list` reuses the same `Arc<Model>` across refreshes when the `(id, cached)` fingerprint is unchanged, so the inner `AtomicUsize` selected-variant index survives. Explicit `SelectVariant()` choices are preserved trivially because the wrapper identity is preserved. ## Limitations If the previously-selected variant is removed by a refresh, `SelectedVariant` keeps pointing at the dropped wrapper and callers must explicitly re-select - pre-existing issue that would take ~30 lines per SDK to fix, but unlikely to happen. ## Tests New `CatalogSelfHealTests` and `CatalogIncrementalRefreshTests` (+ JS / Python equivalents) cover: self-heal on cache miss, no refresh on empty/whitespace input, identity + info propagation across refreshes, selection survival, and add/remove transitions. Validated against a real BYOM directory dropped into the cache at runtime. ================= More detailed description: _cachedModels (disk) and _cachedInfo (Azure metadata) are always populated at the start FLCore Changes: - Disk gets scanned in - `RefreshLocalCachedModelsAsync` (thread-safe function bc lock) in `AzureModelCatalog.cs` — refreshes the in-memory `_cachedModels` dictionary from disk. - *"We expect users will not manually delete an Azure-cataloged model, so we do not make a new trip to Azure Catalog or invalidate `_cachedInfo` here."* - New BYOM ids get inserted with `Model = null` (we assume they do not match a model in Azure Catalog), so no Azure call. - `GetCurrentCachedModelsAndPaths` (existing) to extract `modelId`. In FLCore, `RefreshLocalCachedModelsAsync` is called by `GetCachedModelsAsync`, `GetModelsAsync`, and `RemoveCachedModelAsync`. It will be called by `GetModelInfoAsync` only if we have not created a BYOM model stub for the catalog, but it will be cached after that. | Core method | When RefreshLocalCachedModelsAsync is called | |------------------------------------------|------------------------------------------------------------| | `GetCachedModelsAsync` | Always | | `GetModelsAsync` | Always | | `RemoveCachedModelAsync` | Always | | `GetModelInfoAsync` | At most once each time we see a new BYOM model | `GetModelInfoAsync` ONLY calls `RefreshLocalCachedModelsAsync` if we looked up a BYOM model by calling (`get_model_path`, `load_model`, `unload_model` IPC handlers called by SDKs or OpenAI API and by `GetChatClientAsync` on every chat completion), and we have not created a BYOM model stub for the catalog. Once we have created a BYOM model stub, it is cached for the rest of the session, so we only validate `Directory.Exists` for the BYOM model (1 syscall is much quicker than scanning whole directory) after that to ensure it has not been deleted. (If it has been deleted, we return `null` for AzureFoundryLocalModel) HTTP Method changes: | HTTP endpoint | Core method backing it | |------------------------------------------------|--------------------------------------------------------------------------------------------------------------| | `GET /v1/models` | `GetCachedModelsAsync()` (always rescans disk after this PR) + then will only check if directory for BYOM model exists in `GetModelInfoAsync` | | `GET /openai/models` | `GetCachedModelsAsync()` (always rescans disk after this PR) | | `POST /v1/chat/completions` (and friends) | `GetModelInfoAsync(id_or_alias)` — validates `Directory.Exists`, falls through to refresh + synth if needed | | `GET /openai/load/{model}` | same `GetModelInfoAsync` lookup | | `GET /openai/unload/{model}` | same `GetModelInfoAsync` lookup | SDK changes: | SDK method | Scan disk trigger | |-----------------------------------------|------------------------------------------------------------| | `catalog.GetModelAsync(alias)` | Alias not in local map and if we exceed TTL of 6 hours* | | `catalog.GetModelVariantAsync(id)` | Id not in local map and if we exceed TTL of 6 hours* | | `catalog.GetCachedModelsAsync()` | Always **| | `catalog.GetLoadedModelsAsync()` | If load-manager's list contains an id missing from local map (unlikely, but may be possible if multiple processes using SDK) | | `model.IsCachedAsync()` | Always, bc this is fresher than model.Info.Cached | \* we may end up scanning twice if we have a. once if we have hit the TTL and would have anyway and b. because of this PR, after scanning, if we still can't find the model (likely bc of a typo), we will scan again due to force refresh after cache miss ** - we will end up scanning twice - once to get the model name (string) for GetCachedModels, another time because we call `UpdateModels(ct, force: true)` to get the ModelInfo Flow for GetModelAsync ``` Program.cs └─ catalog.GetModelAsync(byomAlias) │ sdk/cs/src/Catalog.cs:73 → GetModelImplAsync at line 157 │ ├─ UpdateModels(ct, force: false) # within 6h TTL → no-op │ ├─ _modelAliasToModel.TryGetValue(alias, …)→null # cache miss │ ├─ UpdateModels(ct, force: true) # SELF-HEAL │ │ └─ _coreInterop.ExecuteCommandAsync("get_model_list") │ │ │ IPC → foundry-local-service │ │ ▼ │ │ Core AzureModelCatalog.GetModelsAsync(ct) │ │ ├─ base.GetModelsAsync(ct) # uses 4h Azure cache; no extra catalog calls │ │ ├─ RefreshLocalCachedModelsAsync(ct) # ALWAYS re-scans disk │ │ │ └─ GetCurrentCachedModelsAndPaths() │ │ │ └─ Directory.GetDirectories(_cacheDirectory, *, AllDirectories) │ │ │ + for each dir: read inference_model.json → modelId │ │ ├─ drop stale Local stubs whose dir was deleted │ │ ├─ append cache-only stubs for new BYOMs │ │ └─ return combined list │ ▼ └─ _modelAliasToModel.TryGetValue(alias, …) → Model ``` Flow for GetModelVariantAsync is similar; we just access a different map ``` Program.cs └─ catalog.GetModelVariantAsync(byomId) │ sdk/cs/src/Catalog.cs:81 → GetModelVariantImplAsync at line 187 │ ├─ UpdateModels(ct, force: false) # within 6h TTL → no-op │ ├─ _modelIdToModelVariant.TryGetValue(id, …) → null # cache miss │ ├─ UpdateModels(ct, force: true) # SELF-HEAL (same IPC + disk rescan as A1) │ ▼ └─ _modelIdToModelVariant.TryGetValue(id, …) → Model ``` Flow for GetCachedModelsAsync must scan the disk twice - once to return strings for model names from FLCore's GetCachedModelsAsync and once to fetch latest GetModelsAsync (which has Model information). GetLoadedModelsAsync is similar. ``` Program.cs └─ catalog.GetCachedModelsAsync(byomId) │ sdk/cs/src/Catalog.cs:73 → GetModelImplAsync at line 157 │ ├─ UpdateModels(ct, force: false) # within 6h TTL → no-op │ ├─ _modelAliasToModel.TryGetValue(alias, …)→null # cache miss │ ├─ UpdateModels(ct, force: true) # SELF-HEAL │ │ └─ _coreInterop.ExecuteCommandAsync("get_model_list") │ │ │ IPC → foundry-local-service │ │ ▼ │ │ Core AzureModelCatalog.GetModelsAsync(ct) │ │ ├─ base.GetModelsAsync(ct) # uses 4h Azure cache; no extra catalog calls │ │ ├─ RefreshLocalCachedModelsAsync(ct) # ALWAYS re-scans disk │ │ │ └─ GetCurrentCachedModelsAndPaths() │ │ │ └─ Directory.GetDirectories(_cacheDirectory, *, AllDirectories) │ │ │ + for each dir: read inference_model.json → modelId │ │ ├─ drop stale Local stubs whose dir was deleted │ │ ├─ append cache-only stubs for new BYOMs │ │ └─ return combined list │ ▼ └─ _modelAliasToModel.TryGetValue(alias, …) → Model ``` Get Loaded models ``` Program.cs └─ model.LoadAsync(ct) # sdk/cs/src/Detail/Model.cs:153 └─ SelectedVariant.LoadAsync(ct) # sdk/cs/src/Detail/ModelVariant.cs:95 └─ _modelLoadManager.LoadModelAsync(modelId, customPath=null, ct) # ModelManager.cs:65 ├─ _modelCatalog.GetModelInfoAsync(modelId) │ │ AzureModelCatalog.GetModelInfoAsync override │ ├─ base.GetModelInfoAsync(modelId) # miss for new BYOM │ ├─ RefreshLocalCachedModelsAsync(ct) # SELF-HEAL: rescan disk │ │ └─ GetCurrentCachedModelsAndPaths() # finds the new BYOM dir │ └─ TrySynthesizeByomStubAsync(modelId) # writes stub into _cachedInfo │ └─ returns stub # cached for next time ├─ modelInfo != null → proceed to load └─ _modelCatalog.GetModelPathAsync(modelId) # re-uses cached stub → InferenceModel constructed from dir returns Status.Success ✓ FIX_ACTIVE ``` --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent c4d5350 commit 374f285

14 files changed

Lines changed: 1472 additions & 107 deletions

File tree

sdk/cs/src/Catalog.cs

Lines changed: 135 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
namespace Microsoft.AI.Foundry.Local;
88
using System;
99
using System.Collections.Generic;
10+
using System.Linq;
1011
using System.Text.Json;
1112
using System.Threading.Tasks;
1213

@@ -102,52 +103,113 @@ private async Task<List<IModel>> ListModelsImplAsync(CancellationToken? ct = nul
102103

103104
private async Task<List<IModel>> GetCachedModelsImplAsync(CancellationToken? ct = null)
104105
{
106+
await UpdateModels(ct).ConfigureAwait(false);
107+
105108
var cachedModelIds = await Utils.GetCachedModelIdsAsync(_coreInterop, ct).ConfigureAwait(false);
109+
return await ResolveModelIdsAsync(cachedModelIds, ct).ConfigureAwait(false);
110+
}
111+
112+
private async Task<List<IModel>> GetLoadedModelsImplAsync(CancellationToken? ct = null)
113+
{
114+
await UpdateModels(ct).ConfigureAwait(false);
106115

107-
List<IModel> cachedModels = [];
108-
foreach (var modelId in cachedModelIds)
116+
var loadedModelIds = await _modelLoadManager.ListLoadedModelsAsync(ct).ConfigureAwait(false);
117+
return await ResolveModelIdsAsync(loadedModelIds, ct).ConfigureAwait(false);
118+
}
119+
120+
// Resolve a list of model ids against the in-memory catalog, self-healing once
121+
// if any id is unknown (e.g. a manually-added BYOM model the SDK has not yet seen).
122+
// Preserves the input order of modelIds (minus unknowns).
123+
private async Task<List<IModel>> ResolveModelIdsAsync(string[] modelIds, CancellationToken? ct)
124+
{
125+
bool needsRefresh = false;
126+
using (var disposable = await _lock.LockAsync().ConfigureAwait(false))
109127
{
110-
if (_modelIdToModelVariant.TryGetValue(modelId, out ModelVariant? modelVariant))
128+
foreach (var modelId in modelIds)
111129
{
112-
cachedModels.Add(modelVariant);
130+
if (!_modelIdToModelVariant.ContainsKey(modelId))
131+
{
132+
needsRefresh = true;
133+
break;
134+
}
113135
}
114136
}
115137

116-
return cachedModels;
117-
}
118-
119-
private async Task<List<IModel>> GetLoadedModelsImplAsync(CancellationToken? ct = null)
120-
{
121-
var loadedModelIds = await _modelLoadManager.ListLoadedModelsAsync(ct).ConfigureAwait(false);
122-
List<IModel> loadedModels = [];
138+
if (needsRefresh)
139+
{
140+
await UpdateModels(ct, force: true).ConfigureAwait(false);
141+
}
123142

124-
foreach (var modelId in loadedModelIds)
143+
List<IModel> resolved = new(modelIds.Length);
144+
using (var disposable = await _lock.LockAsync().ConfigureAwait(false))
125145
{
126-
if (_modelIdToModelVariant.TryGetValue(modelId, out ModelVariant? modelVariant))
146+
foreach (var modelId in modelIds)
127147
{
128-
loadedModels.Add(modelVariant);
148+
if (_modelIdToModelVariant.TryGetValue(modelId, out ModelVariant? variant))
149+
{
150+
resolved.Add(variant);
151+
}
129152
}
130153
}
131154

132-
return loadedModels;
155+
return resolved;
133156
}
134157

135158
private async Task<Model?> GetModelImplAsync(string modelAlias, CancellationToken? ct = null)
136159
{
160+
if (string.IsNullOrWhiteSpace(modelAlias))
161+
{
162+
return null;
163+
}
164+
137165
await UpdateModels(ct).ConfigureAwait(false);
138166

139-
using var disposable = await _lock.LockAsync().ConfigureAwait(false);
140-
_modelAliasToModel.TryGetValue(modelAlias, out Model? model);
167+
Model? model;
168+
using (var disposable = await _lock.LockAsync().ConfigureAwait(false))
169+
{
170+
_modelAliasToModel.TryGetValue(modelAlias, out model);
171+
}
172+
if (model is not null)
173+
{
174+
return model;
175+
}
141176

177+
// Self-heal: the alias may belong to a BYOM model added to the cache
178+
// directory after our last catalog refresh.
179+
await UpdateModels(ct, force: true).ConfigureAwait(false);
180+
using (var disposable = await _lock.LockAsync().ConfigureAwait(false))
181+
{
182+
_modelAliasToModel.TryGetValue(modelAlias, out model);
183+
}
142184
return model;
143185
}
144186

145187
private async Task<ModelVariant?> GetModelVariantImplAsync(string modelId, CancellationToken? ct = null)
146188
{
189+
if (string.IsNullOrWhiteSpace(modelId))
190+
{
191+
return null;
192+
}
193+
147194
await UpdateModels(ct).ConfigureAwait(false);
148195

149-
using var disposable = await _lock.LockAsync().ConfigureAwait(false);
150-
_modelIdToModelVariant.TryGetValue(modelId, out ModelVariant? modelVariant);
196+
ModelVariant? modelVariant;
197+
using (var disposable = await _lock.LockAsync().ConfigureAwait(false))
198+
{
199+
_modelIdToModelVariant.TryGetValue(modelId, out modelVariant);
200+
}
201+
if (modelVariant is not null)
202+
{
203+
return modelVariant;
204+
}
205+
206+
// Self-heal: the id may belong to a BYOM model added to the cache
207+
// directory after our last catalog refresh.
208+
await UpdateModels(ct, force: true).ConfigureAwait(false);
209+
using (var disposable = await _lock.LockAsync().ConfigureAwait(false))
210+
{
211+
_modelIdToModelVariant.TryGetValue(modelId, out modelVariant);
212+
}
151213
return modelVariant;
152214
}
153215

@@ -190,10 +252,13 @@ private async Task<IModel> GetLatestVersionImplAsync(IModel modelOrModelVariant,
190252
return latest.Id == modelOrModelVariant.Id ? modelOrModelVariant : latest;
191253
}
192254

193-
private async Task UpdateModels(CancellationToken? ct)
255+
private async Task UpdateModels(CancellationToken? ct, bool force = false)
194256
{
195257
// TODO: make this configurable
196-
if (DateTime.Now - _lastFetch < TimeSpan.FromHours(6))
258+
// Skip if the cache is still fresh, unless the caller forced a refresh
259+
// (e.g. self-heal after a cache miss caused by a manually-added BYOM
260+
// model dropped into the cache directory).
261+
if (!force && DateTime.Now - _lastFetch < TimeSpan.FromHours(6))
197262
{
198263
return;
199264
}
@@ -215,26 +280,64 @@ private async Task UpdateModels(CancellationToken? ct)
215280

216281
using var disposable = await _lock.LockAsync().ConfigureAwait(false);
217282

218-
// TODO: Do we need to clear this out, or can we just add new models?
219-
_modelAliasToModel.Clear();
220-
_modelIdToModelVariant.Clear();
283+
// Incremental refresh: preserve wrapper identity for ids/aliases that
284+
// survive the refresh so externally-held IModel references keep
285+
// working with up-to-date metadata and (for Model) keep any explicit
286+
// SelectVariant() choice. New ids get fresh wrappers; removed ids get
287+
// evicted.
288+
289+
var freshIds = new HashSet<string>(StringComparer.Ordinal);
290+
var freshAliasGroups = new Dictionary<string, List<ModelInfo>>(StringComparer.Ordinal);
291+
foreach (var info in models)
292+
{
293+
freshIds.Add(info.Id);
294+
if (!freshAliasGroups.TryGetValue(info.Alias, out var group))
295+
{
296+
group = [];
297+
freshAliasGroups[info.Alias] = group;
298+
}
299+
group.Add(info);
300+
}
221301

222-
foreach (var modelInfo in models)
302+
foreach (var staleId in _modelIdToModelVariant.Keys.Where(id => !freshIds.Contains(id)).ToList())
223303
{
224-
var variant = new ModelVariant(modelInfo, _modelLoadManager, _coreInterop, _logger);
304+
_modelIdToModelVariant.Remove(staleId);
305+
}
306+
foreach (var staleAlias in _modelAliasToModel.Keys.Where(a => !freshAliasGroups.ContainsKey(a)).ToList())
307+
{
308+
_modelAliasToModel.Remove(staleAlias);
309+
}
225310

226-
var existingModel = _modelAliasToModel.TryGetValue(modelInfo.Alias, out Model? value);
227-
if (!existingModel)
311+
foreach (var info in models)
312+
{
313+
if (_modelIdToModelVariant.TryGetValue(info.Id, out var existing))
228314
{
229-
value = new Model(variant, _logger);
230-
_modelAliasToModel[modelInfo.Alias] = value;
315+
existing.RefreshInfo(info);
231316
}
232317
else
233318
{
234-
value!.AddVariant(variant);
319+
_modelIdToModelVariant[info.Id] = new ModelVariant(info, _modelLoadManager, _coreInterop, _logger);
235320
}
321+
}
236322

237-
_modelIdToModelVariant[variant.Id] = variant;
323+
foreach (var kvp in freshAliasGroups)
324+
{
325+
var alias = kvp.Key;
326+
var aliasInfos = kvp.Value;
327+
var aliasVariants = aliasInfos.ConvertAll(i => _modelIdToModelVariant[i.Id]);
328+
if (_modelAliasToModel.TryGetValue(alias, out var existingModel))
329+
{
330+
existingModel.RefreshVariants(aliasVariants);
331+
}
332+
else
333+
{
334+
var m = new Model(aliasVariants[0], _logger);
335+
for (var i = 1; i < aliasVariants.Count; i++)
336+
{
337+
m.AddVariant(aliasVariants[i]);
338+
}
339+
_modelAliasToModel[alias] = m;
340+
}
238341
}
239342

240343
_lastFetch = DateTime.Now;

sdk/cs/src/Detail/Model.cs

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ public class Model : IModel
1212
{
1313
private readonly ILogger _logger;
1414

15-
private readonly List<IModel> _variants;
15+
private List<IModel> _variants;
1616
public IReadOnlyList<IModel> Variants => _variants;
1717
internal IModel SelectedVariant { get; set; } = default!;
1818

@@ -50,7 +50,9 @@ internal void AddVariant(ModelVariant variant)
5050
_logger);
5151
}
5252

53-
_variants.Add(variant);
53+
// Build a fresh list and swap by reference so concurrent readers of
54+
// Variants never observe a torn collection mid-mutation.
55+
_variants = [.. _variants, variant];
5456

5557
// prefer the highest priority locally cached variant
5658
if (variant.Info.Cached && !SelectedVariant.Info.Cached)
@@ -59,6 +61,35 @@ internal void AddVariant(ModelVariant variant)
5961
}
6062
}
6163

64+
/// <summary>
65+
/// Replace the variant list in place while preserving wrapper identity.
66+
/// Called by <see cref="Catalog.UpdateModels"/> during incremental
67+
/// refresh so a user's held <see cref="Model"/> reference keeps pointing
68+
/// at the same object across refreshes. Because
69+
/// <see cref="Catalog.UpdateModels"/> reuses the same
70+
/// <see cref="ModelVariant"/> wrappers for ids that survive a refresh,
71+
/// any explicit <see cref="SelectVariant"/> choice that survives the
72+
/// refresh is preserved without any extra work here.
73+
///
74+
/// We swap the backing list by reference rather than mutating it so
75+
/// readers iterating <see cref="Variants"/> on another thread cannot
76+
/// observe a torn collection.
77+
/// </summary>
78+
// TODO: tighten the held-reference contract for the case where the
79+
// previously selected variant is removed by a refresh; today
80+
// SelectedVariant keeps pointing at the dropped wrapper and callers must
81+
// explicitly re-select.
82+
internal void RefreshVariants(IList<ModelVariant> variants)
83+
{
84+
if (variants == null || variants.Count == 0)
85+
{
86+
throw new FoundryLocalException(
87+
$"Cannot refresh model {Alias} with an empty variant list", _logger);
88+
}
89+
90+
_variants = [.. variants];
91+
}
92+
6293
/// <summary>
6394
/// Select a specific model variant from <see cref="Variants"/> to use for <see cref="IModel"/> operations.
6495
/// </summary>

sdk/cs/src/Detail/ModelVariant.cs

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,12 @@ internal class ModelVariant : IModel
1717
private readonly ICoreInterop _coreInterop;
1818
private readonly ILogger _logger;
1919

20-
public ModelInfo Info { get; } // expose the full info record
20+
public ModelInfo Info { get; private set; } // expose the full info record
2121

2222
// expose a few common properties directly
2323
public string Id => Info.Id;
2424
public string Alias => Info.Alias;
25-
public int Version { get; init; } // parsed from Info.Version if possible, else 0
25+
public int Version { get; private set; } // parsed from Info.Version if possible, else 0
2626

2727
public IReadOnlyList<IModel> Variants => [this];
2828

@@ -38,6 +38,23 @@ internal ModelVariant(ModelInfo modelInfo, IModelLoadManager modelLoadManager, I
3838

3939
}
4040

41+
/// <summary>
42+
/// Replace the cached <see cref="ModelInfo"/> snapshot in place.
43+
/// Called by <see cref="Catalog.UpdateModels"/> during incremental refresh
44+
/// so wrapper identity is preserved across refreshes while still surfacing
45+
/// fresh metadata (notably <c>Cached</c>) on held references. <c>Id</c>
46+
/// and <c>Alias</c> are immutable for a given variant; callers must only
47+
/// invoke this with a <paramref name="modelInfo"/> whose id matches
48+
/// <see cref="Id"/>. Reference assignment in CLR is atomic, so concurrent
49+
/// readers observe either the old or new snapshot, never a torn
50+
/// intermediate.
51+
/// </summary>
52+
internal void RefreshInfo(ModelInfo modelInfo)
53+
{
54+
Info = modelInfo;
55+
Version = modelInfo.Version;
56+
}
57+
4158
// simpler and always correct to check if loaded from the model load manager
4259
// this allows for multiple instances of ModelVariant to exist
4360
public async Task<bool> IsLoadedAsync(CancellationToken? ct = null)

0 commit comments

Comments
 (0)