Skip to content

Commit 7692f1e

Browse files
mohnjilesclaude
andcommitted
Apply Gemini review: fix page-load CTS race + harden docs cache IO
- Replace pageCts synchronously before any await so rapid page selections can't leak a stale load that overwrites newer content - Cache write failures after a successful fetch no longer fail the operation (nested try-catch, logged as warnings) - Unreadable/corrupt cached tree listing now falls back to network instead of throwing - Match Engine as IMarkdownEngine2 (the getter's actual type; upgraded wrappers don't implement IMarkdownEngine) - Avoid Split('/') allocations in hot-ish path helpers Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent c82b737 commit 7692f1e

3 files changed

Lines changed: 52 additions & 19 deletions

File tree

StabilityMatrix.Avalonia/Controls/DocumentationMarkdownViewer.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,10 @@ protected override void OnPropertyChanged(AvaloniaPropertyChangedEventArgs chang
6464

6565
private void ApplyLinkCommand()
6666
{
67-
// The engine (IMarkdownEngine) owns the HyperlinkCommand used for all rendered links.
68-
if (Engine is IMarkdownEngine engine)
67+
// The engine owns the HyperlinkCommand used for all rendered links. The Engine getter
68+
// always returns an IMarkdownEngine2 (custom IMarkdownEngine values are upgraded to a
69+
// wrapper that only implements IMarkdownEngine2), so match on that interface.
70+
if (Engine is IMarkdownEngine2 engine)
6971
{
7072
engine.HyperlinkCommand = LinkCommand;
7173
}

StabilityMatrix.Avalonia/ViewModels/DocumentationViewModel.cs

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -173,11 +173,18 @@ partial void OnSelectedPageChanged(DocumentationPageNavItem? oldValue, Documenta
173173

174174
private async Task LoadPageAsync(string docsRelativePath)
175175
{
176-
// Cancel any in-flight page load
177-
await (pageCts?.CancelAsync() ?? Task.CompletedTask);
178-
pageCts?.Dispose();
179-
pageCts = new CancellationTokenSource();
180-
var ct = pageCts.Token;
176+
// Replace the CTS before any await so rapid selections can't race on the old one,
177+
// then cancel the superseded load.
178+
var oldCts = pageCts;
179+
var newCts = new CancellationTokenSource();
180+
pageCts = newCts;
181+
var ct = newCts.Token;
182+
183+
if (oldCts is not null)
184+
{
185+
await oldCts.CancelAsync();
186+
oldCts.Dispose();
187+
}
181188

182189
IsPageLoading = true;
183190
ErrorMessage = null;

StabilityMatrix.Core/Services/DocumentationService.cs

Lines changed: 36 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,14 @@ public async Task<IReadOnlyList<DocumentationSection>> GetSectionsAsync(
5252
try
5353
{
5454
paths = await FetchDocsPathsAsync(cancellationToken).ConfigureAwait(false);
55-
await WriteCachedPathsAsync(cacheFile, paths, cancellationToken).ConfigureAwait(false);
55+
try
56+
{
57+
await WriteCachedPathsAsync(cacheFile, paths, cancellationToken).ConfigureAwait(false);
58+
}
59+
catch (Exception cacheEx) when (cacheEx is not OperationCanceledException)
60+
{
61+
logger.LogWarning(cacheEx, "Failed to write docs tree to cache");
62+
}
5663
}
5764
catch (DocumentationNotAvailableException)
5865
{
@@ -99,8 +106,16 @@ public async Task<string> GetPageMarkdownAsync(
99106
client.Timeout = TimeSpan.FromSeconds(30);
100107
var markdown = await client.GetStringAsync(url, cancellationToken).ConfigureAwait(false);
101108

102-
CacheDir.Create();
103-
await cacheFile.WriteAllTextAsync(markdown, cancellationToken).ConfigureAwait(false);
109+
try
110+
{
111+
CacheDir.Create();
112+
await cacheFile.WriteAllTextAsync(markdown, cancellationToken).ConfigureAwait(false);
113+
}
114+
catch (Exception cacheEx) when (cacheEx is not OperationCanceledException)
115+
{
116+
logger.LogWarning(cacheEx, "Failed to write markdown to cache for {Path}", docsRelativePath);
117+
}
118+
104119
return markdown;
105120
}
106121
catch (Exception e) when (e is not OperationCanceledException)
@@ -162,7 +177,8 @@ private static bool IsDocPage(string docsRelativePath)
162177
if (docsRelativePath.StartsWith("images/", StringComparison.OrdinalIgnoreCase))
163178
return false;
164179

165-
var fileName = docsRelativePath.Split('/').Last();
180+
var lastSlash = docsRelativePath.LastIndexOf('/');
181+
var fileName = lastSlash < 0 ? docsRelativePath : docsRelativePath[(lastSlash + 1)..];
166182
if (fileName.Equals(".gitkeep", StringComparison.OrdinalIgnoreCase))
167183
return false;
168184

@@ -191,7 +207,7 @@ internal static IReadOnlyList<DocumentationSection> BuildSections(IReadOnlyList<
191207
}
192208

193209
var folder = path[..separatorIndex];
194-
var fileName = path.Split('/').Last();
210+
var fileName = path[(path.LastIndexOf('/') + 1)..];
195211

196212
if (!sections.TryGetValue(folder, out var list))
197213
{
@@ -247,14 +263,22 @@ private static bool IsFresh(FilePath cacheFile) =>
247263

248264
private static async Task<List<string>?> ReadCachedPathsAsync(FilePath cacheFile, CancellationToken ct)
249265
{
250-
if (!cacheFile.Exists)
251-
return null;
266+
try
267+
{
268+
if (!cacheFile.Exists)
269+
return null;
252270

253-
var text = await cacheFile.ReadAllTextAsync(ct).ConfigureAwait(false);
254-
return text.Split('\n', StringSplitOptions.RemoveEmptyEntries)
255-
.Select(line => line.Trim())
256-
.Where(line => line.Length > 0)
257-
.ToList();
271+
var text = await cacheFile.ReadAllTextAsync(ct).ConfigureAwait(false);
272+
return text.Split('\n', StringSplitOptions.RemoveEmptyEntries)
273+
.Select(line => line.Trim())
274+
.Where(line => line.Length > 0)
275+
.ToList();
276+
}
277+
catch (Exception e) when (e is not OperationCanceledException)
278+
{
279+
// Unreadable/corrupt cache — treat as a miss so callers fall back to the network.
280+
return null;
281+
}
258282
}
259283

260284
private static async Task WriteCachedPathsAsync(

0 commit comments

Comments
 (0)