diff --git a/docs/design/review-mark/configuration/review-mark-configuration.md b/docs/design/review-mark/configuration/review-mark-configuration.md index a25c417..27eb6f3 100644 --- a/docs/design/review-mark/configuration/review-mark-configuration.md +++ b/docs/design/review-mark/configuration/review-mark-configuration.md @@ -100,7 +100,10 @@ returns a Markdown document with a heading at `markdownDepth`, a metadata table Fingerprint), an optional Context subsection, and a Files subheading. The Context subsection lists all resolved context files as plain paths (global context files from `GlobalContext` first, followed by per-review-set files from `review.Context`), and is omitted when no -context files resolve. Throws `ArgumentException` +context files resolve. **The files-under-review set is resolved before context so that any +context file that also appears in the review set's `paths:` list is suppressed from the +Context subsection; a file cannot serve both purposes in the same elaboration output.** +Throws `ArgumentException` for unknown IDs; throws `ArgumentOutOfRangeException` when `markdownDepth > 5`. **Coverage note:** Context entries are reference material only. A file listed in `context:` is diff --git a/docs/reqstream/review-mark/configuration/review-mark-configuration.yaml b/docs/reqstream/review-mark/configuration/review-mark-configuration.yaml index e18c4f0..e84d72b 100644 --- a/docs/reqstream/review-mark/configuration/review-mark-configuration.yaml +++ b/docs/reqstream/review-mark/configuration/review-mark-configuration.yaml @@ -240,3 +240,18 @@ sections: - ReviewMarkConfiguration_ElaborateReviewSet_GlobalContext_AppearsInOutput - ReviewMarkConfiguration_ElaborateReviewSet_LocalContext_AppearsInOutput - ReviewMarkConfiguration_ElaborateReviewSet_NoContext_ContextSectionOmitted + + - id: ReviewMark-Config-ContextDeduplication + title: >- + ElaborateReviewSet shall suppress from the Context subsection any file that also + appears in the review set's Files list, so that no file is presented with + conflicting purpose in the same elaboration output. + justification: | + A file listed in global context (e.g. docs/design/introduction.md) is normally + reference material. However, when a review set explicitly includes that same file + in its paths:, it is under review for that set. Showing it in both sections + simultaneously presents contradictory guidance. Suppressing it from context + ensures the file's purpose is unambiguous in every elaboration output. + tests: + - ReviewMarkConfiguration_ElaborateReviewSet_FileInBothContextAndPaths_SuppressedFromContext + - ReviewMarkConfiguration_ElaborateReviewSet_ContextFileNotInPaths_NotSuppressed diff --git a/docs/verification/review-mark/configuration/review-mark-configuration.md b/docs/verification/review-mark/configuration/review-mark-configuration.md index 6bfd879..8fd9937 100644 --- a/docs/verification/review-mark/configuration/review-mark-configuration.md +++ b/docs/verification/review-mark/configuration/review-mark-configuration.md @@ -144,6 +144,21 @@ the context files do not appear in the `Files` subsection. Requirement coverage: `ReviewMark-Config-ContextExcludedFromCoverage`. This scenario is tested by `ReviewMarkConfiguration_ElaborateReviewSet_ContextNotUnderReview`. +**ReviewMarkConfiguration_ElaborateReviewSet_FileInBothContextAndPaths_SuppressedFromContext**: +`ElaborateReviewSet` is called on a configuration where a file is listed in both the global +`context:` list and the review set's `paths:` list. Expected outcome: the file appears in the +`Files` subsection and is absent from the `Context` subsection; the path appears exactly once +as a list item. Requirement coverage: `ReviewMark-Config-ContextDeduplication`. This scenario +is tested by +`ReviewMarkConfiguration_ElaborateReviewSet_FileInBothContextAndPaths_SuppressedFromContext`. + +**ReviewMarkConfiguration_ElaborateReviewSet_ContextFileNotInPaths_NotSuppressed**: +`ElaborateReviewSet` is called on a configuration where a context file is not in the review +set's `paths:` list. Expected outcome: the file appears in the `Context` subsection and is +absent from the `Files` subsection. Requirement coverage: `ReviewMark-Config-ContextDeduplication`. +This scenario is tested by +`ReviewMarkConfiguration_ElaborateReviewSet_ContextFileNotInPaths_NotSuppressed`. + **ReviewMarkConfiguration_PublishReviewPlan_ContextOnlyFile_StillReportedAsUncovered**: `PublishReviewPlan` is called when a file matching `needs-review` appears only in a review set's `context:` list and is not matched by any `paths:` entry. Expected outcome: `HasIssues` is true @@ -233,3 +248,6 @@ list. Requirement coverage: `ReviewMark-Config-Loading`. This scenario is tested ReviewMarkConfiguration_ElaborateReviewSet_GlobalContext_AppearsInOutput, ReviewMarkConfiguration_ElaborateReviewSet_LocalContext_AppearsInOutput, ReviewMarkConfiguration_ElaborateReviewSet_NoContext_ContextSectionOmitted +- **ReviewMark-Config-ContextDeduplication**: + ReviewMarkConfiguration_ElaborateReviewSet_FileInBothContextAndPaths_SuppressedFromContext, + ReviewMarkConfiguration_ElaborateReviewSet_ContextFileNotInPaths_NotSuppressed diff --git a/src/DemaConsulting.ReviewMark/Configuration/ReviewMarkConfiguration.cs b/src/DemaConsulting.ReviewMark/Configuration/ReviewMarkConfiguration.cs index 85c360e..2de3d48 100644 --- a/src/DemaConsulting.ReviewMark/Configuration/ReviewMarkConfiguration.cs +++ b/src/DemaConsulting.ReviewMark/Configuration/ReviewMarkConfiguration.cs @@ -1102,12 +1102,18 @@ internal ElaborateResult ElaborateReviewSet(string reviewSetId, string directory sb.AppendLine($"| Fingerprint | `{fingerprint}` |"); sb.AppendLine(); - // Resolve global and local context files; global files appear first + // Resolve the files-under-review first so we can suppress them from context + var files = review.GetFiles(directory); + var filesUnderReview = new HashSet(files, StringComparer.Ordinal); + + // Resolve global and local context files; global files appear first. + // Any file that is also under review is suppressed from the Context subsection. var globalContextFiles = GlobMatcher.GetMatchingFiles(directory, GlobalContext); var localContextFiles = GlobMatcher.GetMatchingFiles(directory, review.Context); var allContext = globalContextFiles .Concat(localContextFiles) .Distinct(StringComparer.Ordinal) + .Where(f => !filesUnderReview.Contains(f)) .ToList(); // Emit the context subsection only when at least one context file resolves @@ -1127,7 +1133,6 @@ internal ElaborateResult ElaborateReviewSet(string reviewSetId, string directory // Emit the files subsection sb.AppendLine($"{subHeading} Files"); sb.AppendLine(); - var files = review.GetFiles(directory); foreach (var file in files) { sb.AppendLine($"- `{file}`"); diff --git a/test/DemaConsulting.ReviewMark.Tests/Configuration/ReviewMarkConfigurationTests.cs b/test/DemaConsulting.ReviewMark.Tests/Configuration/ReviewMarkConfigurationTests.cs index 1f7f9eb..d1efe7d 100644 --- a/test/DemaConsulting.ReviewMark.Tests/Configuration/ReviewMarkConfigurationTests.cs +++ b/test/DemaConsulting.ReviewMark.Tests/Configuration/ReviewMarkConfigurationTests.cs @@ -1343,6 +1343,101 @@ public void ReviewMarkConfiguration_ElaborateReviewSet_ContextNotUnderReview() Assert.DoesNotContain("`docs/design.md`", filesSection); } + /// + /// Test that ElaborateReviewSet suppresses a context file from the Context subsection + /// when that same file also appears in the review set's paths: list. + /// + [Fact] + public void ReviewMarkConfiguration_ElaborateReviewSet_FileInBothContextAndPaths_SuppressedFromContext() + { + // Arrange — context file in docs/ and source file in src/; global context matches docs/, + // and paths: matches both docs/ and src/ + var srcDir = PathHelpers.SafePathCombine(_testDirectory, "src"); + var docsDir = PathHelpers.SafePathCombine(_testDirectory, "docs"); + Directory.CreateDirectory(srcDir); + Directory.CreateDirectory(docsDir); + File.WriteAllText(PathHelpers.SafePathCombine(srcDir, "A.cs"), "class A {}"); + File.WriteAllText(PathHelpers.SafePathCombine(docsDir, "introduction.md"), "# Introduction"); + + var yaml = """ + context: + - "docs/**/*.md" + evidence-source: + type: none + reviews: + - id: Core-Logic + title: Review of core business logic + paths: + - "docs/**/*.md" + - "src/**/*.cs" + """; + var config = ReviewMarkConfiguration.Parse(yaml); + + // Act + var result = config.ElaborateReviewSet("Core-Logic", _testDirectory); + + // Assert — docs/introduction.md is present in the Files subsection + var filesIndex = result.Markdown.IndexOf("## Files", StringComparison.Ordinal); + var filesSection = result.Markdown[filesIndex..]; + Assert.Contains("- `docs/introduction.md`", filesSection); + + // Assert — docs/introduction.md is absent from the Context subsection + var contextIndex = result.Markdown.IndexOf("## Context", StringComparison.Ordinal); + if (contextIndex >= 0) + { + var contextSection = result.Markdown[contextIndex..filesIndex]; + Assert.DoesNotContain("`docs/introduction.md`", contextSection); + } + + // Assert — the path appears exactly once as a list item (in Files only) + var occurrences = result.Markdown.Split("- `docs/introduction.md`").Length - 1; + Assert.Equal(1, occurrences); + } + + /// + /// Test that ElaborateReviewSet does not suppress a context file from the Context + /// subsection when that file does not appear in the review set's paths: list. + /// + [Fact] + public void ReviewMarkConfiguration_ElaborateReviewSet_ContextFileNotInPaths_NotSuppressed() + { + // Arrange — context file in docs/ and source file in src/; global context matches docs/, + // but paths: matches only src/ + var srcDir = PathHelpers.SafePathCombine(_testDirectory, "src"); + var docsDir = PathHelpers.SafePathCombine(_testDirectory, "docs"); + Directory.CreateDirectory(srcDir); + Directory.CreateDirectory(docsDir); + File.WriteAllText(PathHelpers.SafePathCombine(srcDir, "B.cs"), "class B {}"); + File.WriteAllText(PathHelpers.SafePathCombine(docsDir, "context.md"), "# Context"); + + var yaml = """ + context: + - "docs/**/*.md" + evidence-source: + type: none + reviews: + - id: Core-Logic + title: Review of core business logic + paths: + - "src/**/*.cs" + """; + var config = ReviewMarkConfiguration.Parse(yaml); + + // Act + var result = config.ElaborateReviewSet("Core-Logic", _testDirectory); + + // Assert — docs/context.md appears in the Context subsection + Assert.Contains("## Context", result.Markdown); + var contextIndex = result.Markdown.IndexOf("## Context", StringComparison.Ordinal); + var filesIndex = result.Markdown.IndexOf("## Files", StringComparison.Ordinal); + var contextSection = result.Markdown[contextIndex..filesIndex]; + Assert.Contains("`docs/context.md`", contextSection); + + // Assert — docs/context.md does NOT appear in the Files subsection + var filesSection = result.Markdown[filesIndex..]; + Assert.DoesNotContain("`docs/context.md`", filesSection); + } + /// /// Test that Load returns a lint warning when a top-level needs-review list contains /// a whitespace-only entry, and that the valid pattern is still present in the configuration.