[api-docs] Fix false-positive extraction and add merge guard (#4030)

[api-docs] Fix false-positive extraction and add merge guard (#4030)

Context: mono/SkiaSharp-API-docs#115
Companion: mono/SkiaSharp-API-docs#116

Review of the automated docs output (PR #115) revealed three systemic
pipeline issues: the extract regex matched legitimate prose containing
"to be added" (e.g. SKPath.AddPath's "elements to be added to the
current path"), the merge step had no guard against agent-invented
fields, and the writer produced wrong domain facts (gamma 2.8 vs 2.2
for BT.470, contradictory bit-packing for Bgra10101010XR).

docs-tool.ps1:
  * Anchor extraction regex to `^\s*To be added\.?\s*$` — only full-text
    placeholder matches trigger extraction
  * Record `_extractedKeys` metadata during extract so the merge phase
    knows which fields were originally placeholders
  * Add merge guard that rejects any agent-added fields not present in
    the original extract (prevents invented documentation)
  * Exclude manifest.json from merge processing
  * Fix PowerShell falsy-empty-array check (`@()` is falsy in boolean
    context; use .PSObject.Properties existence test instead)

SKILL.md:
  * Writer prompt: JSON integrity rules (never add/remove/rename fields),
    trust hierarchy for native type facts (header > reference > knowledge)
  * Factual verifier prompt: standard value verification against
    skia-patterns.md reference file
  * Phase 2: simplified for automated workflow awareness

Validated across 8 workflow runs — 0 tooling regressions post-fix.

Co-authored-by: Matthew Leibowitz <mattleibow@live.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: mattleibow

.agents/skills/api-docs/SKILL.md

@@ -87,6 +87,14 @@ STEPS:
    c. Fill all "To be added." fields following the rules below
    d. Write the completed JSON back to the same path
 
+JSON INTEGRITY RULES (NON-NEGOTIABLE):
+- NEVER add new keys to the JSON that are not already present in the file
+- NEVER add new entries (members) to the JSON that were not extracted
+- Only modify values of EXISTING keys — the extract tool determines scope
+- The "_extractedKeys" array in each entry lists exactly which fields you may fill
+- If a member has only "remarks" extracted, fill ONLY "remarks" — do NOT add
+  "summary", "params", or other fields even if you think they could be improved
+
 WRITING RULES:
 - NEVER invent API calls — verify every method/overload exists by reading C# source
 - NEVER guess numeric values — read MemberValue from JSON, cross-reference source
@@ -95,6 +103,14 @@ WRITING RULES:
 - Use <see langword="null" /> not <see langword="default" /> for nullable params
 - Use <see langword="true" /> and <see langword="false" /> for boolean literals in prose
 
+STANDARD VALUES (CICP, Vulkan, OpenType, etc.):
+- For enum members referencing external standards, you MUST read the C/C++ header
+  in externals/skia/ where the enum is defined and verify numeric values
+- Do NOT rely on your own knowledge of standards for specific numeric values
+  (gamma values, bit depths, transfer function parameters, etc.)
+- If you cannot find the header, state "value from standard" without inventing
+  specific numbers (e.g., say "assumed display gamma" not "assumed gamma 2.2")
+
 REMARKS RULES:
 - Type-level entries (memberType=type) have remarksRequired=true with a CDATA template.
   Complete it fully — never leave [bracketed placeholders]. Include description,
@@ -157,6 +173,18 @@ SPECIFIC CHECKS:
 - "Gets or sets" vs "Gets": check if property has { get; set; } or only { get; }
 - Cross-library: SkiaSharp and HarfBuzzSharp are DIFFERENT libraries with different conventions.
 
+STANDARD VALUE VERIFICATION (CRITICAL):
+- For enum members that cite external standards (CICP, ITU-T, Vulkan, OpenType),
+  you MUST locate and read the C/C++ header where the enum is defined (check
+  externals/skia/include/ or binding/ generated files)
+- Cross-reference the MemberValue in JSON against the enum constant in source
+- If documentation claims a specific numeric property of a standard (e.g., "gamma 2.2",
+  "10-bit precision", "64-bit packed"), verify it is consistent:
+  - Does the bit math add up? (e.g., "64-bit with 10+10+10+10 packed" = only 40 bits → ERROR)
+  - Does the gamma match the correct CICP value number?
+- If you cannot find the header, flag the claim as UNVERIFIED rather than assuming correct
+- Provide file path + line number for every standard value you verify
+
 TRUST HIERARCHY for native type facts (bit layouts, byte orders, macro expansions):
 1. Native C/C++ header in repo (if you can find and read it) — AUTHORITATIVE
 2. skia-patterns.md reference file — PRE-VERIFIED, trust it if header unavailable
@@ -259,7 +287,10 @@ After fixing all CRITICAL review issues, merge the JSON back into XML:
 pwsh .agents/skills/api-docs/scripts/docs-tool.ps1 merge output/docs-work/
 ```
 
-The merge tool has built-in safety checks — it counts `MemberSignature` and `TypeSignature` elements before and after merging each file and aborts with a fatal error if any were lost. It also validates the output XML is well-formed.
+The merge tool has built-in safety checks:
+- **Signature preservation** — counts `MemberSignature` and `TypeSignature` elements before and after merging each file and aborts with a fatal error if any were lost
+- **Extract guard** — rejects any field not listed in `_extractedKeys` (prevents agents from overwriting existing documentation with "improved" versions). Rejected fields emit a warning.
+- **XML validation** — validates the output XML is well-formed after save
 
 Then run formatting to clean up:
 

.agents/skills/api-docs/scripts/docs-tool.ps1

@@ -195,24 +195,37 @@ function Extract-DocsBlock([System.Xml.XmlElement]$docs) {
 
         switch ($child.LocalName) {
             "param" {
-                if ($text -match "To be added") {
+                if ($text -match "^\s*To be added\.?\s*$") {
                     if (-not $fields.ContainsKey("params")) { $fields["params"] = @{} }
                     $fields["params"][$child.GetAttribute("name")] = $text
                 }
             }
             "typeparam" {
-                if ($text -match "To be added") {
+                if ($text -match "^\s*To be added\.?\s*$") {
                     if (-not $fields.ContainsKey("typeparams")) { $fields["typeparams"] = @{} }
                     $fields["typeparams"][$child.GetAttribute("name")] = $text
                 }
             }
             { $_ -in "summary", "returns", "value", "remarks" } {
-                if ($text -match "To be added") {
+                if ($text -match "^\s*To be added\.?\s*$") {
                     $fields[$child.LocalName] = $text
                 }
             }
         }
     }
+
+    # Record which fields were extracted so the merge can reject agent-added fields
+    if ($fields.Count -gt 0) {
+        $allowedKeys = @($fields.Keys | Where-Object { $_ -ne "params" -and $_ -ne "typeparams" })
+        if ($fields.ContainsKey("params")) {
+            $allowedKeys += ($fields["params"].Keys | ForEach-Object { "params.$_" })
+        }
+        if ($fields.ContainsKey("typeparams")) {
+            $allowedKeys += ($fields["typeparams"].Keys | ForEach-Object { "typeparams.$_" })
+        }
+        $fields["_extractedKeys"] = $allowedKeys
+    }
+
     return $fields
 }
 
@@ -225,7 +238,7 @@ function Merge-Docs([string]$inputPath) {
         @(Get-Item $inputPath)
     }
     else {
-        Get-ChildItem -Path $inputPath -Filter "*.json" | Sort-Object Name
+        Get-ChildItem -Path $inputPath -Filter "*.json" | Where-Object { $_.Name -ne "manifest.json" } | Sort-Object Name
     }
 
     $totalUpdates = 0
@@ -263,10 +276,26 @@ function Merge-Docs([string]$inputPath) {
 
             $fields = $entry.fields
 
+            # Build allowed-keys set from extract metadata (guards against agent-added fields)
+            $allowedKeys = $null
+            $hasExtractMeta = $null -ne $fields.PSObject -and $null -ne $fields.PSObject.Properties['_extractedKeys']
+            if ($hasExtractMeta) {
+                $keyArray = @($fields._extractedKeys)
+                $allowedKeys = [System.Collections.Generic.HashSet[string]]::new(
+                    [string[]]$keyArray,
+                    [System.StringComparer]::OrdinalIgnoreCase
+                )
+            }
+
             # Update scalar fields
             foreach ($fieldName in @("summary", "returns", "value", "remarks")) {
                 $content = $fields.$fieldName
-                if ($null -ne $content -and $content -notmatch "To be added") {
+                if ($null -ne $content -and $content -notmatch "^\s*To be added\.?\s*$") {
+                    # Reject fields not in original extract
+                    if ($allowedKeys -and -not $allowedKeys.Contains($fieldName)) {
+                        Write-Warning "Skipping $($docId ?? 'type').$fieldName — not in original extract (agent-added)"
+                        continue
+                    }
                     $elem = $docs.SelectSingleNode($fieldName)
                     if ($elem) {
                         if ($DryRun) {
@@ -286,7 +315,12 @@ function Merge-Docs([string]$inputPath) {
                     $h = @{}; $fields.params.PSObject.Properties | ForEach-Object { $h[$_.Name] = $_.Value }; $h
                 }
                 foreach ($kv in $paramMap.GetEnumerator()) {
-                    if ($null -ne $kv.Value -and $kv.Value -notmatch "To be added") {
+                    if ($null -ne $kv.Value -and $kv.Value -notmatch "^\s*To be added\.?\s*$") {
+                        # Reject params not in original extract
+                        if ($allowedKeys -and -not $allowedKeys.Contains("params.$($kv.Key)")) {
+                            Write-Warning "Skipping $($docId ?? 'type').params.$($kv.Key) — not in original extract (agent-added)"
+                            continue
+                        }
                         $elem = $docs.SelectSingleNode("param[@name='$($kv.Key)']")
                         if ($elem) {
                             if (-not $DryRun) { Set-ElementContent $elem $kv.Value }
@@ -302,7 +336,12 @@ function Merge-Docs([string]$inputPath) {
                     $h = @{}; $fields.typeparams.PSObject.Properties | ForEach-Object { $h[$_.Name] = $_.Value }; $h
                 }
                 foreach ($kv in $tpMap.GetEnumerator()) {
-                    if ($null -ne $kv.Value -and $kv.Value -notmatch "To be added") {
+                    if ($null -ne $kv.Value -and $kv.Value -notmatch "^\s*To be added\.?\s*$") {
+                        # Reject typeparams not in original extract
+                        if ($allowedKeys -and -not $allowedKeys.Contains("typeparams.$($kv.Key)")) {
+                            Write-Warning "Skipping $($docId ?? 'type').typeparams.$($kv.Key) — not in original extract (agent-added)"
+                            continue
+                        }
                         $elem = $docs.SelectSingleNode("typeparam[@name='$($kv.Key)']")
                         if ($elem) {
                             if (-not $DryRun) { Set-ElementContent $elem $kv.Value }