Fix: Stale file tracking for incremental validation#65
Merged
Conversation
- Add HashSet<Guid> to track files with stale metadata - Add methods: MarkFileAsStale(), IsFileStale(), ClearStaleFlag(), GetStaleFiles() - Auto-clear stale flags when files are removed from batch - Add logging for stale file lifecycle (Debug level) - Update all test constructors to include ILogger<BatchConfiguration> Supports #62
- In MainViewModel.ProcessBatchAsync(), mark successfully processed files as stale - Add stale file tracking methods to IBatchConfiguration interface - Files marked stale will be re-scanned before next validation Supports #62
- In InputViewModel.ImportStorageFilesAsync(), re-scan stale files before validation - Add logging methods for re-scanning operations (Info/Debug/Warning levels) - Freshly scanned metadata replaces stale entries in FileList - Clear stale flags after successful re-scan - Continue processing if individual re-scan fails Supports #62
- Add 5 tests for BatchConfiguration stale tracking methods - Test MarkFileAsStale, ClearStaleFlag, IsFileStale, GetStaleFiles - Test automatic stale flag clearing on file removal - All 159 Core unit tests passing Supports #62
- Use early return pattern in MarkFileAsStale and ClearStaleFlag - Move stale flag cleanup logic into OnFileRemoval method body - Fix OnFileRemoval parameter type to NotifyCollectionChangedEventArgs - Extract RescanStaleFilesAsync as dedicated method - Extract complex LINQ query to variable in MainViewModel - Add clarifying comments and improve code structure
- Add early return pattern with guard clauses - Add preference for ArgumentException.ThrowIf* methods (.NET 6+) - Add LINQ query extraction guidelines - Add reference to Serena code style memory in copilot-instructions.md - Keep both documentation sources synchronized
There was a problem hiding this comment.
Pull request overview
Implements stale file tracking so previously-processed files are re-scanned before validating newly-added files, preventing validation from comparing outdated metadata.
Changes:
- Added stale-file tracking APIs to
IBatchConfigurationand implemented tracking inBatchConfigurationvia aHashSet<Guid>. - Marked successfully processed files as stale after batch processing completes.
- Re-scanned stale files during import (pre-validation) and added unit tests + logging for the new behavior.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/MatroskaBatchFlow.Uno.IntegrationTests/TrackModificationIntegrationTests.cs | Updates integration test to use new BatchConfiguration constructor with logger. |
| tests/MatroskaBatchFlow.Core.UnitTests/Services/BatchConfigurationTests.cs | Adds unit tests covering stale-file tracking APIs and updates ctor usage. |
| src/MatroskaBatchFlow.Uno/Presentation/MainViewModel.cs | Marks successfully processed files as stale after processing. |
| src/MatroskaBatchFlow.Uno/Presentation/InputViewModel.cs | Re-scans stale files before validating newly imported files. |
| src/MatroskaBatchFlow.Uno/Presentation/InputViewModel.Logging.cs | Adds structured logs for stale-file rescan flow. |
| src/MatroskaBatchFlow.Core/Services/IBatchConfiguration.cs | Extends interface with stale-file tracking methods. |
| src/MatroskaBatchFlow.Core/Services/BatchConfiguration.cs | Adds stale tracking storage + implementation and wires stale cleanup on removal. |
| src/MatroskaBatchFlow.Core/Services/BatchConfiguration.Logging.cs | Introduces source-generated logger messages for stale tracking. |
| .serena/memories/code_style_and_conventions.md | Documents preferred control-flow patterns (early returns, LINQ readability, etc.). |
| .github/copilot-instructions.md | Links to the detailed style/conventions document. |
src/MatroskaBatchFlow.Core/Services/BatchConfiguration.Logging.cs
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| protected virtual void OnPropertyChanged(string propertyName) | ||
| private void OnPropertyChanged(string propertyName) |
There was a problem hiding this comment.
OnPropertyChanged was changed from protected virtual to private, which reduces extensibility and is potentially a breaking change if anything derived from BatchConfiguration previously relied on overriding/calling it. If BatchConfiguration is intended to be non-inheritable, sealing the class (consistently across partials) would make this intent explicit; otherwise, consider reverting to protected virtual.
Suggested change
| private void OnPropertyChanged(string propertyName) | |
| protected virtual void OnPropertyChanged(string propertyName) |
- Add PathEquals method to IScannedFileInfoPathComparer interface - Implement PathEquals in ScannedFileInfoPathComparer using existing comparison logic - Inject IScannedFileInfoPathComparer into MainViewModel - Use PathEquals instead of string == for file path matching - Prevents path comparison failures due to case sensitivity or normalization differences
- Batch re-scan all stale files in single ScanAsync call instead of one-by-one - Reduces scanner setup overhead and improves performance for large batches - Inject IScannedFileInfoPathComparer for platform-appropriate path matching - Add logging for files that couldn't be re-scanned (not found/deleted) - Add logging for complete batch re-scan failures - Clear stale flags even on failure to prevent repeated error loops - Improve observability with comprehensive logging at all failure points
- Add MigrateFileConfiguration to transfer config between old/new Guid - Fixes InvalidOperationException after processing files - Preserves user's track modifications (names, flags, etc.) during re-scan - Add comprehensive tests for migration logic (5 new tests, 204 total) - Add logging to migration method for observability
- BatchConfiguration.OnPropertyChanged: protected virtual (can be overridden) - TrackConfiguration.OnPropertyChanged: protected (sealed class, no virtual) - Fixes accidental visibility change from previous commit
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
1 participant
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.
Summary
Fixes #62 - Implements stale file tracking to ensure validation works correctly when files are added incrementally after previous files have been processed.
Problem
When files are processed (modified on disk), their ScannedFileInfo metadata becomes stale. If new files are added to the batch afterward, validation compares stale metadata against fresh metadata, missing real inconsistencies.
Example scenario:
Solution
Implemented automatic re-scanning of processed files:
Breaking Changes
None - backward compatible changes only.