Enhance review hierarchy and improve DotNet/C++ compliance#31
Merged
Conversation
…cture context flow - Rename Specification review to Decomposition (repo-level, no system prefix) - Rename Purpose review to repo-level (no system prefix) - Add README + user_guide context to all Architecture reviews - Add system requirements as context to all Design reviews - Add system requirements as context to all Verification reviews - Move system-level requirements from paths to context in AllRequirements reviews - Upgrade ReviewMark to 1.3.0-beta.3 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add 8-level review context hierarchy to .reviewmark.yaml so every software item receives design, requirements, and parent-system context during formal review - Update ReviewMark to 1.3.0; exclude design intro from Purpose review using '!' syntax to prevent design back-feeding into user-doc reviews - Add verification/introduction.md to Core-Verification review-set paths ApiMark.Core contract hardening (driven by 9 rounds of formal review): - EmitConfig: validate HeadingDepth range 1-3 at construction - FileMarkdownWriter: null/empty/shape guards on all 6 write methods; WriteTable validates headers non-empty, row lengths, null rows/elements - GlobFileCollector: StringComparer.Ordinal fix; null guards on Collect - PathHelpers: zero-segment returns basePath; clarified error handling - IMarkdownWriterFactory: whitespace subFolder treated as root-level - IApiGenerator/IApiEmitter: XML doc corrections and requirement splits Test helpers hardened to enforce same preconditions as production code: - InMemoryMarkdownWriter: heading range 1-6, table shape, null elements - InMemoryContext: null guards New artifacts for FileMarkdownWriter and FileMarkdownWriterFactory: - docs/design, docs/reqstream, docs/verification unit documents - EmitConfigTests.cs created Documentation: - design/introduction.md: scope clarified; test/ removed from Folder Layout - verification/introduction.md: scope clarified; Folder Layout section added - i-markdown-writer.md: preconditions documented for all write methods - N/A em-dash sweep across 36 design/verification docs - docs/design/ots.md: OTS strategy split by type; lock-file claim removed - docs/user_guide/cpp.md: @code/@Endcode authoring section added - README.md: ClangPath -> ApiMarkClangPath 114 Core tests passing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Restructure DotNetGenerator, DotNetEmitter, and sub-emitters for clarity - Merge ApiVisibility.cs and DotNetGeneratorOptions.cs into DotNetGenerator.cs - Merge ExternalTypeInfo.cs into TypeLinkResolver.cs - Add IContext progress reporting to DotNetGenerator - Fix init accessor detection via RequiredModifierType (IsExternalInit modreq) - Fix array rank rendering in TypeLinkResolver and TypeNameSimplifier - Fix BuildPropertyAccessors accessibility logic - Add ThrowIfNull guards for options and context in DotNetGenerator - Add BuildInheritanceChain requirement and RejectNullContext test - Add operators page, NamespaceDoc, and nested type support - Update requirements: move emitting reqs from Generator to unit YAMLs - Update design docs: all 9 DotNet unit design docs for consistency - Update verification docs: all DotNet unit verification docs - Add copyright headers to fixture files; add new fixture classes - Add 237 passing tests across DotNet test suite Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Design docs: restore clang interface, expand MSBuild 4->9 props, add GD/SF ->CppAstModel diagram edges, add RunProcess with concurrent-drain detail, expand Parse Key Method, add platform path-comparison note, add CppGenerator External Interfaces section, fix CppNamespaceDecl in deps, cross-ref CppExternalTypeInfo, expand GD Emit method spec. Requirements: rewrite impl-detail reqs, rewrite ParseHeaders (no HOW), add RejectNullContext + WorkingDirectory reqs, remove cross-unit test refs, fix SupportAbsolute to mention WorkingDirectory. Source code: position-aware splice in CppTypeLinkResolver (String.Replace bug), knownTypes null guard, PathConventionRows 2 missing rows, TypeAliases.Count in declaration count, SanitizeFileName null guard, CppGenerator Parse null-context guard + XML doc fixes. Tests: template-arg prefix corruption regression, namespace operators page, Parse null-context, ClangAstParser null options + empty errors, IsDeleted assertion, CppEmitter final+base + GetIncludePath + WriteExternalTypesSection, tightened weak Contains assertion. Verification docs: clang-ast-parser 9 scenarios, cpp-ast-model 15 scenarios, cpp-emitter 7 scenarios, GD 5 new scenarios, SF writer-key scenario, cpp-generator ownership/links/external/deleted, TLR 4+ scenarios, api-mark-cpp single-file criterion and scenario. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Group A: TLR fixes - Remove null/whitespace tests from LinkifyPrimitiveType requirement (A1) - Add ArgumentNullException.ThrowIfNull(externalTypes) guard in Linkify (A2) - Add currentFolder ??= string.Empty null-coalescing semantics (A3) - Add 8th acceptance criterion to TLR verification doc (A4) Group B: CppGenerator external types integration test - Create ExternalTypeFixture.h with external::ns::Logger* return type (B1) - Add CppGenerator_Generate_ExternalTypeReference_EmitsExternalTypesSection test (B1) - Add external types criterion and scenario to api-mark-cpp.md (B2) Group C: Verification doc prose format fixes - Rewrite 10 scenario descriptions in cpp-ast-model.md (C1) - Fix 2 scenario descriptions in cpp-emitter-single-file.md (C2) - Add inheritance criterion and scenario to api-mark-cpp.md (C3) Group D: Test organisation - Move CppTypeLinkResolver_Linkify_UnknownNamespacedType_TracksExternalType from CppGeneratorTests.cs to CppTypeLinkResolverTests.cs (D1) - Replace with CppGenerator_Generate_ExternalTypeReference_EmitsExternalTypesSection (D1) Group E: CppEmitter low findings - Fix GetIncludePath XML doc: 'Must not be null or empty' -> 'Must not be null' (E1) - Add CppEmitter_GetIncludePath_NullSourceFile_ThrowsArgumentNullException test (E2) - Add CppEmitter_WriteCombinedMemberPage_TooFewMembers_ThrowsArgumentException test (E2) - Update cpp-emitter.md verification doc with new criteria and scenarios (E2/E3) - Add FinalClassWithBaseTypes test to BuildClassDeclaration requirement yaml (E3) Group F: CppAstModel low findings - Add Assert.False(cppEnum.IsDeprecated) to CppEnum construction test (F1) - Add Assert.False(alias.IsDeprecated) to CppTypeAlias construction test (F1) - Add Assert.Null(fn.Location/fn.Doc) to CppFunction construction test (F2) - Add Assert.Null(cls.Location/cls.Doc) to CppClass construction test (F2) - Add Assert.Null(field.Doc) to CppField construction test (F2) Group G: GD low findings - Reorder PathConventionRows table to match code array order (G1) - Add WriteClassSignatureBlock null-Location early-return note (G2) - Add WriteTypePage early-exit condition for empty classes (G2) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The test CppGenerator_Generate_ExternalTypeReference_EmitsExternalTypesSection exercises the full Cpp pipeline (CppGenerator + CppTypeLinkResolver + CppEmitter) and is an integration test at the subsystem level. Add a top-level subsystem requirement ApiMarkCpp-EmitExternalTypesSection to api-mark-cpp.yaml and link the test to it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… R6 F-01)
Add Assert.True(factory.HasWriter("testlib/Widget", "size_type")) to
Emit_TypeAlias_CreatesTypeAliasPage so the class-scoped alias path is
explicitly verified. Update the verification doc scenario and acceptance
criterion to cover both namespace-level and class-scoped type aliases.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…rarchy Add ApiMarkCpp-EmitExternalTypesSection to the parent requirement's children list and add a children list pointing to the two implementing unit requirements (CppEmitter-RepresentExternalTypeInfo, CppTypeLinkResolver-LinkifyExternalTypeTracking) to satisfy structural traceability (AllRequirements R7 findings R7-01, R7-02). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Design: add CppEmitterGradualDisclosure/SingleFile->CppEmitter back-edges - Architecture: remove CppAstModel from Dependencies (internal unit, not OTS) - Verification: expand operator criterion and naming convention to include namespace-level operators.md; add namespace-level operator scenario; add absolute-pattern scenario; normalize 'This scenario is tested by' prose in cpp-ast-model.md, cpp-emitter.md, cpp-emitter-single-file.md Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Design: - Remove CppEmitterSingleFile->CppTypeLinkResolver diagram edge (SingleFile accepts but discards the resolver; type links are omitted in single-file mode) TLR design: - Remove CppEmitterSingleFile from Callers section (confirmed unused) - Add empty-stripped-name early-return note to Linkify Key Methods TLR tests: - Add AAA comments to NullKnownTypes and NullExternalTypes null-guard tests TLR verification: - Add test scenarios for constructor null-guard and Linkify null externalTypes - Normalize 4x 'This scenario is tested by' -> 'Tested by' System verification (api-mark-cpp.md): - Normalize 31x 'This scenario is tested by' -> 'Tested by' - Add namespace/operators.md rows to user guide output structure table GD verification: - Clarify operator criterion: class-level and namespace-level operator pages Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…k-cpp.md
8 instances spanned line breaks ('This scenario is<newline>tested by' or
'This scenario is tested<newline>by') that weren't caught by the single-line
replace in R8/R9. Fixed with pattern matching across the line boundary.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- CppEmitter design doc: add instance fields (_options, _namespaceDecls, _cppResolver) to Data Model section and constructor to Key Methods - CppTypeLinkResolver design doc: remove CppEmitterSingleFile from Callers (confirmed by code: resolver is accepted but discarded via _ = cppResolver) - api-mark-cpp.md: remove CppEmitterSingleFile->CppTypeLinkResolver diagram edge (SingleFile does not use the resolver; type links omitted in single-file mode to prevent anchor collisions) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add WriteSingleFileTypeAliasSection to CppEmitterSingleFile.cs
- Wire namespace-level TypeAliases loop in EmitSingleFile
- Emit class-scoped type aliases as H{depth+3} sub-entries in WriteSingleFileClassSection
- Add unit test CppEmitterSingleFile_Emit_TypeAlias_ContainsTypeAliasSection
- Update design doc: Purpose, Emit algorithm, WriteSingleFileClassSection description,
add WriteSingleFileTypeAliasSection Key Method entry, update Dependencies
- Update api-mark-cpp.md Data Flow step 4 to include type aliases in single-file path
- Add ApiMarkCpp-CppEmitterSingleFile-EmitsTypeAliasSection requirement and test link
- Add type alias test scenario to verification doc
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add class-scoped type alias req + test (AllRequirements/SingleFile R10 F-01/F-02) - Fix CppEmitterSingleFile Data Model: document _emitter/_namespaceDecls fields (Design R10 F-01) - Fix CppEmitterGradualDisclosure Data Model: document instance fields (Design R10 F-02) - Add constructors to Key Methods for SingleFile and GD units (Design R10 F-03) - Extend EmitSingleFile algorithm description with namespace heading/summary (Design R10 F-04) - Add system-level type alias criterion + scenario to api-mark-cpp.md (Architecture R10) - Add CWD-relative criterion to system verification (Verification R10 VER-02) - Add final keyword criterion to system verification (Verification R10 VER-06) - Add single-file type alias criterion to system verification (Architecture R10) - Add 6 missing system-level test scenarios to api-mark-cpp.md (Verification R10 VER-01..VER-05) - Add class-scoped alias acceptance criterion to cpp-emitter-single-file.md (VER-08) - Add cross-references to mono-cecil.md and clang.md from ots.md (VER-07) - Normalize 'This scenario is tested by' in ots/clang.md (OBS-02) - Add CppGenerator_SingleFile_TypeAlias_AppearsInOutput integration test Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…Requirements R11)
Split malformed EmitsClassScopedTypeAliasSection block into two proper entries:
- EmitsTypeAliasSection (namespace-level aliases)
- EmitsClassScopedTypeAliasSection (class-scoped aliases at H{depth+3})
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add CppTypeLinkResolver constructor to Key Methods (Design R11) - Add final keyword test scenario to system verification (Architecture R11) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Fix stale test name in api-mark-cpp.md system verification (Verification R12) - Split compound GD EmitTypeAliasPage into namespace-level and class-scoped reqs (AllRequirements R12) - Add CppEmitterGradualDisclosure_Emit_ClassScopedTypeAlias_CreatesAliasPage test Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…s (AllRequirements R13) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Fix 1: correct 5 wrong test references in vhdl-emitter-gradual-disclosure.yaml (EmitSubprogramDetailPage requirement now links to the 4 actual test names) - Fix 2: correct arch test name in vhdl-emitter-gradual-disclosure.md verification doc and add subprogram detail page scenarios - Fix 3a: correct VhdlEmitterGradualDisclosure architecture description in api-mark-vhdl.md (architectures are inline, not separate files) - Fix 3b: remove version-specific platform constraint from api-mark-vhdl.md - Fix 3c: add ANTLR4 OTS cross-reference link in api-mark-vhdl.md dependencies - Fix 4: remove orphaned fragment 'a VhdlDocComment.' from vhdl-ast-parser.md - Fix 5: add VhdlEmitter_ImplementsIApiEmitter test to VhdlEmitterTests.cs and link it in vhdl-emitter.yaml - Fix 6: update VhdlGenerator glob pattern title to include .vhdl extension - Fix 7: split ParsePackage into parent + 7 discrete child requirements (ParsePackageTypes, ParsePackageConstants, ParsePackageComponents, ParsePackageSubprograms, ParseSubprogramParameters, ParseSubprogramReturnType, ParseSubprogramDocComments) in vhdl-ast-parser.yaml - Fix 8: add CounterVhd property to FixturePaths; add entity name and generic inline doc comment tests to VhdlAstParserTests.cs - Fix 9: update api-mark-vhdl.md verification acceptance criteria (precise entity name, generic comments, syntax error rejection) and add three new test scenarios
- Replace markdown link with prose cross-reference for ANTLR4 OTS doc (design documentation standard requires verbal not hyperlink references) - Add inline architecture acceptance criterion and test scenario to docs/verification/api-mark-vhdl.md - Add ApiMarkVhdl-VhdlGenerator-ValidatesLibraryName requirement with Empty/Whitespace constructor test links (moved from AcceptSourceGlobPatterns) - Add EntityNameIsCounter and GenericsHaveInlineDocComments test links to ApiMarkVhdl-VhdlAstParser-ParseEntity Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add VhdlGenerator_Parse_InvalidVhdlFile_EmitsErrorAndSkipsFile test - Add ApiMarkVhdl-VhdlGenerator-ReportParseErrors requirement with test link - Add architecture-inline test links to VhdlEmitterGradualDisclosure requirement Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Strengthen VhdlGenerator_Parse_InvalidVhdlFile_EmitsErrorAndSkipsFile test to use a mixed valid/invalid temp directory and assert valid files still produce output - Add ReportParseErrors acceptance criterion and scenario to vhdl-generator.md Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove sealed modifier from ApiMarkTask to allow test subclassing - Change RunToolProcess from private to protected virtual to provide a testability seam for overriding process execution in unit tests - Add FailingApiMarkTask test subclass that overrides RunToolProcess to simulate a non-zero exit without spawning a real child process - Add ApiMarkTask_Execute_ToolExitsNonZero_ReturnsFalseAndLogsError test verifying Execute returns false and logs an MSBuild error on non-zero exit - Split ApiMarkMsbuild-ApiMarkTask-SupportOutputsItemGroup compound requirement into three focused requirements: SupportOutputsItemGroup (parent), OverrideOutputProperties, and RestoreOutputProperties - Add ApiMarkMsbuild-ApiMarkTask-ResolveDotNetExecutable requirement for dotnet host path resolution behavior - Add ApiMarkMsbuild-ApiMarkTask-SurfaceToolExitCode requirement linked to the new test - Add ApiMarkMsbuild-ApiMarkTask-ForwardStdoutToLog requirement for stdout forwarding behavior - Replace known-gap notes in both verification docs with confirmed acceptance criteria - Add non-zero exit test scenario to api-mark-task verification doc
…sts and split dotnet-resolve requirement - Fix 1: correct ApiMarkOutput (item group) vs ApiMarkOutputs (task property) naming in SupportOutputsItemGroup requirement title and verification docs - Fix 2: change ResolveDotNetExe() from private static to protected virtual so test subclasses can override it; add NoDotNetApiMarkTask helper subclass - Fix 3: split ResolveDotNetExecutable requirement into two separate requirements: ApiMarkMsbuild-ApiMarkTask-ResolveDotNetFromEnvironment (DOTNET_HOST_PATH / PATH resolution, tested by the existing end-to-end test) and ApiMarkMsbuild-ApiMarkTask-ReportMissingDotNetExe (missing dotnet error path, tested by new ApiMarkTask_Execute_DotNetExeNotResolved_ReturnsFalseAndLogsError) - Fix 4: add LogMessageEvent assertion to ApiMarkTask_Execute_WithDotNetProject_ GeneratesDocumentation and link it to the ForwardStdoutToLog requirement - Fix 5: add ApiMarkMsbuild-ApiMarkTask-ForwardStderrErrors requirement and ApiMarkTask_Execute_ToolWritesToStderr_ForwardsToMsBuildErrors test that runs the real tool with invalid inputs to exercise the ErrorDataReceived forwarding path - Fix 6: add ApiMarkTask_Execute_WithMultipleOutputs_RunsToolForEachOutput test with RecordingApiMarkTask helper; link test to SupportOutputsItemGroup requirement and add verification scenario in both verification docs - Fix 7: update verification docs with new acceptance criteria and test scenarios for dotnet resolution failure, stdout forwarding, stderr forwarding, and multi-output execution All 26 MSBuild tests pass (4 new tests added).
- Make ResolveDotNetExe protected virtual; add NoDotNetApiMarkTask subclass - Split ResolveDotNetExecutable into ResolveDotNetFromEnvironment + ReportMissingDotNetExe - Add ApiMarkTask_Execute_DotNetExeNotResolved_ReturnsFalseAndLogsError test - Add ForwardStderrErrors requirement + ApiMarkTask_Execute_ToolWritesToStderr test - Add ForwardStdoutToLog test evidence via end-to-end test assertion - Add RecordingApiMarkTask + multi-output execution test - Fix ApiMarkOutput (item group) vs ApiMarkOutputs (task property) naming in docs - Rewrite ForwardApiHeaders title as single observable criterion; move order/verbatim detail to justification - Update verification docs with new acceptance criteria and scenarios Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…older - Rewrite OverrideOutputProperties/RestoreOutputProperties in observable-behavior terms - Fix ApiMarkOutputs (task property) vs ApiMarkOutput (item group) in design doc - Add test/ApiMark.MSBuild.PackageTests/ to design and verification introduction listings Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ssor accessibility
Bug 1: Context.cs only guarded --depth > 3 when --format single-file was active.
Supplying --depth 4 with the default gradual-disclosure format passed CLI
validation, reached EmitConfig.HeadingDepth = 4, and threw a cryptic
ArgumentOutOfRangeException from the setter. Add an unconditional guard that
rejects any value above 3 with a clear error for every format, before the
existing single-file-specific check.
Bug 2: BuildPropertyAccessors (and BuildPropertySignature) used
GetAccessibilityKeyword(prop.GetMethod ?? prop.SetMethod!)
which always picks the getter when present. For a public property with a
restricted getter (e.g. protected get; set;) this produced the wrong
property-level keyword (protected) and emitted contradictory output such as
'protected Foo { get; public set; }' instead of 'public Foo { protected get; set; }'.
Add a MostPermissiveAccessor(MethodDefinition? a, MethodDefinition? b) helper
backed by an AccessibilityRank scorer, and use it in both call sites so that
the property-level keyword always matches the most permissive accessor.
Tests added:
- Context_Create_WithDepth4_ThrowsArgumentException (Tool.Tests)
- DotNetEmitter_BuildPropertyAccessors_AsymmetricGetSet_UsesMostPermissiveAccessibility (DotNet.Tests)
- Fixture class AsymmetricAccessorClass with public { protected get; set; } property
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace LastIndexOf('::', cppTypeString) approach with a search bounded
before the first '<', preventing corruption when a template argument is a
qualified name sharing a prefix with the type being linked
(e.g. ns::Foo<ns::FooBar> was producing ns::Foo<ns::[Foo](link)Bar>).
Add regression test: QualifiedTypeWithQualifiedTemplateArgSharingPrefix.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…-independent - Change GetRequiredIntArgument upper bound from 6 to 3 so '--depth 7' reports the correct valid range (1-3) instead of the misleading 1-6 - Override ResolveDotNetExe in FailingApiMarkTask to return 'dummy-dotnet', ensuring the non-zero exit-code test always reaches RunToolProcess regardless of whether dotnet is on PATH Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR substantially expands ApiMark’s correctness/contract enforcement and corresponding documentation by adding new guardrails (null/argument validation), improving type/name formatting, and broadening test coverage across Core, Tool, DotNet, Cpp, and VHDL. It also updates verification/design/requirements artifacts to reflect the new behaviors and adds a few tooling/docs adjustments.
Changes:
- Strengthen API boundary validation (e.g., null guards, EmitConfig heading-depth range enforcement) and extend CLI parsing/behavior.
- Improve documentation generation fidelity (e.g., generic type cref rendering, array rank formatting, external-type tracking, C++ type-alias emission).
- Add/extend unit & integration tests plus update verification/design/requirements documentation accordingly.
Reviewed changes
Copilot reviewed 218 out of 218 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| test/ApiMark.Vhdl.Tests/VhdlGeneratorTests.cs | Adds constructor/input and parse edge-case tests for VHDL generator behavior. |
| test/ApiMark.Vhdl.Tests/VhdlEmitterTests.cs | Adds VHDL emitter contract/format dispatch tests. |
| test/ApiMark.Vhdl.Tests/FixturePaths.cs | Adds fixture path for counter.vhd. |
| test/ApiMark.Tool.Tests/SelfTest/ValidationTests.cs | Strengthens results-file assertions via TRX/JUnit round-trip deserialization. |
| test/ApiMark.Tool.Tests/ConsoleCollection.cs | Introduces xUnit collection to serialize tests that mutate global Console streams. |
| test/ApiMark.Tool.Tests/ApiMark.Tool.Tests.csproj | Adds DemaConsulting.TestResults package reference for test deserialization. |
| test/ApiMark.DotNet.Tests/XmlDocReaderTests.cs | Adds test for generic-type cref formatting in summaries. |
| test/ApiMark.DotNet.Tests/DotNetGeneratorTests.cs | Adds generator guard tests and namespace-doc assertions. |
| test/ApiMark.DotNet.Tests/DotNetAstModelTests.cs | Tightens namespace/root assertions and adds read-only/nonnull model checks. |
| test/ApiMark.DotNet.Fixtures/StaticFixtureClass.cs | New fixture for static-class signature emission. |
| test/ApiMark.DotNet.Fixtures/SampleEnum.cs | Adds license header. |
| test/ApiMark.DotNet.Fixtures/SampleClass.cs | Adds license header. |
| test/ApiMark.DotNet.Fixtures/OuterClass.cs | Adds license header. |
| test/ApiMark.DotNet.Fixtures/OperatorsStruct.cs | Adds license header. |
| test/ApiMark.DotNet.Fixtures/IsExternalInit.cs | Adds IsExternalInit polyfill for init-only setters on netstandard targets. |
| test/ApiMark.DotNet.Fixtures/IntVsIntArrayClass.cs | Adds license header. |
| test/ApiMark.DotNet.Fixtures/InitPropertyClass.cs | New fixture for init-only property accessor rendering. |
| test/ApiMark.DotNet.Fixtures/CaseCollisionClass.cs | Adds license header. |
| test/ApiMark.DotNet.Fixtures/AsymmetricAccessorClass.cs | New fixture for asymmetric accessor accessibility rendering. |
| test/ApiMark.DotNet.Fixtures/AbstractFixtureClass.cs | New fixture for abstract modifier/signature rendering. |
| test/ApiMark.Cpp.Tests/CppGeneratorFixture.cs | Updates fixture docs to reflect Parse/Emit pipeline and option-count change. |
| test/ApiMark.Cpp.Tests/CppAstModelTests.cs | Expands default-property assertions for AST records. |
| test/ApiMark.Cpp.Tests/ClangAstParserTests.cs | Adds argument-guard tests and “errors collection empty” test for fixtures. |
| test/ApiMark.Cpp.Fixtures/include/fixtures/ExternalTypeFixture.h | New C++ fixture for external (non-std) type reference tracking. |
| test/ApiMark.Core.Tests/PathHelpersTests.cs | Adds test for zero-segment SafePathCombine behavior. |
| test/ApiMark.Core.Tests/IContextTests.cs | Adds null-guard and ordering tests for InMemoryContext channels. |
| test/ApiMark.Core.Tests/GlobFileCollectorTests.cs | Adds null-argument guard tests for GlobFileCollector. |
| test/ApiMark.Core.Tests/FileMarkdownWriterFactoryTests.cs | Adds constructor and CreateMarkdown validation tests + whitespace subfolder behavior. |
| test/ApiMark.Core.Tests/EmitConfigTests.cs | New tests for EmitConfig defaults and heading-depth range validation. |
| test/ApiMark.Core.TestHelpers/InMemoryContext.cs | Enforces null guards in test context implementation. |
| src/ApiMark.Vhdl/VhdlGenerator.cs | Documents normalization behavior for null Sources. |
| src/ApiMark.Vhdl/VhdlEmitterSingleFile.cs | Clarifies unused context parameter intent in docs. |
| src/ApiMark.Vhdl/VhdlEmitterGradualDisclosure.cs | Clarifies unused config/context parameter intent in docs. |
| src/ApiMark.Vhdl/VhdlEmitter.cs | Refines internal documentation for keyword stripping/options/models. |
| src/ApiMark.Vhdl/VhdlAst/VhdlAstParser.cs | Uses sourceText splitting for line indexing; improves error listener docs. |
| src/ApiMark.Vhdl/VhdlAst/VhdlAstModel.cs | Clarifies port/mode documentation semantics. |
| src/ApiMark.Tool/Program.cs | Updates help text to include --result alias and constructs EmitConfig from CLI context. |
| src/ApiMark.Tool/Cli/Context.cs | Tightens --depth constraints and rejects flag tokens as string values. |
| src/ApiMark.MSBuild/ApiMarkTask.cs | Makes task extensible (unsealed + virtualized process/dotnet resolution). |
| src/ApiMark.DotNet/XmlDocReader.cs | Adds thread-safety remarks; formats generic-type arity as List<T> style placeholders. |
| src/ApiMark.DotNet/TypeLinkResolver.cs | Improves array rank formatting and inlines ExternalTypeInfo record. |
| src/ApiMark.DotNet/ExternalTypeInfo.cs | Removes standalone ExternalTypeInfo (moved into TypeLinkResolver). |
| src/ApiMark.DotNet/DotNetGeneratorOptions.cs | Removes standalone options type (moved elsewhere in DotNet assembly). |
| src/ApiMark.DotNet/DotNetEmitterGradualDisclosure.cs | Refactors member-page writing path (introduces consolidated writer helper usage). |
| src/ApiMark.DotNet/DotNetAstModel.cs | Switches to read-only collection interfaces for model/context records. |
| src/ApiMark.DotNet/ApiVisibility.cs | Removes standalone enum (moved elsewhere in DotNet assembly). |
| src/ApiMark.Cpp/CppGenerator.cs | Adds null-guard for Parse context and updates process description docs. |
| src/ApiMark.Cpp/CppEmitterSingleFile.cs | Emits namespace-level and class-level type aliases in single-file output. |
| src/ApiMark.Cpp/CppEmitter.cs | Adds argument guards; enforces min-size precondition for combined member page writer. |
| src/ApiMark.Cpp/CppAst/CppAstModel.cs | Adds concurrency/read-only safety remarks to AST model. |
| src/ApiMark.Cpp/CppAst/ClangAstParser.cs | Adds null/empty/whitespace header + options validation. |
| src/ApiMark.Core/PathHelpers.cs | Updates docs to reflect zero-segment behavior. |
| src/ApiMark.Core/IMarkdownWriterFactory.cs | Clarifies Parse/Emit pipeline and whitespace subfolder behavior. |
| src/ApiMark.Core/IApiGenerator.cs | Clarifies Parse contract wording. |
| src/ApiMark.Core/GlobFileCollector.cs | Adds null guards; adds literal-absolute-path handling; changes dedupe comparer. |
| src/ApiMark.Core/EmitConfig.cs | Enforces HeadingDepth range (1–3) at init time. |
| requirements.yaml | Adds reqstream includes for FileMarkdownWriter(+Factory) requirements. |
| README.md | Updates MSBuild clang property name reference and clarifies user guide distribution. |
| docs/verification/ots/file-system-globbing.md | Narrows described verified feature subset. |
| docs/verification/ots/dema-consulting-test-results.md | Updates verification narrative to match round-trip assertions. |
| docs/verification/ots/clang.md | Minor wording normalization in verification scenarios. |
| docs/verification/ots.md | Adds Mono.Cecil and TestResults sections; adds cross-links. |
| docs/verification/introduction.md | Expands layout/OTS list and clarifies scope boundaries. |
| docs/verification/api-mark-vhdl/vhdl-generator.md | Updates verification scenarios to include new edge cases. |
| docs/verification/api-mark-vhdl/vhdl-emitter.md | Updates verification scenarios for format dispatch and empty-output behavior. |
| docs/verification/api-mark-vhdl/vhdl-emitter-gradual-disclosure.md | Adds scenarios around architecture inline rendering and package subprogram detail files. |
| docs/verification/api-mark-vhdl/vhdl-ast-parser.md | Adds invalid-VHDL error scenario. |
| docs/verification/api-mark-tool/program.md | Adds results-file scenario and strengthens silent/log description. |
| docs/verification/api-mark-tool/cli/context.md | Updates CLI verification scenarios for depth validation and flag-as-value rejection. |
| docs/verification/api-mark-tool/cli.md | Clarifies silent/log routing expectations. |
| docs/verification/api-mark-msbuild/api-mark-task.md | Updates MSBuild task verification scenarios (stdout/stderr, dotnet resolution, multi-output). |
| docs/verification/api-mark-dot-net/type-link-resolver.md | Adds scenarios for external-type tracking, arrays, and generics. |
| docs/verification/api-mark-dot-net/dot-net-emitter-single-file.md | Expands single-file emitter acceptance criteria and scenarios. |
| docs/verification/api-mark-dot-net/dot-net-emitter-gradual-disclosure.md | Adds scenarios for overload/collision/operators/nested types. |
| docs/verification/api-mark-dot-net/dot-net-ast-model.md | Updates scenarios for read-only collections and non-null properties. |
| docs/verification/api-mark-dot-net.md | Clarifies member-page behavior (dedicated vs collision pages). |
| docs/verification/api-mark-core/path-helpers.md | Adds zero-segment behavior scenario. |
| docs/verification/api-mark-core/i-markdown-writer.md | Normalizes wording and updates referenced test name. |
| docs/verification/api-mark-core/i-markdown-writer-factory.md | Adds whitespace-subfolder root behavior scenario. |
| docs/verification/api-mark-core/i-context.md | Updates null-message contract and ordering scenarios. |
| docs/verification/api-mark-core/i-api-generator.md | Updates contract narrative for Parse/Emit separation and null-context behavior. |
| docs/verification/api-mark-core/i-api-emitter.md | Expands emitter contract verification (format selection, null guards). |
| docs/verification/api-mark-core/glob-file-collector.md | Adds literal-path and null-argument scenarios. |
| docs/verification/api-mark-core/file-markdown-writer-factory.md | New verification doc for filesystem writer factory. |
| docs/verification/api-mark-core/emit-config.md | Updates verification approach and adds range-validation scenarios. |
| docs/verification/api-mark-core.md | Updates core verification summary and scenario references. |
| docs/user_guide/introduction.md | Clarifies per-language coverage scope and file discovery responsibilities. |
| docs/user_guide/cpp.md | Adds Doxygen @code/@endcode example section; updates output structure table. |
| docs/reqstream/ots/file-system-globbing.yaml | Removes requirements for excluded features; narrows OTS requirement set. |
| docs/reqstream/ots/dema-consulting-test-results.yaml | Updates requirements and test links for results collection and serialization. |
| docs/reqstream/api-mark-vhdl/vhdl-generator.yaml | Expands VHDL generator requirements (library name validation, no-files matched, parse errors). |
| docs/reqstream/api-mark-vhdl/vhdl-emitter.yaml | Expands emitter requirements hierarchy and links to tests. |
| docs/reqstream/api-mark-vhdl/vhdl-emitter-gradual-disclosure.yaml | Adds requirement for per-subprogram detail pages. |
| docs/reqstream/api-mark-vhdl/vhdl-ast-model.yaml | Adds common_types fixture scenario coverage. |
| docs/reqstream/api-mark-tool/program.yaml | Adds results-file scenario to validation requirement evidence. |
| docs/reqstream/api-mark-tool/cli.yaml | Adds requirements for silent suppression and context-based output routing. |
| docs/reqstream/api-mark-dot-net/xml-doc-reader.yaml | Refines wording and expands inheritdoc requirement statement. |
| docs/reqstream/api-mark-dot-net/type-name-simplifier.yaml | Updates referenced test IDs to match new naming. |
| docs/reqstream/api-mark-dot-net/dot-net-ast-model.yaml | Updates test list for additional model properties/constraints. |
| docs/reqstream/api-mark-cpp/cpp-type-link-resolver.yaml | Refactors resolver requirements into finer-grained child requirements. |
| docs/reqstream/api-mark-cpp/cpp-emitter.yaml | Refactors emitter requirements (null guards, sanitization, external types, collision pages). |
| docs/reqstream/api-mark-cpp/cpp-ast-model.yaml | Adds/extends test coverage mapping for AST model requirements. |
| docs/reqstream/api-mark-cpp/clang-ast-parser.yaml | Updates Parse requirements and adds argument-guard tests. |
| docs/reqstream/api-mark-cpp.yaml | Adds top-level external-types requirement and other requirement references. |
| docs/reqstream/api-mark-core/path-helpers.yaml | Updates requirement to allow zero segments and adds corresponding test. |
| docs/reqstream/api-mark-core/i-markdown-writer-factory.yaml | Refines subfolder/root and output-dir validation requirements. |
| docs/reqstream/api-mark-core/i-context.yaml | Adds null-message rejection requirements. |
| docs/reqstream/api-mark-core/i-api-generator.yaml | Refines Parse contract requirements (context param, emitter return, null rejection). |
| docs/reqstream/api-mark-core/i-api-emitter.yaml | Expands emitter contract requirements (format support, api.md, null guards). |
| docs/reqstream/api-mark-core/glob-file-collector.yaml | Adds literal-absolute-path support and null-argument rejection requirements. |
| docs/reqstream/api-mark-core/file-markdown-writer-factory.yaml | New requirements file for FileMarkdownWriterFactory behavior. |
| docs/reqstream/api-mark-core/emit-config.yaml | Adds explicit tests for defaults and heading-depth range validation. |
| docs/reqstream/api-mark-core.yaml | Updates core requirements set and test mappings. |
| docs/design/ots/dema-consulting-test-results.md | Adds explicit error-handling behavior for results writing. |
| docs/design/ots.md | Clarifies OTS integration modes (NuGet vs external CLI tools). |
| docs/design/api-mark-vhdl/vhdl-generator.md | Adds Description option and clarifies generator option semantics. |
| docs/design/api-mark-vhdl/vhdl-emitter.md | Documents NoItemsPlaceholder and keyword stripping set. |
| docs/design/api-mark-vhdl/vhdl-emitter-single-file.md | Refines single-file layout and model dependencies list. |
| docs/design/api-mark-vhdl/vhdl-emitter-gradual-disclosure.md | Clarifies optional sections and inline architecture behavior. |
| docs/design/api-mark-vhdl/vhdl-ast-parser.md | Normalizes N/A formatting and clarifies doc parsing method. |
| docs/design/api-mark-vhdl/vhdl-ast-model.md | Normalizes N/A formatting and clarifies invariants. |
| docs/design/api-mark-vhdl.md | Updates architecture handling and parse-error data flow description. |
| docs/design/api-mark-tool/self-test/validation.md | Normalizes N/A formatting. |
| docs/design/api-mark-tool/program.md | Formatting tweaks and normalized N/A wording. |
| docs/design/api-mark-tool/cli/context.md | Documents depth constraints and normalizes N/A wording. |
| docs/design/api-mark-tool/cli.md | Normalizes N/A wording. |
| docs/design/api-mark-tool.md | Normalizes N/A wording. |
| docs/design/api-mark-msbuild/api-mark-task.md | Normalizes N/A wording. |
| docs/design/api-mark-msbuild.md | Clarifies multi-output item group naming and normalizes wording. |
| docs/design/api-mark-dot-net/type-link-resolver.md | Documents ExternalTypeInfo and link-generation behaviors/constraints. |
| docs/design/api-mark-dot-net/dot-net-emitter-single-file.md | Documents heading-depth effects, ordering rules, and delegate/nested behavior. |
| docs/design/api-mark-dot-net/dot-net-ast-model.md | Updates immutability/read-only collection interface design narrative. |
| docs/design/api-mark-core/path-helpers.md | Clarifies zero-segment behavior and escape-check algorithm. |
| docs/design/api-mark-core/i-markdown-writer.md | Expands preconditions and error-handling expectations. |
| docs/design/api-mark-core/i-context.md | Normalizes N/A wording. |
| docs/design/api-mark-core/i-api-generator.md | Normalizes N/A wording. |
| docs/design/api-mark-core/i-api-emitter.md | Clarifies parse/emit relationship in data model narrative. |
| docs/design/api-mark-core/glob-file-collector.md | Documents null-guard and literal-file-path behavior. |
| docs/design/api-mark-core/file-markdown-writer-factory.md | New design doc for FileMarkdownWriterFactory (purpose, method contracts). |
| docs/design/api-mark-core/emit-config.md | Documents HeadingDepth runtime validation. |
| docs/design/api-mark-core.md | Updates role wording and clarifies core dependency on FileSystemGlobbing OTS. |
| .github/standards/design-documentation.md | Clarifies “no version numbers” guidance with examples/allowed identifiers. |
| .github/agents/formal-review.agent.md | Refines ReviewMark context-vs-files review flow and artifact saving steps. |
| .cspell.yaml | Adds domain words to reduce false positives. |
| .config/dotnet-tools.json | Bumps demaconsulting.reviewmark from 1.2.0 to 1.3.0. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
GetRequiredIntArgument now enforces max=3 directly so the subsequent if-HeadingDepth>3 blocks can never be reached. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Paths from Matcher.GetResultsInFullPath already reflect actual on-disk casing, so Ordinal is correct and filesystem-agnostic — no two distinct physical files produce ordinal-equal paths, and case-distinct paths on Linux are not incorrectly collapsed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
TypeNameSimplifier_Simplify_GenericArguments_AreSimplifiedRecursively was referenced as TypeNameSimplifier_GenericArguments_AreSimplifiedRecursively (missing '_Simplify_' segment), causing reqstream to report an unsatisfied requirement. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 219 out of 219 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
src/ApiMark.DotNet/TypeLinkResolver.cs:100
- With nullable reference types enabled, the XML doc now explicitly says
typeRefmay be null and the method has a null guard, but the signature is stillTypeReference typeRef(non-nullable). Consider changing the parameter type toTypeReference?(and updating call sites) so the contract is represented correctly in the type system and you don’t need null-forgiving at call sites.
Add ResolveOnDiskPath helper that uses Directory.GetFiles to resolve the actual filesystem entry name, ensuring literal path keys are casing-consistent with paths returned by Matcher.GetResultsInFullPath. This makes exclusion removal correct on case-insensitive filesystems (Windows/macOS) when the caller-supplied casing differs from on-disk. Update design doc to document ResolveOnDiskPath and the Ordinal/casing rationale. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Context now validates --depth in the range 1-6 only (first-principles CLI validation); the format-specific constraint (single-file requires depth <= 3) is enforced in Program.RunToolLogic after all arguments are known. Changes: - Context.cs: widen GetRequiredIntArgument max from 3 to 6; remove the cross-argument SingleFile+depth>3 check from the --format case - EmitConfig.cs: widen HeadingDepth validation to 1-6; update XML doc - Program.cs RunToolLogic: add cross-argument rejection of single-file + depth > 3 with a clear WriteError diagnostic - ContextTests.cs: replace Depth4_Throws test with Depth4_SetsHeadingDepth; remove DepthAbove3AndSingleFileFormat_Throws (moved to program layer) - EmitConfigTests.cs: update AboveMaximum test to use depth 7; add ValueFour_SetsCorrectly confirming the widened range - ProgramTests.cs: add test verifying single-file + depth 4 returns non-zero exit code with --depth in the error message - docs/design and docs/verification: update --depth range to 1-6 in Context and EmitConfig docs; document new program-layer constraint Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… validates 1-6 Context now validates --depth against first principles (1-6: valid ATX heading levels in Markdown). The single-file format constraint (depth <= 3) is enforced post-parse in Program.cs where both --format and --depth are known. This separation ensures Context does not memorize downstream format constraints. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The ValidateDepthForSingleFileFormat requirement belongs to Program (post-parse cross-argument validation), not Context. Context_Create_ WithDepth3AndSingleFileFormat_Succeeds moved to ParseDepthOption. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Guard Directory.Exists before Directory.GetFiles in ResolveOnDiskPath - Update EmitConfig-ValidateHeadingDepthRange requirement to 1-6 range - Remove duplicate Depth6 scenario in context verification doc - Restore exclusion-pattern and non-existent-root coverage in file-system-globbing verification doc and reqstream requirements Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Validation_Run_WithResultsTrxFile_CreatesTrxFile already proves the program can write results to a file; no new Program_Main_* test needed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…cape angle brackets - GlobFileCollector: add inline block comment explaining why StringComparer.Ordinal is correct — all paths in collected carry on-disk casing via Matcher or ResolveOnDiskPath, so Ordinal deduplication is exact on all filesystems - VhdlAstParser: use ReplaceLineEndings to handle CR-only line endings in addition to CRLF and LF - XmlDocReader: escape angle brackets in FormatTypeArity output as \< and \> so generic type params are not parsed as HTML tags in Markdown prose Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This was referenced Jul 6, 2026
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
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.
This pull request updates formal review configuration and documentation to clarify and standardize the use of context files and review-set organization for ReviewMark. It ensures that review-sets have clearly defined scopes, context, and file patterns, and that the documentation and configuration are aligned. The changes also update spell-checking and tooling versions.
Key changes include:
ReviewMark configuration and review-set organization:
.reviewmark.yamlto explicitly define global and per-review-set context files, align review-set IDs and titles with new documentation standards, and ensure each review-set includes the correct context and file paths. The configuration now follows the hierarchical scope and context management principles described in the documentation. [1] [2] [3].github/standards/reviewmark-usage.mdto provide detailed explanations and tables for context file usage, review-set scope, and naming conventions. Each review-set type now specifies required context files, and the documentation offers improved guidance on review-set design and coverage. [1] [2] [3] [4] [5] [6] [7] [8] [9]Documentation standards and review process:
.github/standards/design-documentation.mdthat version numbers should not be recorded in design documentation, except for stable standards and placeholders, to prevent staleness and duplication with SBOMs. [1] [2].github/agents/formal-review.agent.mdto guide reviewers in using context files for orientation and to ensure a consistent review process.Tooling and spell-checking:
demaconsulting.reviewmarktool version in.config/dotnet-tools.jsonfrom 1.2.0 to 1.3.0..cspell.yamlto improve spell-checking accuracy for technical vocabulary. [1] [2] [3]