Skip to content

Commit 834c56c

Browse files
mattleibowCopilot
andcommitted
Fix models to match actual persisted JSON format
The persisted ai-review JSON differs from the JSON schema: - schema_version is top-level, not in meta - meta uses flat snake_case fields (pr_number, pr_head_sha, etc.) - SHAs are flat in meta, not nested in a 'shas' object - companion_pr is a string ('#3560'), not an object - meta includes enriched PR data (pr_title, pr_state, pr_author) Update SkiaReviewReport, SkiaReviewMeta, GenerateCommand, and SkiaUpdateDetail to match. Both 170.json and 171.json now parse. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 2311a23 commit 834c56c

3 files changed

Lines changed: 67 additions & 86 deletions

File tree

src/SkiaSharp.Triage.Cli/Commands/GenerateCommand.cs

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1032,30 +1032,30 @@ private async Task GenerateSkiaReviewsAsync(CacheService rootCache, DashboardCon
10321032
var review = JsonSerializer.Deserialize<SkiaReviewReport>(json, TriageJsonOptions.Default);
10331033
if (review is null) continue;
10341034

1035-
if (review.Meta.SchemaVersion is not "1.0")
1035+
if (review.SchemaVersion is not "1.0")
10361036
{
10371037
if (!settings.Quiet)
1038-
AnsiConsole.MarkupLine($"[yellow] ⚠ Skipping {Path.GetFileName(file)}: schema {review.Meta.SchemaVersion} (need 1.0)[/]");
1038+
AnsiConsole.MarkupLine($"[yellow] ⚠ Skipping {Path.GetFileName(file)}: schema {review.SchemaVersion} (need 1.0)[/]");
10391039
continue;
10401040
}
10411041

1042-
var prNumber = review.Meta.SkiaPrNumber;
1042+
var prNumber = review.Meta.PrNumber;
10431043

10441044
// Write detail file
10451045
var detailPath = Path.Combine(reviewOutputDir, $"{prNumber}.json");
10461046
await File.WriteAllTextAsync(detailPath, JsonSerializer.Serialize(review, TriageJsonOptions.Default));
10471047

10481048
// Look up PR info
1049-
string? prTitle = null, prState = null, prUrl = null;
1049+
string? prTitle = review.Meta.PrTitle, prState = review.Meta.PrState, prUrl = null;
10501050
if (prByNumber.TryGetValue(prNumber, out var prItem))
10511051
{
1052-
prTitle = prItem.Title;
1053-
prState = prItem.State;
1054-
prUrl = $"https://github.com/{repoConfig?.FullName ?? review.Meta.Repo}/pull/{prNumber}";
1052+
prTitle ??= prItem.Title;
1053+
prState ??= prItem.State;
1054+
prUrl = $"https://github.com/{repoConfig?.FullName ?? "mono/skia"}/pull/{prNumber}";
10551055
}
10561056
else
10571057
{
1058-
prUrl = $"https://github.com/{review.Meta.Repo}/pull/{prNumber}";
1058+
prUrl = $"https://github.com/{repoConfig?.FullName ?? "mono/skia"}/pull/{prNumber}";
10591059
}
10601060

10611061
// Stats
@@ -1072,9 +1072,9 @@ private async Task GenerateSkiaReviewsAsync(CacheService rootCache, DashboardCon
10721072

10731073
indexEntries.Add(new SkiaReviewIndexEntry(
10741074
PrNumber: prNumber,
1075-
UpstreamBranch: review.Meta.UpstreamBranch,
1075+
UpstreamBranch: review.Meta.NewUpstreamBranch,
10761076
OldUpstreamBranch: review.Meta.OldUpstreamBranch,
1077-
AnalyzedAt: review.Meta.AnalyzedAt,
1077+
AnalyzedAt: review.Meta.Timestamp,
10781078
RiskAssessment: review.RiskAssessment,
10791079
GeneratedFilesStatus: review.GeneratedFiles.Status,
10801080
UpstreamIntegrityStatus: review.UpstreamIntegrity.Status,
@@ -1089,7 +1089,7 @@ private async Task GenerateSkiaReviewsAsync(CacheService rootCache, DashboardCon
10891089
PrTitle: prTitle,
10901090
PrState: prState,
10911091
PrUrl: prUrl,
1092-
SkiasharpPrNumber: review.Meta.SkiasharpPrNumber
1092+
SkiasharpPrNumber: ParseCompanionPrNumber(review.Meta.CompanionPr)
10931093
));
10941094
}
10951095
catch (Exception ex)
@@ -1129,6 +1129,15 @@ private async Task GenerateSkiaReviewsAsync(CacheService rootCache, DashboardCon
11291129
AnsiConsole.MarkupLine($"[green] ✓ {reviewOutputDir}/ ({total} files)[/]");
11301130
}
11311131
}
1132+
1133+
private static int? ParseCompanionPrNumber(string? companionPr)
1134+
{
1135+
if (companionPr is null) return null;
1136+
var hashIdx = companionPr.LastIndexOf('#');
1137+
if (hashIdx >= 0 && int.TryParse(companionPr[(hashIdx + 1)..], out var num))
1138+
return num;
1139+
return null;
1140+
}
11321141
}
11331142

11341143
// Helper class for building monthly trends with per-repo breakdown

src/SkiaSharp.Triage.Dashboard/Pages/SkiaUpdateDetail.razor

Lines changed: 34 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -23,17 +23,17 @@ else
2323
<!-- Header -->
2424
<div class="triage-detail-header triage-detail-header-minimal">
2525
<div class="detail-title-row">
26-
<a href="https://github.com/@_review.Meta.Repo/pull/@_review.Meta.SkiaPrNumber" target="_blank" rel="noopener noreferrer" class="issue-link-lg">#@_review.Meta.SkiaPrNumber</a>
27-
<h1>@(_indexEntry?.PrTitle ?? $"Skia Update: {FormatBranch(_review.Meta.OldUpstreamBranch)} → {FormatBranch(_review.Meta.UpstreamBranch)}")</h1>
26+
<a href="https://github.com/mono/skia/pull/@_review.Meta.PrNumber" target="_blank" rel="noopener noreferrer" class="issue-link-lg">#@_review.Meta.PrNumber</a>
27+
<h1>@(_review.Meta.PrTitle ?? _indexEntry?.PrTitle ?? $"Skia Update: {FormatBranch(_review.Meta.OldUpstreamBranch)} → {FormatBranch(_review.Meta.NewUpstreamBranch)}")</h1>
2828
</div>
2929
<div class="detail-header-meta">
3030
<span class="badge-risk badge-risk-@_review.RiskAssessment.ToString().ToLower()">
3131
@_review.RiskAssessment Risk
3232
</span>
3333
<span class="detail-meta-text">
34-
@FormatBranch(_review.Meta.OldUpstreamBranch)@FormatBranch(_review.Meta.UpstreamBranch)
35-
· Analyzed: @_review.Meta.AnalyzedAt.ToString("yyyy-MM-dd HH:mm UTC")
36-
· <a href="https://github.com/@_review.Meta.Repo" target="_blank" rel="noopener noreferrer">@_review.Meta.Repo</a>
34+
@FormatBranch(_review.Meta.OldUpstreamBranch)@FormatBranch(_review.Meta.NewUpstreamBranch)
35+
· Analyzed: @_review.Meta.Timestamp.ToString("yyyy-MM-dd HH:mm UTC")
36+
· <a href="https://github.com/mono/skia" target="_blank" rel="noopener noreferrer">mono/skia</a>
3737
</span>
3838
</div>
3939
</div>
@@ -71,12 +71,12 @@ else
7171
@(_review.DepsAudit.Status == SkiaReviewStatus.Pass ? "✓ PASS" : "⚠ REVIEW")
7272
</span>
7373
</div>
74-
@if (_review.Meta.SkiasharpPrNumber is not null)
74+
@if (ParseCompanionPrNumber(_review.Meta.CompanionPr) is { } companionNum)
7575
{
7676
<div class="qf-group">
7777
<span class="qf-label">Companion</span>
78-
<a href="https://github.com/mono/SkiaSharp/pull/@_review.Meta.SkiasharpPrNumber" target="_blank" rel="noopener noreferrer" class="label">
79-
SkiaSharp #@_review.Meta.SkiasharpPrNumber
78+
<a href="https://github.com/mono/SkiaSharp/pull/@companionNum" target="_blank" rel="noopener noreferrer" class="label">
79+
SkiaSharp #@companionNum
8080
</a>
8181
</div>
8282
}
@@ -99,7 +99,7 @@ else
9999
<button type="button" class="content-tab @(_activeTab == "deps" ? "active" : "")" @onclick='() => _activeTab = "deps"'>
100100
DEPS (@(_review.DepsAudit.Added.Count + _review.DepsAudit.Changed.Count))
101101
</button>
102-
@if (_review.CompanionPr is not null)
102+
@if (_review.Meta.CompanionPr is not null)
103103
{
104104
<button type="button" class="content-tab @(_activeTab == "companion" ? "active" : "")" @onclick='() => _activeTab = "companion"'>
105105
Companion PR
@@ -133,7 +133,7 @@ else
133133

134134
<p>
135135
This review covers the Skia upstream merge from
136-
<strong>@_review.Meta.OldUpstreamBranch</strong> to <strong>@_review.Meta.UpstreamBranch</strong>.
136+
<strong>@_review.Meta.OldUpstreamBranch</strong> to <strong>@_review.Meta.NewUpstreamBranch</strong>.
137137
</p>
138138

139139
<div class="overview-grid">
@@ -201,9 +201,9 @@ else
201201

202202
<h3 class="section-title">Commit SHAs</h3>
203203
<div class="skia-shas">
204-
<span><strong>PR Head:</strong> <code>@Truncate(_review.Meta.Shas.PrHead)</code></span>
205-
<span><strong>Base:</strong> <code>@Truncate(_review.Meta.Shas.Base)</code></span>
206-
<span><strong>Upstream:</strong> <code>@Truncate(_review.Meta.Shas.Upstream)</code></span>
204+
<span><strong>PR Head:</strong> <code>@Truncate(_review.Meta.PrHeadSha)</code></span>
205+
<span><strong>Base:</strong> <code>@Truncate(_review.Meta.BaseSha)</code></span>
206+
<span><strong>Upstream:</strong> <code>@Truncate(_review.Meta.UpstreamSha)</code></span>
207207
</div>
208208
</div>
209209
}
@@ -552,47 +552,26 @@ else
552552
}
553553

554554
<!-- Tab: Companion PR -->
555-
@if (_activeTab == "companion" && _review.CompanionPr is not null)
555+
@if (_activeTab == "companion" && _review.Meta.CompanionPr is not null)
556556
{
557-
var cp = _review.CompanionPr;
557+
var companionPrNumber = ParseCompanionPrNumber(_review.Meta.CompanionPr);
558558
<div class="section-card">
559559
<h3 class="section-title">Companion PR</h3>
560560
<div class="quick-facts-row">
561561
<div class="qf-group">
562562
<span class="qf-label">PR</span>
563-
<a href="https://github.com/mono/SkiaSharp/pull/@cp.PrNumber" target="_blank" rel="noopener noreferrer" class="label">
564-
SkiaSharp #@cp.PrNumber
565-
</a>
566-
</div>
567-
@if (cp.FilesChanged is not null)
568-
{
569-
<div class="qf-group">
570-
<span class="qf-label">Files Changed</span>
571-
<span class="qf-detail">@cp.FilesChanged</span>
572-
</div>
573-
}
574-
</div>
575-
@if (cp.GeneratedFilesSkipped is { Count: > 0 })
576-
{
577-
<h4 class="change-category">Generated Files Skipped (@cp.GeneratedFilesSkipped.Count)</h4>
578-
<ul class="skia-file-list">
579-
@foreach (var file in cp.GeneratedFilesSkipped)
563+
@if (companionPrNumber is not null)
580564
{
581-
<li><code>@file</code></li>
565+
<a href="https://github.com/mono/SkiaSharp/pull/@companionPrNumber" target="_blank" rel="noopener noreferrer" class="label">
566+
@_review.Meta.CompanionPr
567+
</a>
582568
}
583-
</ul>
584-
}
585-
@if (cp.Findings is { Count: > 0 })
586-
{
587-
<h4 class="change-category">Findings (@cp.Findings.Count)</h4>
588-
@foreach (var finding in cp.Findings)
589-
{
590-
<div class="skia-file-entry">
591-
<div class="file-path"><code>@finding.File</code></div>
592-
<div class="file-summary">@finding.Finding</div>
593-
</div>
594-
}
595-
}
569+
else
570+
{
571+
<span class="label">@_review.Meta.CompanionPr</span>
572+
}
573+
</div>
574+
</div>
596575
</div>
597576
}
598577
}
@@ -624,6 +603,15 @@ else
624603
private static string Truncate(string sha) =>
625604
sha.Length > 12 ? sha[..12] : sha;
626605

606+
private static int? ParseCompanionPrNumber(string? companionPr)
607+
{
608+
if (companionPr is null) return null;
609+
var hashIdx = companionPr.LastIndexOf('#');
610+
if (hashIdx >= 0 && int.TryParse(companionPr[(hashIdx + 1)..], out var num))
611+
return num;
612+
return null;
613+
}
614+
627615
private void ToggleDiff(string key)
628616
{
629617
if (!_expandedDiffs.Remove(key))

src/SkiaSharp.Triage.Models/SkiaReviewModels.cs

Lines changed: 13 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -5,34 +5,31 @@ namespace SkiaSharp.Triage.Models;
55
// ── Root ─────────────────────────────────────────────────────────
66

77
public record SkiaReviewReport(
8+
[property: JsonPropertyName("schema_version")] string SchemaVersion,
89
SkiaReviewMeta Meta,
910
string Summary,
1011
List<string> Recommendations,
1112
[property: JsonPropertyName("generated_files")] GeneratedFilesCheck GeneratedFiles,
1213
[property: JsonPropertyName("upstream_integrity")] SourceIntegrity UpstreamIntegrity,
1314
[property: JsonPropertyName("interop_integrity")] SourceIntegrity InteropIntegrity,
1415
[property: JsonPropertyName("deps_audit")] DepsAudit DepsAudit,
15-
[property: JsonPropertyName("risk_assessment")] SkiaRiskAssessment RiskAssessment,
16-
[property: JsonPropertyName("companion_pr")] CompanionPr? CompanionPr = null
16+
[property: JsonPropertyName("risk_assessment")] SkiaRiskAssessment RiskAssessment
1717
);
1818

1919
// ── Meta ─────────────────────────────────────────────────────────
2020

2121
public record SkiaReviewMeta(
22-
[property: JsonPropertyName("schemaVersion")] string SchemaVersion,
23-
[property: JsonPropertyName("skiaPrNumber")] int SkiaPrNumber,
24-
[property: JsonPropertyName("skiasharpPrNumber")] int? SkiasharpPrNumber,
25-
string Repo,
26-
[property: JsonPropertyName("upstreamBranch")] string UpstreamBranch,
27-
[property: JsonPropertyName("oldUpstreamBranch")] string OldUpstreamBranch,
28-
[property: JsonPropertyName("analyzedAt")] DateTime AnalyzedAt,
29-
SkiaReviewShas Shas
30-
);
31-
32-
public record SkiaReviewShas(
33-
[property: JsonPropertyName("prHead")] string PrHead,
34-
string Base,
35-
string Upstream
22+
[property: JsonPropertyName("pr_number")] int PrNumber,
23+
[property: JsonPropertyName("pr_title")] string? PrTitle,
24+
[property: JsonPropertyName("pr_state")] string? PrState,
25+
[property: JsonPropertyName("pr_author")] string? PrAuthor,
26+
[property: JsonPropertyName("old_upstream_branch")] string OldUpstreamBranch,
27+
[property: JsonPropertyName("new_upstream_branch")] string NewUpstreamBranch,
28+
[property: JsonPropertyName("base_sha")] string BaseSha,
29+
[property: JsonPropertyName("pr_head_sha")] string PrHeadSha,
30+
[property: JsonPropertyName("upstream_sha")] string UpstreamSha,
31+
DateTime Timestamp,
32+
[property: JsonPropertyName("companion_pr")] string? CompanionPr = null
3633
);
3734

3835
// ── Generated Files ──────────────────────────────────────────────
@@ -117,16 +114,3 @@ public record DepChanged(
117114
[property: JsonPropertyName("new_revision")] string? NewRevision = null
118115
);
119116

120-
// ── Companion PR (optional) ──────────────────────────────────────
121-
122-
public record CompanionPr(
123-
[property: JsonPropertyName("pr_number")] int PrNumber,
124-
[property: JsonPropertyName("files_changed")] int? FilesChanged = null,
125-
[property: JsonPropertyName("generated_files_skipped")] List<string>? GeneratedFilesSkipped = null,
126-
List<CompanionPrFinding>? Findings = null
127-
);
128-
129-
public record CompanionPrFinding(
130-
string File,
131-
string Finding
132-
);

0 commit comments

Comments
 (0)