Skip to content

refactor(scripts): unify asset-doc section contract - #2678

Open
Jamie Kim (jkim323) wants to merge 12 commits into
mainfrom
refactor/2468-unify-asset-doc-section-contract
Open

refactor(scripts): unify asset-doc section contract#2678
Jamie Kim (jkim323) wants to merge 12 commits into
mainfrom
refactor/2468-unify-asset-doc-section-contract

Conversation

@jkim323

Copy link
Copy Markdown
Collaborator

Pull Request

Description

This PR centralized asset-documentation section identity, order, content source, and per-kind status rules in one shared contract.

  • Added an ordered section contract and resolver that produced Required, Optional, or NotApplicable for each asset kind and interactivity state.
  • Updated generation to render new scaffolds from the contract while preserving existing authored tails and contract-derived orphan safety checks.
  • Reworked the scaffold template into named human-authored body regions so output headings came only from the shared contract.
  • Updated structure validation to derive required headings from the same contract instead of maintaining a separate literal list.
  • Expanded focused Pester coverage for contract semantics, heading order, malformed template markers, authored-tail preservation, orphan behavior, and validator derivation.

Related Issue(s)

Closes #2468

Type of Change

Select all that apply:

Code & Documentation:

  • Bug fix (non-breaking change fixing an issue)
  • New feature (non-breaking change adding functionality)
  • Breaking change (fix or feature causing existing functionality to change)
  • Documentation update

Infrastructure & Configuration:

  • GitHub Actions workflow
  • Linting configuration (markdown, PowerShell, etc.)
  • Security configuration
  • DevContainer configuration
  • Dependency update

AI Artifacts:

  • Reviewed contribution with hve-builder and addressed all actionable findings
  • Copilot instructions (.github/instructions/*.instructions.md)
  • Copilot prompt (.github/prompts/*.prompt.md)
  • Copilot agent (.github/agents/*.agent.md)
  • Copilot skill (.github/skills/*/SKILL.md)
  • Copilot hook (.github/hooks/*/*.json)
  • Eval spec added/updated for changed AI artifacts (evals/)

Note for AI Artifact Contributors:

  • Agents: Research, indexing/referencing other project (using standard VS Code GitHub Copilot/MCP tools), planning, and general implementation agents likely already exist. Review .github/agents/ before creating new ones.
  • Skills: Must include both bash and PowerShell scripts. See Skills.
  • Model Versions: Contributions MUST target models listed in the model catalog (scripts/linting/model-catalog.json) whose provider appears in providerAllowlist and whose status is ga or preview. Run npm run lint:models to validate references.
  • See Agents Not Accepted and Model Version Requirements.

Other:

  • Script/automation (.ps1, .sh, .py)
  • Other (please describe):

Sample Prompts (for AI Artifact Contributions)

User Request:

Execution Flow:

Output Artifacts:

Success Indicators:

For detailed contribution requirements, see:

Testing

Focused implementation validation completed before PR drafting:

  • npm run test:ps -- -TestPath scripts/tests/docs/DocsHelpers.Tests.ps1 passed 74 tests.
  • npm run test:ps -- -TestPath scripts/tests/docs/Generate-AssetDocs.Tests.ps1 passed 28 tests.
  • npm run test:ps -- -TestPath scripts/tests/linting/Validate-AssetDocs.Tests.ps1 passed 44 tests.
  • npm run lint:ps completed with zero errors; its one warning was outside the changed files.
  • npm run docs:generate:check reported 263 unchanged pages and zero drift.
  • npm run lint:asset-docs completed with zero errors; existing authored-stub warnings remained.
  • npm run lint:frontmatter, template Markdown lint, editor diagnostics, and git diff --check passed.

PR-readiness local checks are recorded in the checklist below. Manual testing was not performed.

Security analysis found no sensitive data, dependency changes, permission changes, security-script changes, or unintended files in the branch diff.

Checklist

Required Checks

  • Documentation is updated (if applicable)
  • Files follow existing naming conventions
  • Changes are backwards compatible (if applicable)
  • Tests added for new functionality (if applicable)

AI Artifact Contributions

  • Used hve-builder review mode to review contribution
  • Addressed all actionable findings from the hve-builder review
  • Verified contribution follows common standards and type-specific requirements

Required Local Checks

The following local-safe validation commands must pass before merging:

  • Local validation aggregate: npm run validate:local
  • Documentation validation (if docs changed): npm run validate:docs
  • Spell checking: npm run spell-check
  • Link validation: npm run lint:md-links

Security Considerations

  • This PR does not contain any sensitive or NDA information
  • Any new dependencies have been reviewed for security issues (N/A - no dependency changes)
  • Security-related scripts follow the principle of least privilege (N/A - no security scripts changed)

@jkim323
Jamie Kim (jkim323) requested a review from a team as a code owner August 12, 2026 02:59
@github-actions

Copy link
Copy Markdown
Contributor

Eval Execution

⚠️ No eval summary was produced.

@codecov-commenter

Codecov Comments Bot (codecov-commenter) commented Aug 12, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.22034% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.41%. Comparing base (5b2119e) to head (922e1ab).

Files with missing lines Patch % Lines
scripts/docs/Generate-AssetDocs.ps1 88.46% 6 Missing ⚠️
scripts/docs/Modules/DocsHelpers.psm1 96.22% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2678      +/-   ##
==========================================
- Coverage   83.17%   81.41%   -1.76%     
==========================================
  Files         180      150      -30     
  Lines       32201    21815   -10386     
  Branches       25        0      -25     
==========================================
- Hits        26782    17760    -9022     
+ Misses       5416     4055    -1361     
+ Partials        3        0       -3     
Flag Coverage Δ
docusaurus ?
pester 83.39% <93.22%> (-0.83%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
scripts/linting/Validate-AssetDocs.ps1 94.94% <100.00%> (+0.17%) ⬆️
scripts/docs/Modules/DocsHelpers.psm1 96.86% <96.22%> (-0.17%) ⬇️
scripts/docs/Generate-AssetDocs.ps1 87.36% <88.46%> (-0.46%) ⬇️

... and 50 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jkim323 Jamie Kim (jkim323) self-assigned this Aug 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The shared contract consolidation looks sound, and all focused tests pass. One contract value appears inconsistent with the linked issues: instruction “Example usage” is currently required rather than optional. The heading-order validation point is non-blocking but worth clarifying.

Comment thread scripts/docs/Modules/DocsHelpers.psm1 Outdated
Comment thread scripts/linting/Validate-AssetDocs.ps1 Outdated
@jkim323

Copy link
Copy Markdown
Collaborator Author

Thanks Katrien De Graeve (@katriendg) for the review! Both comments are addressed:

  1. You were right that the value contradicted. example-usage now resolves to Optional for instruction, with an inline note on why: an always-on instruction has no invocation to demonstrate. Generation still emits the template region for authors who do have one. Tests now assert the real contract entry across all four kinds, plus a validator test for an instruction page that omits the section.
  2. Enforcing it rather than documenting it as generation-only. The generator preserves the authored tail byte-for-byte, so nothing self-heals — a page reordered once stays reordered. Scoping order to generation would make it authoritative for one moment in each page's life, reintroducing the divergence refactor: unify asset-doc section contract into a single ordered section list #2468 set out to remove. Test-AssetDocStructure now reports a Structure error when a present heading precedes its predecessor. Optional sections are order-checked only when present; NotApplicable skipped. The check is deliberately partial — non-contract ## subsections are ignored, since enforcing a closed heading set would be a real behavior change outside this PR's scope.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the additional changes, all looking good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor: unify asset-doc section contract into a single ordered section list

3 participants