Skip to content

Commit 9ee0a7e

Browse files
mohnjilesclaude
andcommitted
Address review feedback and fix CI test failures
- Switch debounce from DispatcherTimer to CancellationTokenSource + Task.Delay so it works in headless unit tests (where there's no Avalonia UI dispatcher) and removes the orphan `_ = SearchModels()` fire-and-forget — the debounced helper now awaits SearchModels directly - Expose SearchDebounceInterval as a public property so tests can collapse it to TimeSpan.Zero instead of waiting hundreds of ms per assertion - Update the two ViewModel tests broken by debounce: set the interval to zero, and add a brief await in the queued-while-loading test so the debounced fire-and-forget Task.Delay(0) propagates before SetResult - Track array depth in CivArchiveSingleStringConverter so a hypothetical nested array doesn't strand the JSON reader mid-structure (return on the matching EndArray, not past it — STJ verifies cursor position) - Serialize concurrent first-load callers of GetFilterOptionsAsync via a SemaphoreSlim so a cold cache + concurrent invocations don't fire redundant fetches (matches the existing buildId pattern) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 31b41b2 commit 9ee0a7e

3 files changed

Lines changed: 105 additions & 44 deletions

File tree

StabilityMatrix.Avalonia/ViewModels/CheckpointBrowser/CivArchiveBrowserViewModel.cs

Lines changed: 43 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,14 @@ IModelIndexService modelIndexService
3636
private bool filterOptionsLoaded;
3737
private bool searchQueued;
3838
private int currentPage = 1;
39-
private DispatcherTimer? searchDebounceTimer;
39+
private CancellationTokenSource? searchDebounceCts;
40+
41+
/// <summary>
42+
/// How long to wait after the most recent filter change before firing the search.
43+
/// Kept settable (not a const) so unit tests can collapse it to <see cref="TimeSpan.Zero"/>
44+
/// instead of waiting hundreds of ms per assertion.
45+
/// </summary>
46+
public TimeSpan SearchDebounceInterval { get; set; } = TimeSpan.FromMilliseconds(300);
4047

4148
/// <summary>
4249
/// All search results we've fetched so far across pages, regardless of client-side filters.
@@ -272,16 +279,13 @@ protected override async Task OnInitialLoadedAsync()
272279
EventManager.Instance.ModelIndexChanged += indexHandler;
273280
AddDisposable(Disposable.Create(() => EventManager.Instance.ModelIndexChanged -= indexHandler));
274281

275-
// Debounce filter-driven searches so rapid toggles (e.g. picking a dozen base
276-
// models in a row) collapse into a single API call instead of N — CivArchive
277-
// 429s aggressively otherwise.
278-
searchDebounceTimer = new DispatcherTimer { Interval = TimeSpan.FromMilliseconds(300) };
279-
searchDebounceTimer.Tick += OnSearchDebounceElapsed;
282+
// Cancel any pending debounced search when the VM is disposed.
280283
AddDisposable(
281284
Disposable.Create(() =>
282285
{
283-
searchDebounceTimer.Stop();
284-
searchDebounceTimer.Tick -= OnSearchDebounceElapsed;
286+
searchDebounceCts?.Cancel();
287+
searchDebounceCts?.Dispose();
288+
searchDebounceCts = null;
285289
})
286290
);
287291

@@ -350,16 +354,10 @@ private void RebuildVisibleResults()
350354
OnPropertyChanged(nameof(IsEndOfResults));
351355
}
352356

353-
private void OnSearchDebounceElapsed(object? sender, EventArgs e)
354-
{
355-
searchDebounceTimer?.Stop();
356-
_ = SearchModels();
357-
}
358-
359357
/// <summary>
360-
/// Restart the debounce timer. The actual search runs once the user pauses for the
361-
/// timer's interval — multiple rapid filter changes within that window collapse into
362-
/// a single fetch.
358+
/// Cancel any pending debounced search and start a new wait window. The actual
359+
/// search runs once the user pauses for <see cref="SearchDebounceInterval"/> —
360+
/// multiple rapid filter changes within that window collapse into a single fetch.
363361
/// </summary>
364362
private void RequestDebouncedSearch()
365363
{
@@ -375,16 +373,40 @@ private void RequestDebouncedSearch()
375373
return;
376374
}
377375

378-
searchDebounceTimer?.Stop();
379-
searchDebounceTimer?.Start();
376+
searchDebounceCts?.Cancel();
377+
searchDebounceCts?.Dispose();
378+
379+
var cts = new CancellationTokenSource();
380+
searchDebounceCts = cts;
381+
382+
_ = RunDebouncedSearchAsync(cts.Token);
383+
}
384+
385+
private async Task RunDebouncedSearchAsync(CancellationToken token)
386+
{
387+
try
388+
{
389+
await Task.Delay(SearchDebounceInterval, token);
390+
}
391+
catch (TaskCanceledException)
392+
{
393+
return;
394+
}
395+
396+
if (token.IsCancellationRequested)
397+
{
398+
return;
399+
}
400+
401+
await SearchModels();
380402
}
381403

382404
[RelayCommand]
383405
private async Task SearchModels(bool isInfiniteScroll = false)
384406
{
385407
// Cancel any pending debounced search so an explicit invocation (search button,
386-
// ResetFilters, etc.) doesn't get shadowed by a redundant fire 300ms later.
387-
searchDebounceTimer?.Stop();
408+
// ResetFilters, etc.) doesn't get shadowed by a redundant fire moments later.
409+
searchDebounceCts?.Cancel();
388410

389411
if (IsLoading)
390412
{

StabilityMatrix.Core/Api/CivArchiveApiClient.cs

Lines changed: 52 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ IHttpClientFactory httpClientFactory
2929

3030
private readonly HttpClient httpClient = CreateHttpClient(httpClientFactory);
3131
private readonly SemaphoreSlim buildIdLock = new(1, 1);
32+
private readonly SemaphoreSlim filterOptionsLock = new(1, 1);
3233
private string? cachedBuildId;
3334

3435
private readonly LRUCache<string, CacheEntry<CivArchiveSearchResponse>> searchCache = new(64);
@@ -118,25 +119,41 @@ public async Task<CivArchiveFilterOptions> GetFilterOptionsAsync(
118119
return cached.Value;
119120
}
120121

121-
// Parameterless URL is intentional — CivArchive returns empty filter-option arrays
122-
// the moment any query param is present, even if every value is the default.
123-
var response = await GetNextDataAsync<CivArchiveListPageResponse>(
124-
"/top-models.json",
125-
cancellationToken
126-
);
122+
// Singleton API client → multiple concurrent first-load callers could otherwise
123+
// fire redundant fetches before the cache is populated. Serialize them so only
124+
// one request goes out per cold cache.
125+
await filterOptionsLock.WaitAsync(cancellationToken);
126+
try
127+
{
128+
if (cachedFilterOptions is { } cachedRetry && cachedRetry.IsFresh(FilterOptionsCacheTtl))
129+
{
130+
return cachedRetry.Value;
131+
}
127132

128-
var pageProps =
129-
response.PageProps
130-
?? throw new InvalidOperationException("CivArchive list page was missing pageProps");
133+
// Parameterless URL is intentional — CivArchive returns empty filter-option arrays
134+
// the moment any query param is present, even if every value is the default.
135+
var response = await GetNextDataAsync<CivArchiveListPageResponse>(
136+
"/top-models.json",
137+
cancellationToken
138+
);
131139

132-
var result = new CivArchiveFilterOptions
133-
{
134-
BaseModels = pageProps.FilterOptions?.BaseModels ?? [],
135-
ModelTypes = pageProps.FilterOptions?.ModelTypes ?? [],
136-
};
140+
var pageProps =
141+
response.PageProps
142+
?? throw new InvalidOperationException("CivArchive list page was missing pageProps");
137143

138-
cachedFilterOptions = new CacheEntry<CivArchiveFilterOptions>(DateTimeOffset.UtcNow, result);
139-
return result;
144+
var result = new CivArchiveFilterOptions
145+
{
146+
BaseModels = pageProps.FilterOptions?.BaseModels ?? [],
147+
ModelTypes = pageProps.FilterOptions?.ModelTypes ?? [],
148+
};
149+
150+
cachedFilterOptions = new CacheEntry<CivArchiveFilterOptions>(DateTimeOffset.UtcNow, result);
151+
return result;
152+
}
153+
finally
154+
{
155+
filterOptionsLock.Release();
156+
}
140157
}
141158

142159
public async Task<CivArchiveModelDetailsResponse> GetModelDetailsAsync(
@@ -654,17 +671,29 @@ JsonSerializerOptions options
654671

655672
if (reader.TokenType == JsonTokenType.StartArray)
656673
{
674+
// Track depth so a hypothetical nested array (e.g. [["a"]]) doesn't leave
675+
// the reader stranded mid-structure for the next property to deserialize.
676+
// Must return *on* the matching EndArray token (not past it) — STJ strictly
677+
// verifies the converter's final cursor position.
657678
string? first = null;
679+
var depth = 1;
658680
while (reader.Read())
659681
{
660-
if (reader.TokenType == JsonTokenType.EndArray)
661-
{
662-
return first;
663-
}
664-
665-
if (reader.TokenType == JsonTokenType.String && first is null)
682+
switch (reader.TokenType)
666683
{
667-
first = reader.GetString();
684+
case JsonTokenType.StartArray:
685+
depth++;
686+
break;
687+
case JsonTokenType.EndArray:
688+
depth--;
689+
if (depth == 0)
690+
{
691+
return first;
692+
}
693+
break;
694+
case JsonTokenType.String when first is null:
695+
first = reader.GetString();
696+
break;
668697
}
669698
}
670699

StabilityMatrix.Tests/Avalonia/CivArchiveBrowserViewModelTests.cs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,9 @@ public async Task ChangingFilters_TriggersNewQueryAndResetsPaging()
4545
});
4646

4747
var vm = CreateViewModel(apiClient, out _, out _);
48+
// Collapse the debounce so the sort-change re-fetch happens within the test
49+
// window instead of after the production-default 300ms idle delay.
50+
vm.SearchDebounceInterval = TimeSpan.Zero;
4851
vm.OnLoaded();
4952

5053
await vm.SearchModelsCommand.ExecuteAsync(false);
@@ -147,13 +150,20 @@ public async Task ChangingFilterWhileLoading_QueuesRefreshWithLatestFilter()
147150
});
148151

149152
var vm = CreateViewModel(apiClient, out _, out _);
153+
vm.SearchDebounceInterval = TimeSpan.Zero;
150154
vm.OnLoaded();
151155

152156
await vm.SearchModelsCommand.ExecuteAsync(false);
153157

154158
var loadingSearch = vm.SearchModelsCommand.ExecuteAsync(false);
155159
vm.SelectedSort = vm.AllSorts.First(x => x.Value == CivArchiveSortOption.Newest);
156160

161+
// Let the debounced fire-and-forget task spin up: it'll await Task.Delay(0),
162+
// call SearchModels, see IsLoading=true, and set searchQueued. After the
163+
// delayed in-flight call completes, the queued mechanism re-fires with the
164+
// newest sort.
165+
await Task.Delay(50);
166+
157167
delayedResponse.SetResult(CreateSearchResponse(1));
158168
await loadingSearch;
159169

0 commit comments

Comments
 (0)