Simplify requirements loading with LoadResult

[Copilot]

Pull Request

Description

Replaces the dual Read/Load API and tuple return type with a single Requirements.Load() that returns a LoadResult sealed class. The result encapsulates the nullable requirements tree, all lint issues, and knows how to route them to the context output — eliminating the repetitive foreach-over-issues pattern at every call site.

LoadResult (new)

• Requirements? Requirements — null when any error-level issue is present
• IReadOnlyList<LintIssue> Issues
• bool HasErrors
• ReportIssues(Context context) — routes warnings to context.WriteLine and errors to context.WriteError

API simplification

• Requirements.Read() removed; Requirements.Load() now returns LoadResult
• RequirementsLoader.Load return type updated from tuple to LoadResult

Call-site before/after

// Before
var (requirements, loadIssues) = Requirements.Load(files);
foreach (var issue in loadIssues)
    context.WriteError(issue.ToString());
if (requirements == null) return;

// After
var result = Requirements.Load(files);
result.ReportIssues(context);
if (result.Requirements == null) return;

Tests

• All Requirements.Read() usages in tests converted to Load() + null/HasErrors checks; test methods renamed from Requirements_Read_* to Requirements_Load_*
• Test file RequirementsReadTests.cs renamed to RequirementsLoadParsingTests.cs to match the class name
• All Requirements.Load(reqPath).Requirements! usages replaced with a safe loadResult + Assert.IsNotNull pattern across TraceMatrixTests.cs, TraceMatrixReadTests.cs, TraceMatrixExportTests.cs, and RequirementsExportTests.cs
• All issue.ToString().Contains(...) assertions in RequirementsLoadParsingTests.cs replaced with issue.Description.Contains(...) for message text and issue.Location.Contains(...) for file paths
• 5 new tests for LoadResult.ReportIssues and HasErrors, using Context to verify routing
• ReportIssues tests use block-scoped using to dispose Context before reading log files, preventing file-in-use errors on Windows

Requirements traceability

• docs/reqstream/modeling/requirements.yaml and docs/reqstream/ots/mstest.yaml updated to reference the renamed Requirements_Load_* test method names

Docs

• docs/design/modeling/requirements.md updated to document LoadResult and the simplified single-method API; both call-site references corrected to result.ReportIssues(context)

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

Requirements.Read() is a breaking removal — callers outside this repo that depended on it must migrate to Requirements.Load() and check result.Requirements / result.HasErrors.

LoadResult is implemented as a sealed class (not a C# record) since value-semantics and with-expressions are not needed for this type.

[Copilot]

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

• https://storage.googleapis.com/chromium-browser-snapshots/Linux_x64/1108766/chrome-linux.zip
  • Triggering command: /home/REDACTED/work/_temp/ghcca-node/node/bin/node node install.js (http block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

[Malcolmnixon]

@copilot When creating new contexts for capturing output, make sure they're disposed before trying to read the log file to prevent file-in-use errors on Windows.

[Copilot]

@copilot When creating new contexts for capturing output, make sure they're disposed before trying to read the log file to prevent file-in-use errors on Windows.

Fixed in b896f98. All three ReportIssues tests now use block-scoped using (var context = ...) and capture context.ExitCode before the block ends, so the context (and its log file writer) is fully disposed before File.ReadAllText is called.

[Copilot]

Pull request overview

This PR unifies requirements parsing/linting behind a single Requirements.Load() API that returns a LoadResult object, and updates call sites/tests/docs to use the new result-centric pattern.

Changes:

• Replaced tuple-based load results (and removed Requirements.Read) with a LoadResult type returned from Requirements.Load().
• Centralized lint issue reporting via LoadResult.ReportIssues(Context) and updated CLI flow accordingly.
• Updated and expanded unit tests to validate LoadResult behavior and new calling patterns; refreshed modeling design documentation.

Reviewed changes

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

Show a summary per file

File	Description
test/DemaConsulting.ReqStream.Tests/Tracing/TraceMatrixTests.cs	Migrates Requirements.Read usage to Requirements.Load(...).Requirements in trace matrix tests.
test/DemaConsulting.ReqStream.Tests/Tracing/TraceMatrixReadTests.cs	Updates trace matrix read tests to the new Load API.
test/DemaConsulting.ReqStream.Tests/Tracing/TraceMatrixExportTests.cs	Updates trace matrix export tests to the new Load API.
test/DemaConsulting.ReqStream.Tests/Modeling/RequirementsReadTests.cs	Renames/repurposes parsing tests to use LoadResult instead of exception-based Read.
test/DemaConsulting.ReqStream.Tests/Modeling/RequirementsLoadTests.cs	Updates existing load tests and adds new tests for LoadResult.HasErrors and ReportIssues.
test/DemaConsulting.ReqStream.Tests/Modeling/RequirementsLoaderTests.cs	Refactors lint helper logic to rely on LoadResult.HasErrors / Issues.
test/DemaConsulting.ReqStream.Tests/Modeling/RequirementsExportTests.cs	Migrates export tests from Read to Load.
src/DemaConsulting.ReqStream/Program.cs	Replaces repetitive issue-foreach loops with result.ReportIssues(context) and uses result.Issues.
src/DemaConsulting.ReqStream/Modeling/RequirementsLoader.cs	Changes loader return type to LoadResult and constructs it based on whether errors exist.
src/DemaConsulting.ReqStream/Modeling/Requirements.cs	Removes Read API and makes Load return LoadResult.
src/DemaConsulting.ReqStream/Modeling/LoadResult.cs	Introduces LoadResult type with Requirements, Issues, HasErrors, and ReportIssues.
docs/design/modeling/requirements.md	Documents the unified Load API and new LoadResult type.

---

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

[Malcolmnixon]

@copilot The build failed with:

Error: Only 71 of 85 requirements are satisfied with tests.
Unsatisfied requirements:
  - ReqStream-Sys-FullPipeline
  - ReqStream-Req-YamlProcessing
  - ReqStream-Req-Validation
  - ReqStream-Req-YamlErrorReporting
  - ReqStream-Req-Hierarchy
  - ReqStream-Req-Includes
  - ReqStream-Req-SectionMerging
  - ReqStream-Req-UniqueIds
  - ReqStream-Req-RequiredTitle
  - ReqStream-Req-ParentChild
  - ReqStream-Req-TestMappings
  - ReqStream-OTS-MSTest
  - ReqStream-Modeling-YamlParsing
  - ReqStream-Modeling-Export

[Copilot]

@copilot The build failed with:

Error: Only 71 of 85 requirements are satisfied with tests.

...

Fixed in b532796. The Requirements_Read_* test method names in docs/reqstream/modeling/requirements.yaml and docs/reqstream/ots/mstest.yaml were still referencing the old names after the methods were renamed to Requirements_Load_*. Updated all 25 references across those two files. Self-validation now passes with all 85 requirements satisfied.

[Malcolmnixon]

@copilot apply changes based on the comments in this thread

[Copilot]

@copilot apply changes based on the comments in this thread

Done in 6d721ce. Applied all the review comments:

• docs: Fixed both occurrences of the stale delegate signature → result.ReportIssues(context)
• test file rename: RequirementsReadTests.cs → RequirementsLoadParsingTests.cs
• NRE-prone Load pattern: Replaced all 51 occurrences of Requirements.Load(reqPath).Requirements! across 4 test files with the loadResult + Assert.IsNotNull pattern
• ToString() coupling: Replaced all issue.ToString().Contains(...) with issue.Description.Contains(...) for message text and issue.Location.Contains(...) for file paths