Skip to content

feat: resolve see/seealso cref references to real cross-reference links - #55

Merged
Malcolmnixon merged 13 commits into
mainfrom
feature/cref-cross-reference-linking
Oct 8, 2026
Merged

Malcolmnixon merged 13 commits into
mainfrom
feature/cref-cross-reference-linking

Conversation

@Malcolmnixon

@Malcolmnixon Malcolmnixon commented Oct 8, 2026 •

Copy link
Copy Markdown
Member

Implements the follow-up flagged in #54: actual cross-reference links for <see cref>/<seealso cref> references in XML-doc prose, instead of always falling back to a code-span-wrapped plain-text rendering.

Change

  • New CrefTargetResolver (src/ApiMark.DotNet/CrefTargetResolver.cs): a reverse index from the exact XML-doc-ID strings DotNetEmitter.BuildTypeId/BuildMemberId already produce (i.e. the same format that appears in a cref="..." attribute) back to the TypeDefinition/IMemberDefinition they identify, built once per assembly.
  • New CrefLinkContext (src/ApiMark.DotNet/CrefLinkContext.cs): an optional, model-agnostic bundle (resolver, member-page index, visibility delegates, current folder) that XmlDocReader can use to attempt link resolution without taking a hard dependency on DotNetEmitter/DotNetAstModel.
  • TypeLinkResolver gains LinkifyResolvedType/LinkifyResolvedMember, reusing the existing GetTypePageKey/ComputeRelativePath link-building logic already used for parameter/member type links.
  • XmlDocReader's public API (GetSummary, GetSummaryMarkdown, GetRemarks, GetParams, GetReturns, GetExampleParts) gains a new optional, trailing CrefLinkContext? linkContext = null parameter, threaded down through the internal rendering chain to the single <see>/<seealso> formatting call site. When a cref resolves to an intra-assembly type or member that will actually be emitted, the existing code-span text (from Render type-only cref references as inline code, matching member crefs #54) is now wrapped in a real relative Markdown link to that target's page. (GetExample — which returns raw, untransformed element text rather than routing through this rendering chain — and GetExceptions/GetExceptionDetails are unchanged — out of scope. Use GetExampleParts for structured, link-aware example rendering.)
  • A <see cref>/<seealso cref> with an explicit display label (e.g. <see cref="Foo">custom label</see>) now also resolves and links its cref, wrapping the explicit label itself in the link rather than leaving it unlinked merely because it carries a custom label — previously this case returned the label before the cref was ever consulted.
  • Extracted GetCandidateMembers from DotNetEmitter.GetVisibleMembers as a standalone, behavior-preserving refactor so the gradual-disclosure emitter's existing overload/operator/case-collision member-page grouping decisions can be reused to build a MemberPageIndex (member XML-doc-ID -> relative page path) ahead of time, without duplicating that grouping logic.
  • DotNetGenerator/DotNetAstModel build the CrefTargetResolver and MemberPageIndex once per generation run and wire them into the gradual-disclosure emitter's CrefLinkContext.
  • Fallback to the existing Render type-only cref references as inline code, matching member crefs #54 behavior (code-span text, no link) is preserved exactly for: external/framework types, members filtered out by the configured ApiVisibility, malformed/unresolvable crefs, and constructor crefs (never specially formatted, per existing precedent).

Scope

  • Gradual-disclosure emitter only. Single-file emitter mode (DotNetEmitterSingleFile.cs) is untouched — it already deliberately disables all intra-document linking ("same-name anchors can collide across types in single file"), so cref linking there would reintroduce exactly the risk that design decision avoids. Single-file crefs keep rendering as code-span-only text, unconditionally, same as before this PR.
  • Members that share a generated page (method overload groups, operator pages, case-collision pages) link to that shared page, not to a specific in-page heading anchor — replicating heading-slug anchoring has no existing precedent in this codebase and is left as a follow-up if wanted.

Tests

  • New CrefTargetResolverTests.cs, extended TypeLinkResolverTests.cs/XmlDocReaderTests.cs covering every fallback path (external type, filtered visibility, malformed cref, constructor cref) and both success paths (type cref, member cref, including seealso).
  • New fixture type CrefLinkingClass.cs and extended DotNetEmitterGradualDisclosureTests.cs proving an in-assembly cref renders as a real relative Markdown link in generated output.
  • Dedicated regression test in DotNetEmitterSingleFileTests.cs proving single-file output is unchanged (still code-span-only).
  • 436/436 tests passing on net8.0/net9.0/net10.0; full solution build and fix.ps1/lint.ps1 clean. (Pre-existing ApiMark.Cpp.Tests/ApiMark.MSBuild.PackageTests failures are environmental — missing clang on this machine — and untouched by this diff.)

Docs

Added/updated design, verification, and reqstream docs for the new CrefTargetResolver unit and the extended XmlDocReader/TypeLinkResolver/DotNetAstModel/DotNetGenerator/DotNetEmitterGradualDisclosure units, plus a new .reviewmark.yaml entry, following the same pattern established by #54.

Base branch note

This branches off fix/wrap-type-cref-code-span (#54), since it directly extends that PR's ShouldWrapInCodeSpan rendering and is still open. Will rebase onto main once #54 merges.

Base automatically changed from fix/wrap-type-cref-code-span to main October 8, 2026 01:03
Malcolm Nixon added 8 commits October 7, 2026 21:04
Behavior-preserving refactor: splits the structural member-eligibility
filtering (special-name accessor exclusion, compiler-generated exclusion,
value__ field exclusion) out of GetVisibleMembers into a new static
GetCandidateMembers helper, with GetVisibleMembers now simply applying
ShouldIncludeMember on top of it.

This is preparation for the upcoming BuildMemberPageIndex function (full
cref cross-reference linking work), which needs the same structural
member-eligibility rules under a caller-supplied visibility predicate
instead of the instance-bound ShouldIncludeMember, without duplicating
the filtering rules in a second, drift-prone copy.

No behavior change: full existing ApiMark.DotNet.Tests suite (404 tests)
passes unchanged.
- Add CrefTargetResolver: visibility-agnostic index resolving a cref
  XML-doc-ID string to the TypeDefinition/IMemberDefinition that
  defines it within a single assembly, built once from
  AssemblyDefinition.MainModule.GetTypes(). Used later to determine
  whether a <see cref>/<seealso cref> target exists in-assembly at
  all (external types/members simply are not found).
- Add TypeLinkResolver.LinkifyResolvedType/LinkifyResolvedMember,
  reusing the existing GetTypePageKey/ComputeRelativePath helpers so
  resolved cref targets can be rendered as real relative Markdown
  links consistent with existing intra-assembly type links.
- Add DotNetEmitterGradualDisclosure.BuildMemberPageIndex: a pure
  function that reproduces the existing member grouping/page-naming
  decisions (overload groups, operator page, case-collision groups,
  single-member pages) for every emitted member of a type, returning
  {member XML-doc-ID -> relative page path}. This is an independently
  written duplicate of the grouping logic in ProcessTypeMembers
  rather than a true single-source refactor (scope reduction, see
  planning-full-cref-linking-a1f3e9.md); mitigated by a dedicated
  cross-check test to be added against real written page paths.
- Wire CrefTargetResolver and the merged MemberPageIndex/
  EmittedTypeIds into DotNetAstModel/DotNetGenerator.Parse:
  CrefTargetResolver is built eagerly in the DotNetAstModel
  constructor (needs only the assembly); MemberPageIndex and
  EmittedTypeIds are computed after DotNetEmitter construction (they
  need its visibility predicates) via new BuildCrefLinkIndices/
  CollectTypeCrefLinkIndex helpers and populated through a new
  DotNetAstModel.SetMemberPageIndex setter, preserving the existing
  one-pass Parse contract.

No behavior change: these additions are not yet consulted by any
rendering code path.
Add a new internal CrefLinkContext record bundling everything needed
to resolve and linkify a <see cref>/<seealso cref> target: the
CrefTargetResolver and TypeLinkResolver for the current assembly, the
merged member page index, delegates to test whether a resolved type/
member is actually emitted, and the current page's folder (needed to
compute a relative path to the link target).

Thread a new optional, trailing, nullable linkContext parameter
through the entire XmlDocReader rendering chain:
GetDocumentationText/GetSingleLineDocumentationText -> AppendNodeText
(both overloads) -> AppendElementText -> list rendering helpers
(AppendListText/AppendInlineMarkerList/AppendMarkerList/
AppendTableList/RenderTermDescription/RenderInlineElement) ->
GetInlineReferenceText.

Because XmlDocReader is a public type, its public GetSummary/
GetSummaryMarkdown/GetRemarks/GetParams/GetReturns/GetExampleParts
methods cannot directly expose an internal CrefLinkContext parameter
(CS0051). Each is split into its existing public single-argument
overload (behavior unchanged, delegates to the new overload with
linkContext: null) plus a new internal two-argument overload that
accepts the context. GetExample is deliberately left without an
internal overload: it returns the raw element value without going
through AppendNodeText/AppendElementText, so threading a link context
through it would be a no-op; this is documented in its XML remarks.
GetExceptions/GetExceptionDetails are explicitly out of scope per the
approved plan and are untouched.

In GetInlineReferenceText, a new TryLinkifyCref helper implements the
fallback semantics: when linkContext is null, or the cref cannot be
resolved via CrefTargetResolver.TryResolveType/TryResolveMember, or
the resolved symbol's IsTypeEmitted/IsMemberEmitted delegate returns
false, the existing plain-text/code-span behavior is preserved
unchanged. Otherwise the resolved target is linkified via
TypeLinkResolver.LinkifyResolvedType/LinkifyResolvedMember using the
existing display text, and the resulting Markdown link is wrapped in
the same code span the plain text would have been wrapped in.

All ~100 existing call sites across both emitters and
XmlDocReaderTests.cs continue to call the public overloads
unchanged (passing no link context), so existing behavior for both
single-file and gradual-disclosure modes is unaffected by this
commit; CrefLinkContext is not yet constructed or passed by either
emitter. Also disambiguates now-ambiguous <see cref> references in
XmlDocReader.cs's own doc comments and in
DocumentationCoverageChecker.cs after the overload split.

Full existing ApiMark.DotNet.Tests suite (404 tests) passes
unchanged.
- Add CrefTargetResolverTests.cs: covers resolving top-level and
  nested types, methods and fields, resolving a private member
  (confirms no visibility filtering), and failing to resolve unknown
  identifiers, external-assembly types, and malformed/unresolvable
  cref strings.
- Extend TypeLinkResolverTests.cs with cases for
  LinkifyResolvedType/LinkifyResolvedMember: link generation enabled
  vs. disabled, and a member found vs. not found in the supplied
  member page index.
- Extend XmlDocReaderTests.cs with a dedicated CrefLinkContext region
  covering every fallback/success path from the approved plan,
  exercised directly against the fixture assembly (no full emitter
  run required): null link context (baseline unchanged), a resolved
  and emitted type, a resolved and emitted member, a resolved member
  whose IsMemberEmitted delegate returns false (filtered out), an
  external framework type, a malformed cref, a constructor cref
  (never linkified, matching existing precedent), and a seealso
  cref sharing the same resolution path as see.

All 426 tests pass (404 pre-existing + 22 new) on net8.0.
Thread CrefLinkContext into every call site in
DotNetEmitterGradualDisclosure.cs that reads XmlDocReader Get*
documentation text (GetSummary/GetSummaryMarkdown/GetRemarks/
GetReturns/GetParams/GetExampleParts), so <see cref>/<seealso cref>
references now render as real relative Markdown links when their
target is an intra-assembly type or member that will actually be
emitted, and keep today's code-span-only fallback otherwise.

- DotNetAstModel.cs: add a required CrefLinkContext LinkContext
  parameter to TypePageWriteContext and MethodDocContext so page
  writers carry a folder-scoped link context without needing access
  to the emitter instance.
- DotNetEmitterGradualDisclosure.cs: add BuildCrefLinkContext(folder)
  to construct a CrefLinkContext bound to the model's CrefTargets,
  Resolver, and MemberPageIndex plus IsTypeEmitted/IsMemberEmitted
  delegates; wire it through WriteNamespacePage, WriteTypeHeaderSections,
  WriteNestedTypesSection, ProcessSingleMember, ProcessOverloadGroup,
  ProcessCollisionMember, WriteMemberPage, WriteMethodOverloadPage,
  WriteTypeOperatorsPage, WriteCombinedMemberPage,
  WriteNonMethodMemberContent, and WriteMethodDocumentation. Page
  writers at a deeper folder than their caller derive a re-scoped
  context via `linkContext with { CurrentFolder = ... }` rather than
  needing the emitter instance passed in.
- DotNetEmitterSingleFile.cs is intentionally untouched: single-file
  mode never supplies a CrefLinkContext, so it keeps rendering crefs
  exactly as it does today (code-span-only, never linked).

Tests:
- test/ApiMark.DotNet.Fixtures/CrefLinkingClass.cs: new fixture
  exercising every cref resolution outcome — a <see cref> to a
  visible in-assembly type, a <see cref> to a visible in-assembly
  member, a <see cref> to a member filtered out by visibility, a
  <see cref> to an external framework type, a malformed/unresolvable
  cref string, and a <seealso cref> to a visible type (nested in
  <remarks>, since top-level <seealso> siblings of <summary> are not
  read by this pipeline).
- DotNetEmitterGradualDisclosureTests.cs: new end-to-end tests proving
  each of the above renders correctly in generated gradual-disclosure
  output, plus a test asserting every DotNetAstModel.MemberPageIndex
  entry corresponds to an actually-written page (mitigating the
  BuildMemberPageIndex duplicated-logic scope reduction).
- DotNetEmitterSingleFileTests.cs: new regression test proving the
  same cref still renders as plain code-span text with no link in
  single-file mode.
- DotNetGeneratorTests.cs: updated
  DotNetGenerator_Generate_InlineReferences_PreserveSymbolNamesInMarkdown
  expectations, since the SampleStatus/SampleStatus.Active/
  SampleStatus.Pending crefs it asserts on are now resolvable
  intra-assembly symbols and correctly render as real links under the
  new feature.
Cover the new DotNetAstModel.CrefTargets, MemberPageIndex, and
EmittedTypeIds members added for cross-reference linking:
- CrefTargets is constructed eagerly during DotNetGenerator.Parse and
  can resolve a known fixture type by its XML-doc cref ID.
- MemberPageIndex and EmittedTypeIds are populated by the
  gradual-disclosure emitter's post-construction setup step and
  contain entries for known visible fixture symbols.
Update design, verification, and reqstream docs for DotNetAstModel,
DotNetGenerator, DotNetEmitterGradualDisclosure, DotNetEmitterSingleFile,
TypeLinkResolver, and XmlDocReader to cover the new cref cross-reference
linking feature. Add a new CrefTargetResolver unit's design, verification,
and reqstream docs. Register the new unit and requirement IDs in
docs/reqstream/api-mark-dot-net.yaml and requirements.yaml. Add a new
ApiMark-DotNet-CrefTargetResolver entry to .reviewmark.yaml and extend the
existing XmlDocReader entry to cover the new CrefLinkContext.cs file.
Update the subsystem-level docs/design/api-mark-dot-net.md with the new
unit, its dependency edges, and a cref-linking data-flow step.
The verification checklist still claimed type-only crefs were unwrapped, and two design-doc passages still pointed at the old Member-cref inline code span wrapping section name or excluded type-only crefs from the wrap condition. These were flagged in PR #54 review comments but missed by that PR's fix commit.
@Malcolmnixon
Malcolmnixon force-pushed the feature/cref-cross-reference-linking branch from 4115ed6 to b47b81f Compare October 8, 2026 01:07
Copilot AI lite review requested due to automatic review settings October 8, 2026 01:07

Copilot AI left a comment

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.

🟡 Changes recommended

Unresolved issues remain in cref link formatting, page-index consistency, namespace documentation coverage, API exposure, and architecture metadata.

5 open findings
What changed in this PR

Adds intra-assembly cref/seealso link resolution for gradual-disclosure .NET documentation while preserving existing fallback behavior.

Changes:

  • Adds cref target and member-page indexing.
  • Threads link context through XML documentation rendering.
  • Adds tests and design, verification, and requirements updates.
File Description
test/​ApiMark.DotNet.Tests/​XmlDocReaderTests.cs Tests cref rendering and fallbacks.
test/​ApiMark.DotNet.Tests/​TypeLinkResolverTests.cs Tests resolved link helpers.
test/​ApiMark.DotNet.Tests/​DotNetGeneratorTests.cs Updates generator expectations.
test/​ApiMark.DotNet.Tests/​DotNetEmitterSingleFileTests.cs Verifies single-file fallback behavior.
test/​ApiMark.DotNet.Tests/​DotNetEmitterGradualDisclosureTests.cs Tests gradual-disclosure links.
test/​ApiMark.DotNet.Tests/​DotNetAstModelTests.cs Tests stored indices.
test/​ApiMark.DotNet.Tests/​CrefTargetResolverTests.cs Tests target resolution.
test/​ApiMark.DotNet.Fixtures/​CrefLinkingClass.cs Adds cref-linking fixtures.
src/​ApiMark.DotNet/​XmlDocReader.cs Renders cref references with link context.
src/​ApiMark.DotNet/​TypeLinkResolver.cs Builds resolved target links.
src/​ApiMark.DotNet/​DotNetGenerator.cs Builds cref indices.
src/​ApiMark.DotNet/​DotNetEmitterGradualDisclosure.cs Integrates gradual-disclosure linking.
src/​ApiMark.DotNet/​DotNetEmitter.cs Extracts candidate-member filtering.
src/​ApiMark.DotNet/​DotNetAstModel.cs Stores cref and page indices.
src/​ApiMark.DotNet/​DocumentationCoverageChecker.cs Updates XML-documentation references.
src/​ApiMark.DotNet/​CrefTargetResolver.cs Indexes assembly cref targets.
src/​ApiMark.DotNet/​CrefLinkContext.cs Carries link-resolution state.
requirements.yaml Includes new requirements.
docs/​verification/​api-mark-dot-net/​xml-doc-reader.md Adds XML reader verification.
docs/​verification/​api-mark-dot-net/​type-link-resolver.md Adds link-helper verification.
docs/​verification/​api-mark-dot-net/​dot-net-generator.md Adds generator verification.
docs/​verification/​api-mark-dot-net/​dot-net-emitter-single-file.md Verifies single-file fallback.
docs/​verification/​api-mark-dot-net/​dot-net-emitter-gradual-disclosure.md Verifies gradual links.
docs/​verification/​api-mark-dot-net/​dot-net-ast-model.md Verifies model indices.
docs/​verification/​api-mark-dot-net/​cref-target-resolver.md Verifies resolver behavior.
docs/​reqstream/​api-mark-dot-net/​xml-doc-reader.yaml Defines reader requirements.
docs/​reqstream/​api-mark-dot-net/​type-link-resolver.yaml Defines link-resolver requirements.
docs/​reqstream/​api-mark-dot-net/​dot-net-generator.yaml Defines generator requirements.
docs/​reqstream/​api-mark-dot-net/​dot-net-emitter-single-file.yaml Defines single-file requirements.
docs/​reqstream/​api-mark-dot-net/​dot-net-emitter-gradual-disclosure.yaml Defines gradual-emitter requirements.
docs/​reqstream/​api-mark-dot-net/​dot-net-ast-model.yaml Defines model requirements.
docs/​reqstream/​api-mark-dot-net/​cref-target-resolver.yaml Defines resolver requirements.
docs/​reqstream/​api-mark-dot-net.yaml Registers subsystem requirements.
docs/​design/​api-mark-dot-net/​xml-doc-reader.md Documents cref rendering.
docs/​design/​api-mark-dot-net/​type-link-resolver.md Documents resolved links.
docs/​design/​api-mark-dot-net/​dot-net-generator.md Documents index construction.
docs/​design/​api-mark-dot-net/​dot-net-emitter-gradual-disclosure.md Documents emitter integration.
docs/​design/​api-mark-dot-net/​dot-net-ast-model.md Documents model indices.
docs/​design/​api-mark-dot-net/​cref-target-resolver.md Documents target indexing.
docs/​design/​api-mark-dot-net.md Updates the architecture overview.
.reviewmark.yaml Registers the review scope.

🧠 Review effort: Lite


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

Comment thread src/ApiMark.DotNet/CrefTargetResolver.cs
Comment thread src/ApiMark.DotNet/DotNetEmitterGradualDisclosure.cs
Comment thread src/ApiMark.DotNet/DotNetGenerator.cs
Comment thread src/ApiMark.DotNet/XmlDocReader.cs
Comment thread src/ApiMark.DotNet/XmlDocReader.cs Outdated
Critical fix: GetInlineReferenceText/TryLinkifyCref were composing
`[Type.Member](path.md)` (a Markdown link nested inside a code span,
which CommonMark renders literally) instead of [`Type.Member`](path.md)
(code span inside the link label). This made the cref cross-reference
linking feature produce zero working links despite the existing tests
passing, because those tests asserted the broken shape as correct.
Fixed the implementation, all affected test assertions, and the design
docs that described the broken nesting.

Also addresses findings from the code-review and formal-review agents
run across the affected review-sets:

- Corrected design docs describing public XmlDocReader methods as
  threading CrefLinkContext directly; the actual change added internal
  overloads and left the public API surface unchanged.
- Reworded several HOW-oriented requirement titles (method/parameter
  names, refactor mandates) to state only observable behavior, moving
  implementation detail into their justification fields.
- Documented DotNetAstModelArgs (the constructor parameter-bundling
  record) in the DotNetAstModel design doc, including its Data Model
  entry and corrected constructor description.
- Documented GetCandidateMembers in the DotNetEmitter design doc as the
  structural superset consumed by both GetVisibleMembers and
  DotNetEmitterGradualDisclosure.BuildMemberPageIndex.
- Documented that DotNetEmitterSingleFile always uses parameterless
  XmlDocReader overloads so cref references never link in single-file
  output.
- Added missing file-to-unit map entries (CrefTargetResolver,
  DocumentationCoverageChecker) to the DotNetGenerator design doc.
- Added Acceptance Criteria and Test Scenario coverage for cref
  cross-reference linking to the api-mark-dot-net verification doc.
- Added a "Cross-Reference Linking" subsection to the dotnet user guide
  describing the new linking behavior and its single-file limitation.

Validation: dotnet build (0 warnings/errors); ApiMark.DotNet.Tests
436/436 passing on net8.0/net9.0/net10.0; ApiMark.Cpp.Tests 195/195
passing (with clang on PATH); fix.ps1 and lint.ps1 clean.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings October 8, 2026 01:54

Copilot AI left a comment

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.

🔵 Needs a closer look

Unresolved cref-link correctness issues and architecture/requirements traceability gaps remain.

4 open findings
1 resolved since last review
Previously missed (2)

In code that hasn't changed since last review

Medium severity Include generic method arity in XML documentation IDs

src/​ApiMark.DotNet/​CrefTargetResolver.cs:62

BuildMemberId delegates to BuildMethodId, which omits the generic-method arity ( ``N) required in XML-doc IDs. As a result, valid crefs such as M:Type.Method``1(System.Int32) are not added under their compiler-emitted ID and remain unlinked (and can collide with a non-generic overload); include the method arity in the shared ID builder and cover a generic-method cref.

Medium severity Apply cref linking to namespace documentation

src/​ApiMark.DotNet/​DotNetEmitterGradualDisclosure.cs:316

The link context is applied only to type-row summaries here. BuildNamespaceDescription materializes NamespaceDoc summaries, remarks, and examples during Parse with the parameterless XmlDocReader APIs, and WriteNamespacePage later emits those strings verbatim; therefore <see cref>/<seealso cref> inside namespace documentation never gets linked in gradual-disclosure output. Defer or re-render NamespaceDoc content with a folder-scoped context if namespace prose is in this feature's scope.

🧠 Review effort: Lite


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

…inking

GitHub AI review fixes:
- Extract shared GroupMembersByFileName/GetOrderedOverloads/
  DecideGroupPageFileName helpers in DotNetEmitterGradualDisclosure so the
  cref-link index builder and the page writer can no longer silently drift
  out of sync on member grouping/overload-ordering/page-naming decisions.
- Apply CrefLinkContext to namespace-level documentation: NamespaceDescription
  now stores the winning carrier's summary/remarks/example member IDs instead
  of pre-rendered text, deferring rendering to WriteNamespacePage once a
  folder-scoped CrefLinkContext is available, so <see cref>/<seealso cref> in
  a NamespaceDoc carrier's remarks/summary/example now link like type/member
  summaries do. DotNetEmitterSingleFile keeps passing no CrefLinkContext,
  preserving its no-links-in-single-file behavior.
- Add the missing SysML2 model entry for CrefTargetResolver, mirroring the
  existing TypeLinkResolver pattern.

Formal-review follow-ups:
- Correct stale design/verification/reqstream documentation that still
  referenced a non-existent WriteMembersSection function, an inverted
  cref-link nesting description, and stale NamespaceDescription field names.
- Document the (pre-existing, already-fixed) unconditional
  WriteNestedTypesSection call and its rationale in the gradual-disclosure
  design doc, and add verification/reqstream coverage for the
  memberless-container-type and generic-method-arity regression tests that
  already existed but weren't documented.
- Add a single-file-mode regression test confirming a NamespaceDoc remarks
  cref also renders as plain code-span text with no link, matching the
  type/member behavior.
- Remove a duplicate CrefLinkContext construction in WriteNamespacePage.

Other fixes:
- Exclude the gitignored artifacts/ build-output folder from markdownlint-cli2
  scanning; it was never part of any PR diff but was still being linted
  locally.
- Add "Memberless" and "desync" to the cspell word list for new test/comment
  identifiers.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings October 8, 2026 04:24

Copilot AI left a comment

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.

Comment thread src/ApiMark.DotNet/CrefTargetResolver.cs
Comment thread src/ApiMark.DotNet/DotNetEmitter.cs
Comment thread src/ApiMark.DotNet/DotNetEmitter.cs
Comment thread src/ApiMark.DotNet/XmlDocReader.cs
…review-sets

- Relocate DotNetGenerator_BuildMethodIdFromReference_ExplicitGenericInterfaceOverride_MatchesInterfaceMethodId
  from CrefTargetResolverTests.cs to DotNetGeneratorTests.cs (it exercises
  DotNetGenerator, not CrefTargetResolver)
- Add ExplicitIndexerClass fixture and widen MapAccessorReferenceToPropertyId to
  internal (with rationale remarks, matching existing convention) to cover the
  indexer parameter-list handling for explicit accessor overrides
- Add remarks rationale to BuildMethodIdFromReference's internal visibility
- Correct cref-target-resolver.md verification doc (remove misattributed
  acceptance criteria/test scenario) and dot-net-generator.md verification doc
  (add acceptance criteria/test scenarios for the new/relocated tests)
- Fix stale "unmodified" wording in cref-target-resolver.md design doc now that
  BuildMemberId/BuildTypeId formatting is reused as-is rather than duplicated
- Link reqstream requirements to the new/moved tests

All findings from the DotNetGenerator and CrefTargetResolver formal reviews are
resolved; both review-sets are now Approved.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings October 8, 2026 13:15

Copilot AI left a comment

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.

🔵 Needs a closer look

Resolve the explicit-label and GetExample cref gaps, align CrefLinkContext documentation with its API, and remove the unsupported Markdown lint ignore.

2 open findings
4 resolved since last review
Previously missed (1)

In code that hasn't changed since last review

Medium severity Resolve cref links when XML see elements have custom labels

src/​ApiMark.DotNet/​XmlDocReader.cs:1473

This linkification call is unreachable for <see cref="...">custom label</see>: GetInlineReferenceText returns the explicit element.Value before it ever reads cref, so valid in-assembly references with an explicit display label remain unlinked. Preserve the explicit label while still resolving the cref and wrapping that label in the target link (without changing the existing fallback when resolution fails).

🧠 Review effort: Lite


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

- GetInlineReferenceText now resolves and links the cref attribute even when
  the <see>/<seealso> element has explicit display text, wrapping that
  explicit label in the target link instead of leaving it unlinked. Falls
  back to the unlinked label exactly as before when there is no link context
  or the cref does not resolve.
- Add 3 unit tests covering the resolved/linked, null-context, and
  unresolved-cref fallback cases for the explicit-label path.
- Update xml-doc-reader.md design/verification docs and reqstream test links
  to reflect the new behavior.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings October 8, 2026 13:41
@Malcolmnixon

Copy link
Copy Markdown
Member Author

Fixed the third finding from this round too — Resolve cref links when XML see elements have custom labels (XmlDocReader.cs:1473, previously-missed/no discussion thread to reply on directly): GetInlineReferenceText now resolves and links the cref even when the element has an explicit display label, wrapping that label in the target link instead of leaving it unlinked. See bfc541a.

All 3 concrete findings from this review are now fixed and pushed (6db9a13, bfc541a).

One item from the headline I couldn't act on with confidence: "remove the unsupported Markdown lint ignore." I couldn't find an inline comment with specifics, and the only candidate I found — the '**/artifacts/**' entry added to .markdownlint-cli2.yaml — uses the documented, supported ignores config property and verifiably works (lint.ps1/fix.ps1 both report 0 markdown errors with an artifacts/ folder present on disk). If you can point me at the specific concern, I'll fix it.

Copilot AI left a comment

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.

🟡 Changes recommended

Two unresolved moderate findings affect constructor fallback behavior and the claimed public API.

1 open finding
2 resolved since last review

🧠 Review effort: Lite


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

Comment thread src/ApiMark.DotNet/XmlDocReader.cs Outdated
- The explicit-label branch now checks FormatCref's ShouldWrapInCodeSpan
  classification before calling TryLinkifyCref, so a constructor cref
  (M:...#ctor) is never linked even when the <see>/<seealso> element has an
  explicit display label - matching the no-label branch's existing
  precedent that constructor crefs are never linked.
- Add a regression test
  (XmlDocReader_GetSummary_ExplicitLabelConstructorCref_RendersLabelOnlyNoLink)
  covering a labeled constructor cref whose declaring type is otherwise
  resolvable and emitted.
- Update xml-doc-reader.md design/verification docs and reqstream test
  links accordingly.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings October 8, 2026 14:15

Copilot AI left a comment

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.

🔵 Needs a closer look

Address the malformed custom-label links, unnecessary single-file indexing, documentation mismatches, and unrelated lint exception.

0 open findings

1 resolved since last review

🧠 Review effort: Lite


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

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.

2 participants