Skip to content

Add ReviewMark integration (review plan, review report, multi-set config, ValidationTests)#58

Merged
Malcolmnixon merged 8 commits intomainfrom
copilot/update-reviewmark-functionality
Mar 12, 2026
Merged

Add ReviewMark integration (review plan, review report, multi-set config, ValidationTests)#58
Malcolmnixon merged 8 commits intomainfrom
copilot/update-reviewmark-functionality

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 12, 2026

Brings in ReviewMark integration, adds ValidationTests.cs, and performs formal code reviews of all three review-sets (TemplateTool-CLI-Review, TemplateTool-Utilities-Review, TemplateTool-Tests-Review) per the code-review-agent checklist.

ReviewMark integration

  • .reviewmark.yaml — three review sets, each pairing source with tests: TemplateTool-CLI-Review (Program.cs, Context.cs + ProgramTests.cs, ContextTests.cs), TemplateTool-Utilities-Review (PathHelpers.cs, Validation.cs + PathHelpersTests.cs, ValidationTests.cs), TemplateTool-Tests-Review (IntegrationTests.cs, Runner.cs, AssemblyInfo.cs)
  • .config/dotnet-tools.json — adds demaconsulting.reviewmark v0.1.0-rc.3
  • .versionmark.yaml — adds reviewmark entry; pre-release regex ((?:-[a-zA-Z0-9.]+)?) applied to all tool patterns
  • .gitignore — excludes generated review-plan.md, review-report.md, and AGENT_REPORT_*.md files
  • .github/agents/code-review-agent.md — agent instructions for performing formal reviews against the review-set IDs

Review plan / report documents

  • docs/reviewplan/, docs/reviewreport/ — Pandoc definition, title, and introduction for each document; introduction wording corrected to "This plan" instead of "This report"
  • .github/workflows/build.yaml — ReviewMark self-validation, plan and report generation (markdown → HTML → PDF) added to build-docs job

ValidationTests

  • test/.../ValidationTests.cs — 6 tests covering null guard, summary output, exit code, TRX output, JUnit XML output, and unsupported format; uses Guid-based temp paths and reads log files only after Context is disposed (prevents Windows file-lock errors under concurrent multi-target runs)
  • requirements.yamlTemplate-OTS-ReviewMark OTS requirement linked to ReviewMark self-validation tests; positive ValidationTests linked to Template-Cmd-Validate and Template-Cmd-Results

TemplateTool-CLI-Review findings (formal checklist — fingerprint 4045175d…)

All checks pass. Two non-blocking observations addressed:

  • ProgramTests: Program_Run_WithVersionFlag_DisplaysVersionOnly lacked a positive assertion — added Assert.Contains(Program.Version, output)
  • ContextTests: Context_Create_NoArguments_ReturnsDefaultContext missing ResultsFile check — added Assert.IsNull(context.ResultsFile)

TemplateTool-Utilities-Review findings (formal checklist — fingerprint df3d06d7…)

All checks pass (CODE-01–06, TEST-01–04, CODEDOC-01–02). Four non-blocking observations recorded in AGENT_REPORT_review-TemplateTool-Utilities-Review.md (excluded from git). No source changes required.

TemplateTool-Tests-Review findings (formal checklist — fingerprint 22353d9a…)

All checks pass (CODE-01–06, TEST-01–04, CODEDOC-01–02). Three non-blocking observations recorded in AGENT_REPORT_review-TemplateTool-Tests-Review.md (excluded from git). One observation addressed:

  • IntegrationTests: GeneratesTrxFile and GeneratesJUnitFile used Path.GetTempFileName() + Path.ChangeExtension(), leaving orphaned .tmp files on disk — switched to Guid-based paths (consistent with ValidationTests.cs)

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.TemplateDotNetTool --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 .

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

The reviews branch (holding review evidence PDFs and index.json) does not yet exist. The --enforce flag on the ReviewMark step should not be added until that branch is populated.

Only positive ValidationTests are linked to requirements; negative/guard tests are intentionally unlinked per project policy.


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

- Add demaconsulting.reviewmark 0.1.0-rc.3 to .config/dotnet-tools.json
- Create .reviewmark.yaml with three review sets (CLI, Utilities, Tests)
- Create docs/reviewplan/ with definition.yaml, title.txt, introduction.md
- Create docs/reviewreport/ with definition.yaml, title.txt, introduction.md
- Add Template-OTS-ReviewMark requirement to requirements.yaml
- Add reviewmark/ReviewMark/reviewplan/reviewreport words to .cspell.json
- Update build.yaml: add reviewmark to tool versions capture, add self-
  validation step, add ReviewMark report generation, add HTML/PDF generation
  steps for review plan and review report
Copilot AI changed the title [WIP] Update review sets with recent improvements from TemplateDotNetLibrary Add ReviewMark integration (review plan, review report, multi-set config) Mar 12, 2026
Copy link
Copy Markdown
Member

@Malcolmnixon Malcolmnixon left a comment

Choose a reason for hiding this comment

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

You seem to have forgotten some changes - such as the new code-review-agent.md file.

Please review demaconsulting/TemplateDotNetLibrary#37 and make sure you picked up everything.

…ersionmark

Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 12, 2026

You seem to have forgotten some changes - such as the new code-review-agent.md file.

Please review demaconsulting/TemplateDotNetLibrary#37 and make sure you picked up everythi...

Done in commit 0277666. Added the missing .github/agents/code-review-agent.md (adapted for TemplateDotNetTool with the correct review-set IDs), updated .versionmark.yaml with the reviewmark entry and updated pre-release regex patterns, and added the generated review plan/report files to .gitignore.

…eviewmark review set

Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
Copilot AI changed the title Add ReviewMark integration (review plan, review report, multi-set config) Add ReviewMark integration (review plan, review report, multi-set config, ValidationTests) Mar 12, 2026
@Malcolmnixon Malcolmnixon marked this pull request as ready for review March 12, 2026 01:21
Copilot AI review requested due to automatic review settings March 12, 2026 01:21
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Introduces ReviewMark into the TemplateDotNetTool repository to generate and publish review plan/report documentation as part of the existing “build-docs” pipeline, and links the new self-validation + CLI validation behaviors into the requirements trace.

Changes:

  • Add ReviewMark configuration (.reviewmark.yaml) with three review-sets aligned to CLI, utilities, and test infrastructure.
  • Extend the build-docs GitHub Actions workflow to run ReviewMark (self-validation + plan/report generation) and to produce HTML/PDF outputs for the new documents.
  • Add ValidationTests and wire positive validation tests + ReviewMark OTS requirement into requirements.yaml.

Reviewed changes

Copilot reviewed 14 out of 15 changed files in this pull request and generated no comments.

Show a summary per file
File Description
test/DemaConsulting.TemplateDotNetTool.Tests/ValidationTests.cs Adds unit coverage for Validation.Run summary output, exit code behavior, and results file generation.
requirements.yaml Links the new validation tests to CLI requirements and adds an OTS requirement for ReviewMark plan/report generation.
docs/reviewplan/title.txt Adds Pandoc title metadata for the generated Review Plan document.
docs/reviewplan/introduction.md Adds Review Plan introduction content used in generated documentation.
docs/reviewplan/definition.yaml Adds Pandoc defaults for rendering the Review Plan to HTML/PDF.
docs/reviewreport/title.txt Adds Pandoc title metadata for the generated Review Report document.
docs/reviewreport/introduction.md Adds Review Report introduction content used in generated documentation.
docs/reviewreport/definition.yaml Adds Pandoc defaults for rendering the Review Report to HTML/PDF.
.reviewmark.yaml Defines review scope patterns, evidence source, and three review-sets for ReviewMark.
.github/agents/code-review-agent.md Adds repo-specific instructions for performing formal review-set reviews using ReviewMark.
.github/workflows/build.yaml Integrates ReviewMark into build-docs: version capture, self-validation, plan/report generation, and HTML/PDF publishing.
.config/dotnet-tools.json Adds the demaconsulting.reviewmark local tool to the tool manifest.
.versionmark.yaml Updates tool version regexes to support pre-release versions and adds ReviewMark version capture.
.cspell.json Adds ReviewMark-related words to the repository spelling dictionary.
.gitignore Ignores generated review plan/report markdown outputs.

💡 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.

@Malcolmnixon
Copy link
Copy Markdown
Member

@copilot The new tests are failing on Windws:

Test method DemaConsulting.TemplateDotNetTool.Tests.ValidationTests.Validation_Run_WithSilentContext_PrintsSummary threw exception:
System.IO.IOException: The process cannot access the file 'C:\Users\runneradmin\AppData\Local\Temp\tmpbfekwv.tmp' because it is being used by another process.

These tests run concurrently multitargeting and so need to be safe from collisions.

…ndows file lock

Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 12, 2026

@copilot The new tests are failing on Windws:

Test method DemaConsulting.TemplateDotNetTool.Tests.ValidationTests.Validation_Run_WithSilentContext_PrintsSummary threw exception:

...

Fixed in commit 8103c77. The test was reading the log file while the Context still held it open (Windows locks the file). Changed to a braced using block so the context is fully disposed (file flushed and closed) before File.ReadAllText is called. Also switched from Path.GetTempFileName() to a Guid-based unique path to eliminate any cross-run collision risk.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

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


💡 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.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@Malcolmnixon
Copy link
Copy Markdown
Member

@copilot Please run the TemplateTool-CLI-Review code-review

…e default-context check

Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 12, 2026

@copilot Please run the TemplateTool-CLI-Review code-review

Done in commit e08e073. The review found two bugs:

  1. ProgramTests.csProgram_Run_WithVersionFlag_DisplaysVersionOnly only had negative assertions ("doesn't show banner"). A broken Run() that returned without writing anything would have passed. Added Assert.Contains(Program.Version, output) so the test actually proves the version is emitted.

  2. ContextTests.csContext_Create_NoArguments_ReturnsDefaultContext checked all properties except ResultsFile. Added Assert.IsNull(context.ResultsFile) to cover the full default state.

No other issues found — all other code paths, error handling, file lifecycle, and test assertions look correct.

@Malcolmnixon
Copy link
Copy Markdown
Member

@copilot Please run the code-review agent to perform the formal review of the TemplateTool-Utilities-Review review-set

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 12, 2026

@copilot Please run the code-review agent to perform the formal review of the TemplateTool-Utilities-Review review-set

Formal review of TemplateTool-Utilities-Review complete. Full report written to AGENT_REPORT_review-TemplateTool-Utilities-Review.md (excluded from git).

Files reviewed (fingerprint df3d06d7…):

  • src/.../PathHelpers.cs
  • src/.../Validation.cs
  • test/.../PathHelpersTests.cs
  • test/.../ValidationTests.cs

2.3 Code Checks

# Check Outcome
CODE-01 Conforms to project coding standards Pass
CODE-02 No obvious security vulnerabilities Pass
CODE-03 Error conditions handled appropriately Pass
CODE-04 No resource leaks Pass
CODE-05 No hardcoded configurable values Pass
CODE-06 No debug artifacts or commented-out code Pass

2.4 Testing Checks

# Check Outcome
TEST-01 Tests cover happy-path behavior Pass
TEST-02 Tests cover error conditions and boundary cases Pass
TEST-03 Tests are independent and repeatable Pass
TEST-04 Test names clearly describe the behavior Pass

2.8 Code vs Documentation Checks

# Check Outcome
CODEDOC-01 All public APIs and interfaces are documented Pass
CODEDOC-02 Non-obvious algorithms/decisions explained Pass

Overall Outcome: ✅ Pass

No failures. Four non-blocking observations:

  1. OBS-01 (CODE-03): RunVersionTest version check (Split('.').Length >= 3) is semantically weak — Version.TryParse or a semver regex would be more precise.
  2. OBS-02 (CODE-04): TemporaryDirectory.Dispose() silently swallows cleanup exceptions (correct anti-throw-in-Dispose practice, but orphaned dirs go unnoticed in CI).
  3. OBS-03 (TEST-02): Windows absolute-path rejection test only runs under OperatingSystem.IsWindows() — that branch is skipped on Linux/macOS CI agents.
  4. OBS-04 (TEST-01): PrintsSummary only checks label strings exist; a count assertion (e.g., "Passed: 2") would better guard against test-counting regressions.

@Malcolmnixon
Copy link
Copy Markdown
Member

@copilot Please use the code-review agent to run the formal review of the TemplateTool-Tests-Review review-set.

….tmp files (OBS-01 from TemplateTool-Tests-Review)

Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 12, 2026

@copilot Please use the code-review agent to run the formal review of the TemplateTool-Tests-Review review-set.

Formal review of TemplateTool-Tests-Review complete. Full report written to AGENT_REPORT_review-TemplateTool-Tests-Review.md (excluded from git).

Files reviewed (fingerprint 22353d9a…):

  • test/.../AssemblyInfo.cs
  • test/.../Runner.cs
  • test/.../IntegrationTests.cs

2.3 Code Checks

# Check Outcome
CODE-01 Conforms to project coding standards Pass
CODE-02 No obvious security vulnerabilities Pass
CODE-03 Error conditions handled appropriately Pass
CODE-04 No resource leaks Pass (with OBS-01 fix applied)
CODE-05 No hardcoded configurable values Pass
CODE-06 No debug artifacts or commented-out code Pass

2.4 Testing Checks

# Check Outcome
TEST-01 Tests cover happy-path behavior Pass
TEST-02 Tests cover error conditions and boundary cases Pass
TEST-03 Tests are independent and repeatable Pass
TEST-04 Test names clearly describe the behavior Pass

2.8 Code vs Documentation Checks

# Check Outcome
CODEDOC-01 All public APIs and interfaces are documented Pass
CODEDOC-02 Non-obvious algorithms/decisions explained Pass

Overall Outcome: ✅ Pass

OBS-01 (CODE-04) — fixed in commit 05647d9: IntegrationTest_ValidateWithResults_GeneratesTrxFile and IntegrationTest_ValidateWithResults_GeneratesJUnitFile used Path.GetTempFileName() + Path.ChangeExtension(), leaving the original .tmp file on disk. Switched to Guid-based paths (matching the pattern already established in ValidationTests.cs).

Two remaining non-blocking observations (no source changes required):

  • OBS-02 (TEST-01): IntegrationTest_SilentFlag_SuppressesOutput only asserts exit code = 0; doesn't verify output is suppressed.
  • OBS-03 (TEST-01): IntegrationTest_VersionFlag_OutputsVersion lacks a positive assertion that the output contains a version string.

@Malcolmnixon Malcolmnixon merged commit dc3ee67 into main Mar 12, 2026
15 checks passed
@Malcolmnixon Malcolmnixon deleted the copilot/update-reviewmark-functionality branch March 12, 2026 14:03
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.

3 participants