Skip to content

Add design documentation and per-concept requirements files for review sets#71

Merged
Malcolmnixon merged 4 commits into
mainfrom
copilot/add-requirements-and-design-docs
Mar 14, 2026
Merged

Add design documentation and per-concept requirements files for review sets#71
Malcolmnixon merged 4 commits into
mainfrom
copilot/add-requirements-and-design-docs

Conversation

Copilot AI commented Mar 14, 2026

Copy link
Copy Markdown
Contributor

ReviewMark review sets lacked requirements and design documentation, making reviews incomplete. This adds docs/reqstream/ (split requirements YAML) and docs/design/ (design markdown) so each review set contains requirements, design, code, and tests — following the VersionMark pattern. Also adds ValidationTests.cs to provide unit test coverage for the Validation class.

Description

docs/reqstream/ — Per-concept requirements (split from monolithic requirements.yaml)

  • command-line.yamlSarifMark-Cli-*
  • sarif.yamlSarifMark-Sarif-*
  • report.yamlSarifMark-Rpt-*
  • validation.yamlSarifMark-Val-*, SarifMark-Enf-*
  • platform.yamlSarifMark-Plt-* (includes source filter prefix documentation)
  • ots-software.yamlSarifMark-OTS-*

docs/design/ — Design documentation with inline requirement traceability

  • introduction.md — scope, audience, relationship to requirements and code
  • command-line.mdProgram.cs dispatch table, Context.cs property table, argument parser
  • sarif.mdSarifResult / SarifResults reading pipeline, version extraction, ToMarkdown generation
  • utilities.mdPathHelpers.SafePathCombine defence-in-depth validation
  • validation.md — three self-validation tests, WriteResultsFile format dispatch, TemporaryDirectory
  • title.txt, definition.yaml — Pandoc document generation support

requirements.yaml — Refactored to includes: pattern

Source filter prefix documentation moved to docs/reqstream/platform.yaml where it is co-located with the platform requirements that use it.

includes:
  - docs/reqstream/command-line.yaml
  - docs/reqstream/sarif.yaml
  - docs/reqstream/report.yaml
  - docs/reqstream/validation.yaml
  - docs/reqstream/platform.yaml
  - docs/reqstream/ots-software.yaml

.reviewmark.yaml — Restructured from 3 → 5 focused review sets

Each set now lists its reqstream YAML and design markdown alongside source and tests:

Review Set Reqstream Design Tests
SarifMark-CLI-Review command-line.yaml command-line.md ProgramTests.cs, ContextTests.cs
SarifMark-SARIF-Review sarif.yaml, report.yaml sarif.md SarifResultsTests.cs
SarifMark-Validation-Review validation.yaml validation.md ValidationTests.cs
SarifMark-Utilities-Review utilities.md PathHelpersTests.cs
SarifMark-Integration-Review platform.yaml, ots-software.yaml IntegrationTests.cs

test/DemaConsulting.SarifMark.Tests/ValidationTests.cs — New unit tests for Validation

6 unit tests covering Validation.Run:

  • Null context throws ArgumentNullException
  • Valid context prints validation header (version, machine name rows)
  • Valid context runs all three tests and reports them as passed
  • Valid context prints summary with correct totals (3 total, 3 passed, 0 failed)
  • TRX results file is written when --results .trx is supplied
  • JUnit XML results file is written when --results .xml is supplied

Each test disposes the context before reading the log file, ensuring correct behaviour on Windows where file handles are exclusive.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Code quality improvement

Related Issues

Pre-Submission Checklist

Before submitting this pull request, ensure you have completed the following:

Build and Test

  • Code builds successfully: dotnet build --configuration Release
  • All unit tests pass: dotnet test --configuration Release
  • Self-validation tests pass:
    dotnet run --project src/DemaConsulting.SarifMark --configuration Release --framework net10.0
    --no-build -- --validate
  • Code produces zero warnings

Code Quality

  • Code formatting is correct: dotnet format --verify-no-changes
  • New code has appropriate XML documentation comments
  • Static analyzer warnings have been addressed

Quality Checks

Please run the following checks before submitting:

  • Spell checker passes: cspell "**/*.{md,cs}"
  • Markdown linter passes: markdownlint "**/*.md"
  • YAML linter passes: yamllint .
  • Self-validation passes:
    dotnet run --project src/DemaConsulting.SarifMark --configuration Release --framework net10.0 --no-build -- --validate

Testing

  • Added unit tests for new functionality
  • Updated existing tests if behavior changed
  • All tests follow the AAA (Arrange, Act, Assert) pattern
  • Test coverage is maintained or improved

Documentation

  • Updated README.md (if applicable)
  • Updated ARCHITECTURE.md (if applicable)
  • Added code examples for new features (if applicable)
  • Updated requirements.yaml (if applicable)

Additional Notes

All three linters (markdownlint, cspell, yamllint) pass cleanly. ValidationTests.cs passes across net8.0, net9.0, and net10.0 on Linux, macOS, and Windows. CodeQL reports 0 alerts.

Original prompt

This section details on the original issue you should resolve

<issue_title>[Feature]: Design and Requirements Review Sets</issue_title>
<issue_description>### Problem Statement

The ReviewMark review-sets for this project need requirement and design documentation files.

Proposed Solution

The https://github.com/demaconsulting/VersionMark repository demonstrates the approach to organizing the requirements and design documentation:

  • The docs/reqstream folder contains the requirements yaml files split into concepts aligning with the review sets
  • The docs/design folder contains a design document (pandoc + title + markdown files) for the design documentation - with files separated by concepts compatible with the review sets
  • The review-sets of .reviewmark.yaml all contain requirements, design, code, and tests

This approach aligns with https://github.com/demaconsulting/ContinuousCompliance/blob/copilot/expand-reviewmark-assistance-ai/docs/file-reviews.md

Alternatives Considered

No response

Usage Examples

Benefits

The review-sets contain the requirements, design, code, and tests for the concepts in a way that each review-set can be performed with appropriate context.

Priority

Medium - Would improve my workflow

Willingness to Contribute

Yes, I can submit a pull request

Additional Context

No response

Checklist

  • I have searched existing issues to ensure this is not a duplicate
  • I have provided a clear description of the feature
  • I have explained why this feature would be useful</issue_description>

Comments on the Issue (you are @copilot in this section)

Custom agent used: Repo Consistency Agent
Ensures downstream repositories remain consistent with the TemplateDotNetTool template patterns and best practices


📱 Kick off Copilot coding agent tasks wherever you are with GitHub Mobile, available on iOS and Android.

Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
Copilot AI changed the title [WIP] [Feature] Add requirements and design documentation for review sets Add design documentation and per-concept requirements files for review sets Mar 14, 2026
Copilot AI requested a review from Malcolmnixon March 14, 2026 20:18
Comment thread requirements.yaml Outdated
Comment thread .reviewmark.yaml
Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
Copilot AI requested a review from Malcolmnixon March 14, 2026 20:43
@Malcolmnixon Malcolmnixon marked this pull request as ready for review March 14, 2026 20:45
Copilot AI review requested due to automatic review settings March 14, 2026 20:45

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.

Pull request overview

Adds the missing requirements/design artifacts needed for ReviewMark review sets (aligned with the VersionMark pattern) and introduces unit tests for Validation to improve test coverage and traceability.

Changes:

  • Split requirements into per-concept docs/reqstream/*.yaml files and refactor requirements.yaml to use includes:.
  • Add design documentation under docs/design/ with per-conponent descriptions and requirement traceability.
  • Add ValidationTests.cs and restructure .reviewmark.yaml into 5 focused review sets that include reqs + design + code + tests.

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
test/DemaConsulting.SarifMark.Tests/ValidationTests.cs Adds unit tests covering Validation.Run output and results-file writing behavior.
requirements.yaml Refactors monolithic requirements into an includes: list pointing at per-concept reqstream files.
docs/reqstream/command-line.yaml New CLI requirements subset (SarifMark-Cli-*).
docs/reqstream/sarif.yaml New SARIF requirements subset (SarifMark-Sarif-*).
docs/reqstream/report.yaml New reporting requirements subset (SarifMark-Rpt-*).
docs/reqstream/validation.yaml New validation/enforcement requirements subset (SarifMark-Val-, SarifMark-Enf-).
docs/reqstream/platform.yaml New platform requirements subset (SarifMark-Plt-*) including source filter prefix documentation.
docs/reqstream/ots-software.yaml New OTS requirements subset (SarifMark-OTS-*).
docs/design/introduction.md Introduces design-doc scope/audience and relationship to requirements/code.
docs/design/command-line.md Documents CLI dispatch + Context parsing/output behavior with requirement mapping.
docs/design/sarif.md Documents SARIF read/parse/report pipeline and associated requirements.
docs/design/utilities.md Documents PathHelpers.SafePathCombine defense-in-depth behavior.
docs/design/validation.md Documents Validation self-test flow and results-file writing behavior.
docs/design/title.txt Pandoc metadata for design documentation output.
docs/design/definition.yaml Pandoc definition for assembling the design documentation.
.reviewmark.yaml Expands from 3 to 5 review sets and includes requirements + design files alongside code/tests.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment thread docs/design/introduction.md
@Malcolmnixon

Copy link
Copy Markdown
Member

@copilot Got failures on Windows:

Failed!  - Failed:     3, Passed:    90, Skipped:     0, Total:    93, Duration: 2 s - DemaConsulting.SarifMark.Tests.dll (net8.0)
  Failed Validation_Run_ValidContext_PrintsSummary [18 ms]
  Error Message:
   Test method DemaConsulting.SarifMark.Tests.ValidationTests.Validation_Run_ValidContext_PrintsSummary threw exception:
System.IO.IOException: The process cannot access the file 'C:\Users\runneradmin\AppData\Local\Temp\test-a2b75326-52cb-40a3-842d-acc4b1124205.log' because it is being used by another process.
  Stack Trace:
     at Microsoft.Win32.SafeHandles.SafeFileHandle.CreateFile(String fullPath, FileMode mode, FileAccess access, FileShare share, FileOptions options)
   at Microsoft.Win32.SafeHandles.SafeFileHandle.Open(String fullPath, FileMode mode, FileAccess access, FileShare share, FileOptions options, Int64 preallocationSize, Nullable`1 unixCreateMode)
   at System.IO.Strategies.OSFileStreamStrategy..ctor(String path, FileMode mode, FileAccess access, FileShare share, FileOptions options, Int64 preallocationSize, Nullable`1 unixCreateMode)
   at System.IO.Strategies.FileStreamHelpers.ChooseStrategyCore(String path, FileMode mode, FileAccess access, FileShare share, FileOptions options, Int64 preallocationSize, Nullable`1 unixCreateMode)
   at System.IO.StreamReader.ValidateArgsAndOpenPath(String path, Int32 bufferSize)
   at System.IO.File.ReadAllText(String path, Encoding encoding)
   at DemaConsulting.SarifMark.Tests.ValidationTests.Validation_Run_ValidContext_PrintsSummary() in D:\a\SarifMark\SarifMark\test\DemaConsulting.SarifMark.Tests\ValidationTests.cs:line 118
   at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)

Can you investigate and fix please.

Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>

Copilot AI commented Mar 14, 2026

Copy link
Copy Markdown
Contributor Author

@copilot Got failures on Windows:

Failed!  - Failed:     3, Passed:    90, Skipped:     0, Total:    93, Duration: 2 s - DemaConsulting.SarifMark.Tests.dll (net8.0)

...

The root cause was using var context keeping the log file writer open when File.ReadAllText was called — Windows enforces exclusive file access, Linux/macOS don't. Fixed by replacing using var context with a scoped using (...) block so the context (and its log writer) is disposed before reading the file. Applied to all five tests that read the log after Validation.Run. (commit 5098ed7)

@Malcolmnixon Malcolmnixon merged commit dc3b8aa into main Mar 14, 2026
15 checks passed
@Malcolmnixon Malcolmnixon deleted the copilot/add-requirements-and-design-docs branch March 14, 2026 22:06
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.

[Feature]: Design and Requirements Review Sets

3 participants