Skip to content

docs(reference): milestone 8 phase 1 - API reference toolchain wired into CI - #47

Merged
ncipollina merged 7 commits into
mainfrom
docs/milestone-8-phase-1-api-reference
Aug 5, 2026
Merged

docs(reference): milestone 8 phase 1 - API reference toolchain wired into CI#47
ncipollina merged 7 commits into
mainfrom
docs/milestone-8-phase-1-api-reference

Conversation

@ncipollina

Copy link
Copy Markdown
Contributor

Summary

Executes ADR-0032 / PLAN-0008 Phase 1: evaluates and wires the docs/reference/api/ generation toolchain.

  • Bake-off: DefaultDocumentation (1.2.5) vs xmldocmd (2.9.0), against all four publishable packages' real net10.0 assemblies. xmldocmd eliminated outright — its host build only targets net6.0/net7.0 and throws FileNotFoundException loading System.Runtime for a net10.0 target assembly. DefaultDocumentation passed every ADR-0032 criterion (generics, overloads, inheritance, exceptions, nullable signatures, deterministic output) and ships an active net10.0 host build.
  • Two real defects found and fixed during wiring (not left as gaps):
    1. Cross-package <see cref> (e.g. Compono.XunitV3Compono.Composer) fell back to a fabricated learn.microsoft.com URL. Fixed by generating Compono core first with --LinksOutputFilePath, feeding it to the three integration packages' --ExternLinksFilePaths.
    2. Parameterless-constructor pages are named with the raw CLR #ctor metadata name — # is the URL fragment delimiter, breaking MkDocs routing (caught by a real mkdocs build, not just eyeballing Markdown). Post-processed to .ctor in the generation script.
  • New .github/scripts/generate-api-reference.sh + .github/workflows/api-reference.yaml drift-detection CI gate (regeneration must produce no uncommitted diff). Missing-XML-doc-comment enforcement already exists via Phase 0's CS1591 gate — DefaultDocumentation has no independent detection to add on top of it.
  • docs/reference/index.md and mkdocs.yml nav updated to surface the new API Reference section.
  • PLAN-0008 Phase 1 marked Done, with bake-off findings recorded in the plan's Notes.

Test plan

  • dotnet build/dotnet test — full solution, 854/854 tests pass (no C# source touched by this phase).
  • Regenerated docs/reference/api/ from scratch twice — byte-identical output (deterministic).
  • Real uv run mkdocs build --clean against the full site — builds clean; only pre-existing, unrelated warnings (broken .claude/skills/.agents/skills links from earlier ADR pages).
  • Verified the drift-detection gate locally: rebuild + regenerate produces no diff against the committed content.

🤖 Generated with Claude Code

…into CI

DefaultDocumentation wins the ADR-0032 bake-off (xmldocmd's host build
doesn't support net10.0 assemblies, hard-fails on load). Wires it into
a new generate-api-reference.sh + api-reference.yaml drift-detection gate,
core-package-first so integration packages' cross-package <see cref>s
resolve locally instead of DotnetApiFactory's fabricated
learn.microsoft.com fallback, plus a post-processing fix for '#ctor'
filenames (the raw CLR ctor metadata name collides with the URL fragment
delimiter). Verified with a real mkdocs build, not just the generated
Markdown.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@github-actions github-actions Bot added the type: docs Documentation change label Aug 4, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 37ba63960b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .github/scripts/generate-api-reference.sh
Comment thread docs/reference/api/Compono/Compono.CompositionBuilder.md Outdated
…s and stale ctor anchors

Codex review found the #ctor filename rename left same-page anchor names
carrying a stale "ctor.md#" prefix that no longer matches the (already-
fixed) href fragment, and that DotnetApiFactory's BCL-fallback behavior
produces dead learn.microsoft.com links whenever a public member's XML
docs <see cref> an internal Compono type (38 occurrences, not the single
instance originally noted). Both fixed in the generation script's
post-processing pass; regenerated and reverified with a real mkdocs build.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@ncipollina

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ce4849f574

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread docs/reference/api/Compono/Compono.CompositionException..ctor.md Outdated
…eview)

A member's own <paramref>/<typeparamref> self-reference always links back
to its containing type's page, but the anchor lives wherever
OverloadsGenerator actually placed that overload once a member has more
than one. Confirmed general (92 mismatched fragment links across all four
packages, not just the flagged constructor case) rather than patched
narrowly: builds an anchor-id -> actual-file map per package directory and
rewrites any same-package link pointing at the wrong file. 0 mismatches
after the fix; reverified with a real mkdocs build and full test suite.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@ncipollina

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: eddf5d3c38

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .github/scripts/generate-api-reference.sh Outdated
…gus-link fix

Two changes, both from PR #47 review/follow-up:

1. Delete the standalone api-reference.yaml workflow and move its
   regenerate-and-diff-check steps into docs.yml's own build job,
   sequentially before mkdocs build. The two workflows previously had no
   ordering relationship (no needs/workflow_run), so a failing drift
   check could never actually stop a bad deploy - per user direction,
   not a silent choice (the local-generation-only alternative would
   reverse ADR-0032's explicit CI-catches-drift requirement without an
   amendment).

2. Generalize the bogus-fallback-link fix from a compono.* blocklist to
   a system.*/microsoft.* allowlist, catching third-party dependency
   types (Bogus.Faker, NSubstitute.Substitute.For, Xunit.v3.IDataAttribute)
   that DotnetApiFactory fabricates the same dead-link fallback for. Also
   fixes a link-text parsing bug in that same regex: [^\]]+ stops at the
   first literal ']', silently failing to match a signature whose display
   text contains escaped array-parameter brackets.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@ncipollina

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

run: uv run mkdocs build --clean

P2 Badge Make MkDocs warnings fail the docs job

ADR-0032 requires CI to fail on broken internal links, but this workflow still runs mkdocs build --clean without --strict and mkdocs.yml does not enable strict validation; the MkDocs CLI docs state that strict mode aborts the build on warnings. When a generated or handwritten API page introduces a broken internal link, MkDocs will warn but this job will continue to upload/deploy the site, so the new gate does not actually enforce the link-failure criterion.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

#47 review)

docs.yml ran mkdocs build with no --strict, so ADR-0032's own "CI fails on
broken internal links" requirement was never actually enforced - a warning
never fails a plain mkdocs build. Enabling --strict surfaced exactly the 4
pre-existing broken links already visible as noise throughout this PR's
verification: ADR-0014/0015/0016/0022 each link to
.claude/skills/engineering-workflow/references/design-decisions.md, which
sits outside docs_dir entirely and can never resolve in the built site
regardless of relative-path depth. Converted those 4 references from a
dead hyperlink to plain, unlinked text - link syntax only, the ADRs'
Decision/Rationale/Consequences prose is untouched, per this repo's own
ADR-immutability rule.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@ncipollina

Copy link
Copy Markdown
Contributor Author

Re: #47 (review) — Fixed in 2ff6748. You're right that mkdocs build --clean with no --strict never actually enforced ADR-0032's "CI fails on broken internal links" requirement — a warning doesn't fail a plain mkdocs build.

Enabling --strict surfaced exactly 4 pre-existing broken links (ADR-0014/0015/0016/0022, each linking to .claude/skills/engineering-workflow/references/design-decisions.md) — not a path-depth bug, that target sits outside docs_dir entirely and can never resolve in the built site regardless of relative-path correction. Fixed both: docs.yml now runs mkdocs build --clean --strict, and those 4 ADR cross-references were converted from a dead hyperlink to plain, unlinked text — link syntax only, the ADRs' Decision/Rationale/Consequences prose is untouched.

Verified: mkdocs build --clean --strict now exits 0 (previously aborted with exactly those 4 warnings), full test suite still green (854/854).

@ncipollina

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2ff6748fbf

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .github/workflows/docs.yml
… review)

Directory.Build.targets is auto-imported into every project the same way
Directory.Build.props is (just after the project body instead of before),
including the four publishable packages this job's drift-check step
builds - but only Directory.Build.props was in the trigger paths lists.
A PR touching only Directory.Build.targets would silently skip both the
drift check and the site rebuild.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@ncipollina

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 75982d4612

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread docs/documentation-architecture.md
Comment thread docs/plans/0008-milestone-8-public-preview.md Outdated
Two spots hadn't been updated when reference/api went from skeleton to
real, generated content: documentation-architecture.md's top-level intro
still said reference/api didn't exist yet pending a toolchain decision,
contradicting its own Section 11 status a few pages down; PLAN-0008 Phase
1's mkdocs verification note still recorded the earlier non-strict run
with four warnings, contradicting the --strict fix recorded later in the
same Notes section.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@ncipollina
ncipollina merged commit 7b0001d into main Aug 5, 2026
7 checks passed
@ncipollina
ncipollina deleted the docs/milestone-8-phase-1-api-reference branch August 5, 2026 02:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: docs Documentation change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant