Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@
/// "Core-Logic" and "core-logic" are distinct IDs. Evidence-source type uses OrdinalIgnoreCase
/// because types like "fileshare" and "FILESHARE" are semantically identical.
/// </remarks>
internal static void ValidateReviews(

Check warning on line 366 in src/DemaConsulting.ReviewMark/Configuration/ReviewMarkConfiguration.cs

View workflow job for this annotation

GitHub Actions / Build / Build macos-latest

Refactor this method to reduce its Cognitive Complexity from 21 to the 15 allowed.

Check warning on line 366 in src/DemaConsulting.ReviewMark/Configuration/ReviewMarkConfiguration.cs

View workflow job for this annotation

GitHub Actions / Build / Build macos-latest

Refactor this method to reduce its Cognitive Complexity from 21 to the 15 allowed.

Check warning on line 366 in src/DemaConsulting.ReviewMark/Configuration/ReviewMarkConfiguration.cs

View workflow job for this annotation

GitHub Actions / Build / Build macos-latest

Refactor this method to reduce its Cognitive Complexity from 21 to the 15 allowed.

Check warning on line 366 in src/DemaConsulting.ReviewMark/Configuration/ReviewMarkConfiguration.cs

View workflow job for this annotation

GitHub Actions / Build / Build macos-latest

Refactor this method to reduce its Cognitive Complexity from 21 to the 15 allowed.

Check warning on line 366 in src/DemaConsulting.ReviewMark/Configuration/ReviewMarkConfiguration.cs

View workflow job for this annotation

GitHub Actions / Build / Build macos-latest

Refactor this method to reduce its Cognitive Complexity from 21 to the 15 allowed.

Check warning on line 366 in src/DemaConsulting.ReviewMark/Configuration/ReviewMarkConfiguration.cs

View workflow job for this annotation

GitHub Actions / Build / Build macos-latest

Refactor this method to reduce its Cognitive Complexity from 21 to the 15 allowed.

Check warning on line 366 in src/DemaConsulting.ReviewMark/Configuration/ReviewMarkConfiguration.cs

View workflow job for this annotation

GitHub Actions / Build / Build ubuntu-latest

Refactor this method to reduce its Cognitive Complexity from 21 to the 15 allowed.

Check warning on line 366 in src/DemaConsulting.ReviewMark/Configuration/ReviewMarkConfiguration.cs

View workflow job for this annotation

GitHub Actions / Build / Build ubuntu-latest

Refactor this method to reduce its Cognitive Complexity from 21 to the 15 allowed.

Check warning on line 366 in src/DemaConsulting.ReviewMark/Configuration/ReviewMarkConfiguration.cs

View workflow job for this annotation

GitHub Actions / Build / Build ubuntu-latest

Refactor this method to reduce its Cognitive Complexity from 21 to the 15 allowed.

Check warning on line 366 in src/DemaConsulting.ReviewMark/Configuration/ReviewMarkConfiguration.cs

View workflow job for this annotation

GitHub Actions / Build / Build ubuntu-latest

Refactor this method to reduce its Cognitive Complexity from 21 to the 15 allowed.

Check warning on line 366 in src/DemaConsulting.ReviewMark/Configuration/ReviewMarkConfiguration.cs

View workflow job for this annotation

GitHub Actions / Build / Build ubuntu-latest

Refactor this method to reduce its Cognitive Complexity from 21 to the 15 allowed.

Check warning on line 366 in src/DemaConsulting.ReviewMark/Configuration/ReviewMarkConfiguration.cs

View workflow job for this annotation

GitHub Actions / Build / Build ubuntu-latest

Refactor this method to reduce its Cognitive Complexity from 21 to the 15 allowed.

Check warning on line 366 in src/DemaConsulting.ReviewMark/Configuration/ReviewMarkConfiguration.cs

View workflow job for this annotation

GitHub Actions / Build / Build windows-latest

Refactor this method to reduce its Cognitive Complexity from 21 to the 15 allowed.

Check warning on line 366 in src/DemaConsulting.ReviewMark/Configuration/ReviewMarkConfiguration.cs

View workflow job for this annotation

GitHub Actions / Build / Build windows-latest

Refactor this method to reduce its Cognitive Complexity from 21 to the 15 allowed.

Check warning on line 366 in src/DemaConsulting.ReviewMark/Configuration/ReviewMarkConfiguration.cs

View workflow job for this annotation

GitHub Actions / Build / Build windows-latest

Refactor this method to reduce its Cognitive Complexity from 21 to the 15 allowed.

Check warning on line 366 in src/DemaConsulting.ReviewMark/Configuration/ReviewMarkConfiguration.cs

View workflow job for this annotation

GitHub Actions / Build / Build windows-latest

Refactor this method to reduce its Cognitive Complexity from 21 to the 15 allowed.

Check warning on line 366 in src/DemaConsulting.ReviewMark/Configuration/ReviewMarkConfiguration.cs

View workflow job for this annotation

GitHub Actions / Build / Build windows-latest

Refactor this method to reduce its Cognitive Complexity from 21 to the 15 allowed.

Check warning on line 366 in src/DemaConsulting.ReviewMark/Configuration/ReviewMarkConfiguration.cs

View workflow job for this annotation

GitHub Actions / Build / Build windows-latest

Refactor this method to reduce its Cognitive Complexity from 21 to the 15 allowed.
string filePath,
IReadOnlyList<ReviewSetYaml> reviews,
ICollection<LintIssue> issues)
Expand Down Expand Up @@ -1102,12 +1102,18 @@
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<string>(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
Expand All @@ -1127,7 +1133,6 @@
// Emit the files subsection
sb.AppendLine($"{subHeading} Files");
sb.AppendLine();
var files = review.GetFiles(directory);
foreach (var file in files)
{
sb.AppendLine($"- `{file}`");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1343,6 +1343,101 @@ public void ReviewMarkConfiguration_ElaborateReviewSet_ContextNotUnderReview()
Assert.DoesNotContain("`docs/design.md`", filesSection);
}

/// <summary>
/// Test that ElaborateReviewSet suppresses a context file from the Context subsection
/// when that same file also appears in the review set's paths: list.
/// </summary>
[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);
}

/// <summary>
/// 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.
/// </summary>
[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);
}

/// <summary>
/// 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.
Expand Down
Loading