Fill and review API documentation - #156
Closed
github-actions[bot] wants to merge 7 commits into
Closed
Conversation
Rewrite the daily writer to match the upgraded api-docs skill, which edits the mdoc XML directly instead of the extract→write→merge JSON round-trip. Host steps: - Drop the "Extract placeholders and manifest" and "Upload extracted JSON" steps from regenerate-stubs, the "Download pre-extracted JSON" / "Save original JSON" pre-agent steps, and the "Save final JSON" post-step. None are needed without the JSON layer. - Add a non-fatal "Bootstrap SkiaSharp binding for snippet checks" pre-agent step (externals-download + dotnet build binding/SkiaSharp) so the example reviewer can compile-check snippets against a real SkiaSharp.dll. - Keep the SkiaSharpAPI symlink (for the host docs-format-docs post-step) and the formatting post-step. Agent prompt (runtime-imported body): - Replace the JSON phases with the skill's direct-XML add pipeline: resolve-scope new -> writer edits XML -> lint + 3 reviewers + synthesizer -> fix CRITICAL -> structural validate -> commit + PR. - Route each sub-agent through the task tool's per-role model (writer/factual/ examples = opus, quality/synthesizer = sonnet), with an engine.model fallback if the sandbox does not honor per-sub-agent models. - Export DOCS_GIT_ROOT/DOCS_DIR on docs-tool.ps1 calls so scope/validate use the docs repo (primary checkout) for git baselines while source lookups still use the SkiaSharp clone. Recompiled the .lock.yml via `gh aw compile`. Note: depends on the matching api-docs skill update in mono/SkiaSharp (the cloned skiasharp_branch must carry the new skill + docs-tool.ps1 DOCS_GIT_ROOT support). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The api-docs eval per-role bake-off picked gpt-5.5 for reviewer-factual (only candidate that caught the seeded byte-order contradiction; best mean recall + precision). Align the workflow's per-role routing note with the skill's updated review.md table and reviewer-factual Model: header. Body-only change (runtime-imported), so the compiled .lock.yml is unaffected. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…put) Sibling of auto-api-docs-writer that runs the api-docs skill's REVIEW pipeline on a scope (default group:text) instead of filling placeholders. Doubles as the first CI exercise of per-sub-agent model routing: the orchestrator launches each reviewer/writer sub-agent via the task tool with an explicit model (factual -> gpt-5.5 per the eval bake-off) and prints a Routing report so the run is auditable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…g report) Make the daily writer the common path: after the add pass (fill placeholders) it now also runs a REVIEW pass over a baked scope of existing docs (review-scope.txt, default group:text) — lint + 3 reviewers + synthesizer + fix CRITICAL/obsolete via per-role task models — then one validate + PR covers both passes. Because the writer is already registered on the docs default branch, this is dispatchable on a feature branch (skiasharp_branch input) WITHOUT landing a new workflow on main; the review scope is baked (not a dispatch input) so input validation against the default branch still passes. Adds a mandatory Routing report so the run doubles as the first CI exercise of per-sub-agent model routing. Removes the now-redundant standalone auto-api-docs-reviewer workflow. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…tself The first CI pilot run completed green but produced NO PR. Root cause: the orchestrator delegated the terminal fix step to a background "fixer" sub-agent, then ended its turn before that agent (and the PR) completed -- exactly the session-killing pattern the older workflow had guardrails against. Separately, per-sub-agent model routing turned out to be cosmetic in the gh-aw sandbox: the api-proxy token-usage log showed every call was claude-sonnet-4.6 regardless of the requested per-role model. Fixes: - Pin the run model: explicit engine.model claude-sonnet-4.6 (drops the GH_AW_MODEL_AGENT_COPILOT override; orchestrator + every sub-agent run on it). - Drop cosmetic per-role routing and the routing-report requirement; document per-role routing as a local-only skill feature. - Terminal fixes are now the orchestrator's own FOREGROUND work (A4/R4); the only sub-agents launched are the pass-A writer and the reviewers. No terminal background agent. Synthesis is the orchestrator's job (no synthesizer agent). - Restore the tight escape hatch: once reviewers report, timebox fixing to ~10 min, then validate + open the PR. - Add an explicit "No terminal background agent" Critical rule and simplify the completion gate (create_pull_request/noop is the orchestrator's own job). Recompiled auto-api-docs-writer.lock.yml. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
PoliCheck Scan ReportThe following report lists PoliCheck issues in PR files. Before you merge the PR, you must fix all severity-1 and severity-2 issues. The AI Review Details column lists suggestions for either removing or replacing the terms. If you find a false positive result, mention it in a PR comment and include this text: #policheck-false-positive. This feedback helps reduce false positives in future scans. ✅ No issues foundMore information about PoliCheckInformation: PoliCheck | Severity Guidance | Term |
Contributor
|
Learn Build status updates of commit c9bd703: ✅ Validation status: passed
For more details, please refer to the build report. |
Validation run 28203165066 produced a real PR (the orchestration fix worked), but the agent rationalized out of step C: it saw it was already on the dispatch ref (a feature branch ahead of main), decided to "commit here and create the PR from this branch," and skipped `git checkout -b automation/write-api-docs`. Because safe-outputs preserves the committed branch and force-resets it (recreate_ref:true), this force-overwrote the workflow's own source branch. Fix: make step C unconditional — `git checkout -B automation/write-api-docs` before committing, even when already on a feature branch ahead of main, staging only SkiaSharpAPI/. Add a matching Critical rule explaining that committing on the dispatch ref destroys it under recreate_ref. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… runs Root cause of the run-156 branch clobber: gh-aw's checkout step makes the DISPATCH REF the agent's working branch (git checkout -B <dispatch-ref>). When dispatched from a feature branch, the agent starts on that branch; it then committed its doc work there instead of switching to automation/write-api-docs, and safe-outputs (preserve_branch_name + recreate_ref) adopted that branch name as the PR head and force-recreated it — erasing the workflow source commits. Relying on the agent to switch branches is fragile (it rationalized staying put because the branch was "already ahead by 1 commit"). Add a host pre-agent step that renames the working branch to automation/write-api-docs before the agent starts, so every commit and the PR head land on a throwaway branch regardless of which ref triggered the run. The prompt rule remains as defense-in-depth. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
mattleibow
force-pushed
the
dev/p1b-direct-xml-writer
branch
from
June 26, 2026 07:17
c9bd703 to
10fe960
Compare
Collaborator
|
Closing: this PR was a validation artifact from run 28203165066 that accidentally landed on the workflow source branch |
Contributor
PoliCheck Scan ReportThe following report lists PoliCheck issues in PR files. Before you merge the PR, you must fix all severity-1 and severity-2 issues. The AI Review Details column lists suggestions for either removing or replacing the terms. If you find a false positive result, mention it in a PR comment and include this text: #policheck-false-positive. This feedback helps reduce false positives in future scans. ✅ No issues foundMore information about PoliCheckInformation: PoliCheck | Severity Guidance | Term |
Contributor
|
Learn Build status updates of commit 10fe960: ✅ Validation status: passed
For more details, please refer to the build report. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Pass A — Fill new placeholders (3 files)
The mdoc pre-step regenerated stubs for three new/updated types. The writer agent filled all
To be added.placeholders:HarfBuzzSharp/HBColor.xml<remarks>tagsSkiaSharp/SKImageFilter.xmlCreateEmptysummary and returnsSkiaSharp/SKPathMeasure.xmlNextContour, midpoint-sampling example); normalized empty member remarksPass R — Review existing docs (scope:
group:text, 16 files)The three reviewer agents audited the full
group:textscope (SKCanvas, SKFont, SKFontArguments, SKFontManager, SKFontMetrics, SKFontStyle, SKFontStyleSet, SKFontStyleSlant, SKFontStyleWeight, SKFontStyleWidth, SKPaint, SKTextAlign, SKTextBlob, SKTextBlobBuilder, SKTextEncoding, SKTypeface).Fixes applied to 4 files in this PR:
SKCanvas.xmlSKFontoverloads; fixed wrong backend description ("XPS backend" → "Picture backend"); fixedtransfered→transferred(×2)SKPaint.xmlSKPaint.TextSize/TextScaleX/DrawText(4-arg)toSKFontoverloadsSKTypeface.xmlSKPaint.TextSize/TextSkewX/TextScaleX/FakeBoldTextfrom type-level remarks; migratedFromFileexample toSKFont; fixedguarentee→guaranteeSKFont.xmlMeasureTextoverloads with wrong summary/returns copy-pasted fromGetGlyphWidths; now correctly describe the scalar advance-width return valueFindings summary
Linter (pre-fix): 6 CRITICAL
obsolete-in-example— reduced to 3 after fixes. The 3 remaining are linter false positives: the linter strips method signatures, so the bare nameDrawTextmatches the modern non-obsolete 6-argument overload. The canonical replacement inobsolete-api-map.mditself usescanvas.DrawText(...), confirming this is a linter limitation.Factual reviewer: 6 CRITICAL · 6 IMPORTANT · 1 MINOR across 7 files
Quality reviewer: 14 CRITICAL · ~82 IMPORTANT across 16 files
FINDING blocks
What was fixed vs deferred
Fixed (CRITICAL): 9 issues — 3 obsolete example migrations, 3 misspellings, 1 wrong section label, 1 set of 10 MeasureText wrong descriptions, obsolete xrefs removal.
Deferred (IMPORTANT): ~80 quality/factual IMPORTANT findings across SKFontManager, SKFontStyle, SKFontStyleSet, SKFontStyleWeight, SKTypeface, SKFont, SKPaint (constructor wording,
<returns>noun-phrase,<remarks></remarks>non-self-closing tags,true toin<value>, etc.). A follow-up PR can address them.Validation