Apply subsystem folder structure to SarifMark (mirrors TemplateDotNetTool PR #81)#84
Conversation
Agent-Logs-Url: https://github.com/demaconsulting/SarifMark/sessions/de046716-8a2b-4854-8520-1cdeff41c086 Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
|
@copilot There are a few issues:
|
…view-set names Agent-Logs-Url: https://github.com/demaconsulting/SarifMark/sessions/5124cd09-8a76-4644-84e1-025cbe9a7967 Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
Addressed in commit
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:
If you need me to access, download, or install something from one of these locations, you can either:
|
Agent-Logs-Url: https://github.com/demaconsulting/SarifMark/sessions/c0ec5818-3858-4c7c-b314-bdabee8c681e Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
|
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:
If you need me to access, download, or install something from one of these locations, you can either:
|
There was a problem hiding this comment.
Pull request overview
This PR aligns SarifMark with a subsystem-oriented structure (mirroring TemplateDotNetTool PR #81), reorganizing source/tests/docs/requirements into consistent subsystem folders and adding system/subsystem design + requirements artifacts to support traceability and ReviewMark review sets.
Changes:
- Reorganize
src/andtest/into subsystem folders (Cli/Sarif/SelfTest/Utilities) and update supporting test suites accordingly. - Restructure
docs/design/anddocs/reqstream/into subsystem/system documents and update rootrequirements.yamlincludes. - Update
.reviewmark.yamlreview sets and paths to reflect the new system/subsystem/unit breakdown.
Reviewed changes
Copilot reviewed 12 out of 38 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/DemaConsulting.SarifMark.Tests/Utilities/PathHelpersTests.cs | Adds/relocates PathHelpers unit tests under Utilities. |
| test/DemaConsulting.SarifMark.Tests/SelfTest/ValidationTests.cs | Adds/relocates self-validation unit tests under SelfTest. |
| test/DemaConsulting.SarifMark.Tests/Sarif/SarifResultsTests.cs | Adds/relocates SARIF parsing/reporting unit tests under Sarif. |
| test/DemaConsulting.SarifMark.Tests/Cli/ContextTests.cs | Adds/relocates CLI Context unit tests under Cli. |
| src/DemaConsulting.SarifMark/Utilities/PathHelpers.cs | Introduces/relocates safe path-combination utility. |
| src/DemaConsulting.SarifMark/SelfTest/Validation.cs | Introduces/relocates self-validation runner and results output. |
| src/DemaConsulting.SarifMark/Sarif/SarifResults.cs | Introduces/relocates SARIF reader + markdown formatter. |
| src/DemaConsulting.SarifMark/Sarif/SarifResult.cs | Introduces/relocates SARIF result record type. |
| src/DemaConsulting.SarifMark/Cli/Context.cs | Introduces/relocates CLI argument parsing/output routing context. |
| requirements.yaml | Updates root includes to new reqstream file layout. |
| docs/reqstream/utilities/unit-path-helpers.yaml | Adds Utilities unit requirements for PathHelpers. |
| docs/reqstream/utilities/subsystem-utilities.yaml | Adds Utilities subsystem-level requirements. |
| docs/reqstream/unit-program.yaml | Adds Program unit-level requirements under new naming/layout. |
| docs/reqstream/self-test/unit-validation.yaml | Adds Validation unit requirements under SelfTest. |
| docs/reqstream/self-test/subsystem-self-test.yaml | Adds SelfTest subsystem requirements (validation/enforcement). |
| docs/reqstream/sarifmark-system.yaml | Adds system-level requirements and links to integration tests. |
| docs/reqstream/sarif/unit-sarif-results.yaml | Adds SarifResults unit requirements and traceability to tests. |
| docs/reqstream/sarif/unit-sarif-result.yaml | Adds SarifResult unit requirements and traceability to tests. |
| docs/reqstream/sarif/subsystem-sarif.yaml | Adds SARIF subsystem requirements. |
| docs/reqstream/sarif/subsystem-report.yaml | Adds report-generation subsystem requirements. |
| docs/reqstream/platform-requirements.yaml | Adds platform/runtime requirements with evidence filters. |
| docs/reqstream/cli/unit-context.yaml | Adds Context unit requirements and test traceability. |
| docs/reqstream/cli/subsystem-cli.yaml | Adds CLI subsystem requirements and test traceability. |
| docs/design/utilities/utilities.md | Adds Utilities subsystem design doc. |
| docs/design/utilities/path-helpers.md | Adds PathHelpers unit design doc. |
| docs/design/system.md | Adds system-level design doc + Mermaid interaction diagram. |
| docs/design/self-test/validation.md | Adds Validation unit design doc. |
| docs/design/self-test/self-test.md | Adds SelfTest subsystem design doc. |
| docs/design/sarif/sarif.md | Adds Sarif subsystem design doc. |
| docs/design/sarif/sarif-results.md | Adds SarifResults unit design doc. |
| docs/design/sarif/sarif-result.md | Adds SarifResult unit design doc. |
| docs/design/introduction.md | Updates introduction with software structure + folder layout. |
| docs/design/definition.yaml | Updates doc build definition with new inputs/paths. |
| docs/design/command-line.md | Removes legacy monolithic CLI design doc (replaced by subsystem docs). |
| docs/design/cli/context.md | Adds CLI Context unit design doc. |
| docs/design/cli/cli.md | Adds CLI subsystem design doc. |
| AGENTS.md | Updates project structure documentation to include docs/design/. |
| .reviewmark.yaml | Updates review sets and paths for system/subsystem/unit structure. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@copilot Some of the requirement "children" links are broken (referring to requirement IDs that don't exist). Please review all requirements and their linking, and make sure:
|
|
@copilot The CI build failed with: |
Agent-Logs-Url: https://github.com/demaconsulting/SarifMark/sessions/3af2c08c-35a2-4ee4-a57a-94ed9b6ff2ad Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
Fixed in commit
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:
If you need me to access, download, or install something from one of these locations, you can either:
|
... This was already fixed in commit
I've confirmed locally with reqstream that neither |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 38 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (3)
docs/reqstream/sarif/subsystem-sarif.yaml:97
SarifMark-Sarif-Requiredis about requiring the--sarifCLI parameter, but itschildrenlink points toSarifMark-SarifResults-ValidatePath, which only validates the file path once provided. The actual enforcement of a missing--sarifparameter occurs inProgram.ProcessSarifAnalysis(and is covered bySarifMark-Program-SarifAnalysis). Consider updating the child link (or relocating this requirement to the CLI/system level) so the hierarchy reflects where the behavior is implemented.
docs/reqstream/sarif/subsystem-report.yaml:27- For
SarifMark-Report-Depth, thechildrenlink now points only toSarifMark-SarifResults-ValidateDepth, which validates the numeric range but doesn’t cover how the depth is made configurable via--report-depth(implemented inContext/SarifMark-Context-ReportDepthParam). Please consider linking to the CLI/unit requirement as well (or moving this requirement to the CLI subsystem) so the trace matches the end-to-end behavior.
docs/reqstream/sarif/subsystem-report.yaml:62 - For
SarifMark-Report-Headings, the hierarchy only referencesSarifMark-SarifResults-ToMarkdownbut the configurability is also provided by the--headingCLI parameter (implemented inContext/SarifMark-Context-HeadingParam). Consider linking that unit requirement (or moving this requirement to the CLI subsystem) to keep the requirement-to-implementation trace accurate.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Reorganizes source, tests, design docs, and requirements into subsystem subdirectories matching the software hierarchy, following the pattern established in TemplateDotNetTool PR #81.
Description
Source & Tests
Context.cs→Cli/,SarifResult.cs+SarifResults.cs→Sarif/,Validation.cs→SelfTest/,PathHelpers.cs→Utilities/(namespaces unchanged)Design Docs (
docs/design/)docs/design/system.md— system-level design document describing overall architecture, subsystem table, entry point execution flow, and a Mermaid flowchart showing subsystem interactionscli/cli.md,self-test/self-test.md,utilities/utilities.md; movedsarif.md→sarif/sarif.mdintroduction.mddefinition.yamlwith newresource-pathentries andinput-filespaths (includingsystem.md)Requirements (
docs/reqstream/)unit-{unit}.yamlconvention:unit-context.yaml,unit-sarif-result.yaml,unit-sarif-results.yaml,unit-validation.yaml,unit-path-helpers.yaml,unit-program.yamlplatform.yaml→platform-requirements.yamlcli/,sarif/,self-test/,utilities/sarifmark-system.yaml— 9 system-level requirements linked to integration testsutilities/subsystem-utilities.yamlrequirements.yamlincludeschildren:links to go strictly "down" in the software hierarchy (subsystem → unit within that subsystem); removed cross-subsystem references toProgram-*andContext-*units from Sarif/SelfTest/Report subsystem requirements; replaced non-existentSarifMark-PathHelpers-TraversalCheckwithPreCombineCheck,RootedCheck, andPostCombineCheck; corrected two non-existent test method names insarifmark-system.yaml(IntegrationTest_SarifFile_ProcessesSarifFile→IntegrationTest_ValidSarifFile_ProcessesSuccessfully,IntegrationTest_ReportFile_GeneratesMarkdownReport→IntegrationTest_GenerateReport_CreatesReportFile).reviewmark.yamlrequirements.yamltoneeds-review[Product]-[Name]convention (e.g.SarifMark-Cli,SarifMark-Sarif,SarifMark-Context,SarifMark-PathHelpers)SarifMark-System(includesplatform-requirements.yaml,ots-software.yaml,system.md, integration tests),SarifMark-AllRequirements, andSarifMark-Utilitiesreview setsSarifMark-Platform-Subsystem-Review— Platform is not a subsystem; platform and OTS files are now covered underSarifMark-SystemAGENTS.mddesign/folder entry documentingintroduction.mdas the System/Subsystem/Unit breakdown referenceType of Change
Related Issues
Pre-Submission Checklist
Build and Test
dotnet build --configuration Releasedotnet test --configuration Releasedotnet run --project src/DemaConsulting.SarifMark --configuration Release --framework net10.0--no-build -- --validateCode Quality
dotnet format --verify-no-changesQuality Checks
./lint.sh(Unix/macOS) orcmd /c lint.bat/./lint.bat(Windows)Testing
Documentation
Additional Notes
No functional or behavioral changes — this is a pure structural reorganization. All 303 tests pass (101 × net8/net9/net10). CodeQL: 0 alerts. All 102 requirements are satisfied with tests.