.agents/skills/review-skia-update/references/writing-summaries.md
After run_review.py completes, you have raw-results.json with all mechanical check outputs.
Your job is to add human-readable summaries and assemble the final report.
For every item in added, removed, and changed across all four sections
(upstreamIntegrity, interopIntegrity, depsAudit, companionPr), add a summary field.
Note WHY this file was modified. Common reasons:
Example: "SkBitmap::getSubset() return type changed from SkIRect to SkRect — C API callers may need updating"
Describe what C API function/type was changed:
sk_surface_draw_with_sampling)Example: "Added sk_default_fontmgr.cpp/h — new platform singleton replacing removed SkFontMgr::RefDefault()"
Note what the dependency is and what changed:
Example: "libpng bumped from 1.6.40 to 1.6.43 — includes fixes for CVE-2024-31497"
For every removed item in upstreamIntegrity, you MUST verify WHY the patch was
dropped. The orchestrator leaves the working tree checked out with upstream refs fetched,
so you can inspect upstream directly.
For each removed patch:
git show upstream/{new_upstream_branch}:{file_path} # in externals/skia
grep or head for large files — search for the specific
function, constant, or pattern from the old patch.Resolution categories:
| Resolution | Evidence needed |
|---|---|
| Upstream resolved | Show the specific upstream line/value that addresses the fork's concern |
| Replaced by interop | Point to the new interop file that handles this |
| API removed upstream | Show that the patched API no longer exists in upstream |
| Unverified | Could not determine — flag for human reviewer |
Examples:
"Fork patch increased buffer limit from 16 to 128. Patch correctly dropped — new upstream sets limit to 65536 (verified at line N), which exceeds the fork's requirement.""Fork patch added FooClass::BarMethod(). Dropped — upstream removed FooClass entirely; functionality moved to C API shim layer (see src/c/sk_foo.cpp).""This is handled differently in the new upstream or was deemed unnecessary." — NEVER write this. Verify or say "unverified".The same principle applies to changed patches: if a patch changed, check upstream to understand whether the change was forced by an upstream API change vs. a deliberate fork modification.
For each file in the companion PR's added and changed arrays, describe what the C# code does:
Use relatedFiles to cross-link to the corresponding interop files. This enables
one-click navigation between a C# wrapper and its C API implementation.
Example: "New SKFoo wrapper — factory method FromData() wrapping sk_foo_new_from_data(). Validates data parameter, returns null on native failure."
Include diff content for all companion PR files — the viewer renders diffs the same way as upstream/interop sections (Direct/Patch/Old/New for changed files).
Build JSON conforming to skia-review-schema.json v1.0. Use schema-cheatsheet.md for quick reference.
Every section needs:
status — from the raw results (don't change it)summary — brief overview of the section (50+ chars)recommendations — actionable items specific to that section (array, at least 1)Top level needs:
summary — executive summary for 30-second comprehensionrecommendations — top-level action itemsriskAssessment — see risk rules below| Condition | Risk |
|---|---|
generatedFiles.status == FAIL | HIGH |
upstreamIntegrity.status == REVIEW_REQUIRED | HIGH |
interopIntegrity.status == REVIEW_REQUIRED | MEDIUM |
depsAudit.status == REVIEW_REQUIRED | MEDIUM |
| All PASS | LOW |
Highest wins (e.g., if upstream is REVIEW_REQUIRED and deps is REVIEW_REQUIRED → HIGH).
For each source file item, add relatedFiles when the change has connections to other files:
{
"path": "include/core/SkFontMgr.h",
"summary": "Fork patch added MakeDefault(). Patch dropped — C API now uses sk_create_default_fontmgr().",
"relatedFiles": [
{ "path": "src/c/sk_default_fontmgr.cpp", "relationship": "replaced by" },
{ "path": "src/c/sk_default_fontmgr.h", "relationship": "replaced by" }
]
}
Common patterns:
"replaced by" on the removed item, "replaces" on the added item"header" / "implementation"This enables one-click navigation in the HTML viewer between related changes.
The diff, oldDiff, newDiff, and patchDiff fields MUST contain actual diff text,
NOT file path references. The JSON must be self-contained — a dashboard renders diffs directly
from it. The raw results already contain the full diff strings; copy them as-is.
Save the report to {OutputDir}/{pr_number}.json — the same directory where the orchestrator
wrote raw-results.json. The orchestrator prints this path at the end of its output.
Do NOT save to any other location (not skia-review/, not the repo root, not a custom path).