Repository cleanups: PdfParsing requirement, doc updates, test assert comments, README improvements, expanded user guide, required PDF metadata fields

[Copilot]

Eight targeted cleanups: new requirement for PDF metadata parsing, documentation improvements for --dir/--index/self-validation, descriptive assert comments across all test files, a fixed broken link, improved README, a full documentation accuracy review, a significantly expanded user guide, and enforced required date/result fields during PDF evidence parsing.

Description

requirements.yaml — New and updated requirement

Added ReviewMark-Index-PdfParsing requirement covering the Keywords-field parsing behavior during indexing. Updated justification to reflect that all four fields (id, fingerprint, date, result) are required. Linked to six scan tests:

• ReviewIndex_Scan_PdfWithValidMetadata_PopulatesIndex
• ReviewIndex_Scan_PdfWithMissingId_SkipsWithWarning
• ReviewIndex_Scan_PdfWithMissingFingerprint_SkipsWithWarning
• ReviewIndex_Scan_PdfWithMissingDate_SkipsWithWarning
• ReviewIndex_Scan_PdfWithMissingResult_SkipsWithWarning
• ReviewIndex_Scan_PdfWithNoKeywords_SkipsWithWarning

Index.cs — Enforce all four required PDF metadata fields

ProcessPdf now validates all four Keywords fields — id, fingerprint, date, and result. PDFs missing date or result are skipped with a warning, matching the existing behavior for missing id or fingerprint. The optional fallback to empty strings for date/result is removed.

IndexTests.cs — New tests for missing date and result

Added two new tests to cover the new validation behavior:

• ReviewIndex_Scan_PdfWithMissingDate_SkipsWithWarning
• ReviewIndex_Scan_PdfWithMissingResult_SkipsWithWarning

THEORY-OF-OPERATIONS.md

• --dir semantics: Clarified that --dir is the root for both glob-pattern resolution and index.json output; without it, the current working directory is used.
• Multiple --index patterns: Added paragraph + example showing --index can be repeated.
• Self-validation formats: Both TRX (VSTest-compatible) and JUnit XML (.xml) are now documented with separate examples.
• Converted remaining inline links to reference-style links per project style.
• Fixed broken link: [reqstream] reference corrected from the non-existent requirements.md to https://github.com/demaconsulting/ReqStream.
• Updated PDF metadata parsing description to explicitly name all four required fields.

README.md

• Features section: Converted multi-line wrapped bullets to emoji-style single-line bullets (matching BuildMark's format). Status Reporting bullet updated to include Failed status.
• Role in Continuous Compliance: Added new section explaining that ReviewMark fills the file-review evidence role in the Continuous Compliance methodology.
• Self Validation section: Updated sample output and test list to reflect all 7 self-validation tests; updated totals to Total Tests: 7.

docs/guide/guide.md — Significantly expanded

• Self-validation section: Updated to match README — all 7 tests now listed with descriptions; sample output and totals corrected.
• Configuration section (new): Complete annotated .reviewmark.yaml format reference covering all three top-level keys with a full working example.
• Review Sets subsection (new): Field reference table, glob include/exclude pattern explanation, accurate fingerprinting behaviour, and design guidance with a concrete example.
• Evidence Source subsection (new): Both fileshare and url source types with credential configuration.
• Typical Workflow section (new): Six numbered steps walking through the complete end-to-end CI/CD workflow. Step 3 notes that all four metadata fields are required.
• Examples section (expanded): Five realistic examples replacing three sparse off-topic examples.

Test assert comments

Expanded bare // Assert comments to // Assert — <description> across all test files. Normalized // Assert - (hyphen) → // Assert — (em dash) for consistency.

Program_Run_WithVersionFlag_DisplaysVersionOnly

Added missing positive assertion proving the version string is actually emitted.

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

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

All 125 tests pass across .NET 8/9/10.

---

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

[Malcolmnixon]

@copilot I'm not sure why the THEORY-OF-OPERATIONS.md references a 'requirements.md' as the target of the ReqStream link - there is no file - it probably wants to link to the git-repository instead.

Also I think the README.md wants to explain what role ReviewMark fills in Continuous Compliance. I do like the iconic format of the Features table show in https://github.com/demaconsulting/BuildMark as almost a sales-pitch, but that may take shortening the descriptions to fit each bullet on its own line

[Copilot]

@copilot I'm not sure why the THEORY-OF-OPERATIONS.md references a 'requirements.md' as the target of the ReqStream link - there is no file - it probably wants to link to the git-rep...

Fixed in commit 034e401:

• ReqStream link: Updated [reqstream] in THEORY-OF-OPERATIONS.md to point to https://github.com/demaconsulting/ReqStream (was incorrectly referencing the non-existent local requirements.md).

• README.md: Converted the Features list to emoji-style single-line bullets (BuildMark style) and added a new Role in Continuous Compliance section with a table describing the two compliance artifacts ReviewMark produces (Review Plan and Review Report) and how they fit into the broader evidence package.

[Malcolmnixon]

@copilot Is the list of self-validation tests in README.md up to date. In fact it may be better to run the technical-writer agent and have it go through ALL documentation for accuracy.

[Copilot]

@copilot Is the list of self-validation tests in README.md up to date. In fact it may be better to run the technical-writer agent and have it go through ALL documentation for accurac...

The list was indeed out of date — only 2 tests were listed but there are actually 7. Fixed in commit 62e4060: both README.md and docs/guide/guide.md now show all 7 tests (ReviewMark_VersionDisplay, ReviewMark_HelpDisplay, ReviewMark_DefinitionPlan, ReviewMark_DefinitionReport, ReviewMark_IndexScan, ReviewMark_Dir, ReviewMark_Enforce) with updated totals. The rest of the documentation was reviewed and found accurate.

[Malcolmnixon]

@copilot The user guide seems to be fairly sparse - the examples don't even demonstrate how to use the tool for its intended use. It doesn't have any description of the .reviewmark.yaml format. It doesn't have any recommendations on how to create review-sets (grouping related code, tests, and any related markdown files) with examples.

[Copilot]

@copilot The user guide seems to be fairly sparse - the examples don't even demonstrate how to use the tool for its intended use. It doesn't have any description of the .reviewmark.yaml format. It doe...

Expanded the user guide in commit 8625876. The guide now includes:

• Configuration section — complete annotated .reviewmark.yaml format reference covering all three top-level keys (needs-review, evidence-source, reviews) with a full example
• Review Sets subsection — field reference, glob include/exclude pattern explanation, fingerprinting behaviour, and Design Guidance covering cohesive grouping (implementation + tests + docs together), stable IDs, right-sized sets, and excluding generated files — with a concrete realistic example
• Evidence Source subsection — both fileshare and url types with YAML snippets, plus credential configuration via environment variables with a CI pipeline example
• Typical Workflow section — six numbered steps walking through the complete end-to-end flow: create definition → generate plan → perform review → update evidence index → generate report → enforce compliance
• Examples section — replaced the three sparse off-topic examples with five realistic examples demonstrating the primary use case, multiple --index patterns, exclude patterns, and self-validation

[Copilot]

Pull request overview

This PR performs a set of repository cleanups focused on tightening requirements traceability for PDF metadata parsing, improving CLI/user documentation (including expanded user guide content), and polishing test readability (AAA assert comments) plus one missing positive assertion.

Changes:

• Add a new requirement entry for PDF Keywords-field parsing during evidence indexing and link it to existing scan tests.
• Update documentation (README + Theory of Operations + expanded user guide) to clarify --dir / repeated --index usage and self-validation outputs.
• Improve tests by expanding Assert comments for clarity and adding a positive version-output assertion.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
test/DemaConsulting.ReviewMark.Tests/ReviewMarkConfigurationTests.cs	Updates Assert comments for clearer AAA intent.
test/DemaConsulting.ReviewMark.Tests/ProgramTests.cs	Adds a positive assertion for --version output; improves Assert comments.
test/DemaConsulting.ReviewMark.Tests/PathHelpersTests.cs	Updates Assert comments for clearer AAA intent.
test/DemaConsulting.ReviewMark.Tests/IntegrationTests.cs	Updates Assert comments for clearer AAA intent.
test/DemaConsulting.ReviewMark.Tests/ContextTests.cs	Updates Assert comments for clearer AAA intent.
requirements.yaml	Adds a new requirement covering PDF Keywords parsing during indexing.
docs/guide/guide.md	Expands the user guide substantially; updates self-validation test list/output.
THEORY-OF-OPERATIONS.md	Clarifies --dir semantics, repeated --index, and self-validation formats; fixes reqstream link.
README.md	Refreshes features/self-validation sections and adds Continuous Compliance positioning content.

---

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

@copilot Please modify the tool to refuse parsing PDFs (and report warnings) if the date or result are missing. A review that refuses to state its date or whether it passed or failed isn't really a review.

[Copilot]

@copilot Please modify the tool to refuse parsing PDFs (and report warnings) if the date or result are missing. A review that refuses to state its date or whether it passed or failed isn't really a re...

Done in commit 23e7883. ProcessPdf in Index.cs now validates all four fields — id, fingerprint, date, and result — and skips with a warning if any are absent or empty. Two new tests cover the new validation: ReviewIndex_Scan_PdfWithMissingDate_SkipsWithWarning and ReviewIndex_Scan_PdfWithMissingResult_SkipsWithWarning. The requirements.yaml justification and test links are updated accordingly, and both THEORY-OF-OPERATIONS.md and docs/guide/guide.md now explicitly state that all four fields are required. All 125 tests pass.

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

---

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

[Copilot]

The warning text says the PDF Keywords are "missing" the required field, but the condition also triggers when the key is present but the value is empty/whitespace (e.g. date=). Consider updating the warning message to reflect "missing or empty" so it matches the actual skip condition and the XML doc comment.

[Copilot]

This feature bullet says --enforce exits non-zero only for stale/missing review-sets, but the implementation also treats "Failed" review-sets (evidence present for current fingerprint but non-पass result) as issues that trigger non-zero exit when --enforce is set. The README should include "failed" here to match actual behavior.

Suggested change

	- 🚦 **Enforcement** - `--enforce` exits non-zero if any review-set is stale or missing, or any file is uncovered
	- 🚦 **Enforcement** - `--enforce` exits non-zero if any review-set is failed, stale, or missing, or any file is uncovered