Constrain review set paths to needs-review file set#83
Merged
Conversation
Review set 'paths' file resolution is now intersected with the needs-review file set. This prevents build artifacts (bin/, obj/) from being included in review sets when broad glob patterns are used, which was causing fingerprints to change on every build and reviews to go spuriously stale. - ReviewSet.GetFiles/GetFingerprint accept an optional constraint set - ReviewMarkConfiguration passes the needs-review set as the constraint in PublishReviewPlan, PublishReviewReport, and ElaborateReviewSet - When needs-review is absent/empty, no constraint is applied - context patterns remain unconstrained (reference material, not files under review) New requirements, design, verification, and user guide documentation added to cover the constraint behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Requirement ReviewMark-Config-PathsConstrainedToNeedsReview: change 'strict subset' to 'subset' (review set may equal full needs-review set) - User guide examples: add tests/**/*.cs to needs-review in two examples so all paths entries are covered under the new constraint - Design review-mark-configuration.md: add GetNeedsReviewFiles() to Key Methods section - Design review-mark.md: correct Load() description; glob resolution occurs in the generation/reporting methods, not Load() Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates ReviewMark’s review-set file resolution and fingerprinting so that, when top-level needs-review patterns are present, each review set is constrained to that resolved needs-review file set (preventing excluded files like build artifacts from affecting membership or fingerprints), while preserving prior behavior when needs-review is absent/empty.
Changes:
- Added optional
constraintparameters toReviewSet.GetFilesandReviewSet.GetFingerprint, and applied the constraint inPublishReviewPlan,PublishReviewReport, andElaborateReviewSet. - Added unit tests covering constrained vs unconstrained behavior and plan/report consistency.
- Updated design docs, user guide, requirements, and verification scenarios to reflect the new constraint semantics.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/DemaConsulting.ReviewMark/Configuration/ReviewMarkConfiguration.cs | Implements optional constraint for review-set file enumeration and fingerprinting; plumbs needs-review constraint through plan/report/elaboration. |
| test/DemaConsulting.ReviewMark.Tests/Configuration/ReviewMarkConfigurationTests.cs | Adds tests validating constrained/unconstrained file resolution, fingerprint stability, and plan/report behavior with needs-review exclusions. |
| docs/verification/review-mark/configuration/review-mark-configuration.md | Documents new verification scenarios and requirement coverage for constraint behavior. |
| docs/user_guide/introduction.md | Explains the “paths are a subset of needs-review” constraint and updates examples. |
| docs/reqstream/review-mark/configuration/review-mark-configuration.yaml | Adds requirements covering constrained behavior and backward compatibility when unconstrained. |
| docs/design/review-mark/configuration/review-mark-configuration.md | Updates API/method documentation to describe constraint flow and new signatures. |
| docs/design/review-mark.md | Clarifies when glob resolution occurs in the overall system flow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This was referenced Jun 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces a significant update to how ReviewMark handles the relationship between the top-level
needs-reviewpatterns and individual review set file resolution. Now, whenneeds-reviewpatterns are present, each review set's file list and fingerprint are constrained to the files declared as needing review, ensuring that excluded files (like build artifacts) cannot inadvertently be included in a review set or affect its fingerprint. The changes also ensure backward compatibility for configurations that do not specifyneeds-review. The documentation, requirements, and verification have all been updated to reflect these changes.Key changes include:
Core Logic and API Changes
ReviewSet.GetFilesandGetFingerprintmethods to accept an optional constraint set. When a constraint is provided (derived fromneeds-review), only files in this set are considered for review set membership and fingerprinting; otherwise, all glob-matched files are included, preserving previous behavior. (src/DemaConsulting.ReviewMark/Configuration/ReviewMarkConfiguration.cs[1] [2]PublishReviewPlan,PublishReviewReport, andElaborateReviewSetto compute the needs-review constraint once and pass it to each review set, ensuring consistent filtering and fingerprinting. (src/DemaConsulting.ReviewMark/Configuration/ReviewMarkConfiguration.cs[1] [2]Documentation Updates
docs/design/review-mark.md[1]docs/design/review-mark/configuration/review-mark-configuration.md[2] [3] [4] [5]docs/user_guide/introduction.md[6] [7] [8]Requirements and Verification
docs/reqstream/review-mark/configuration/review-mark-configuration.yamldocs/reqstream/review-mark/configuration/review-mark-configuration.yamlR276-R319)docs/verification/review-mark/configuration/review-mark-configuration.md[1] [2]