Make bulk tag updates atomic, prune unloadable library rows, and reconcile modal row refs#572
Draft
jschick04 wants to merge 1 commit into
Draft
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR builds on the filter-library modal redesign to harden tag bulk operations, improve SQLite store resilience when encountering corrupt/unrecognized rows, and prevent stale UI row references from accumulating in the modal.
Changes:
- Make bulk tag rename/delete persist via a single
IFilterLibraryStore.UpdateRange(...)transaction and announce affected-entry counts from the effect, with reload + rollback signaling on failure. - Prune unloadable known-kind SQLite rows during
LoadAll()while keeping unknown kinds as forward-version data, plus a systemic-corruption circuit breaker that reports a persistent banner instead of deleting. - Reconcile modal row refs by notifying the modal on
LibraryEntryRowdisposal and pruning row refs against the current entry set.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Unit/EventLogExpert.UI.Tests/FilterLibrary/LibraryEntryRowTests.cs | Adds coverage for row disposal notifications and JS interop disconnect handling during tag edit. |
| tests/Unit/EventLogExpert.UI.Tests/FilterLibrary/FilterLibraryModalTests.cs | Adds coverage for tag-filter resilience, rollback on rename failure, and per-tabpanel DOM id uniqueness. |
| tests/Unit/EventLogExpert.Runtime.Tests/FilterLibrary/FilterLibraryEffectsTests.cs | Updates tests for atomic tag updates via UpdateRange, success/failure announcements, reload behavior, and failure action dispatching. |
| tests/Integration/EventLogExpert.Runtime.IntegrationTests/FilterLibrary/FilterLibrarySqliteStoreTests.cs | Adds integration coverage for unloadable-row deletion rules, systemic corruption circuit breaker + banner, unknown-kind retention, and UpdateRange. |
| tests/Integration/EventLogExpert.Runtime.IntegrationTests/FilterLibrary/FilterLibraryMigrationIntegrationTests.cs | Updates effect construction and store wrapper to include announcement service and UpdateRange. |
| src/EventLogExpert.UI/FilterLibrary/TagManagementPanel.razor.cs | Moves announcements out of the panel and reorders callbacks to allow modal to snapshot optimistic selection before dispatching commands. |
| src/EventLogExpert.UI/FilterLibrary/LibraryEntryRow.razor.cs | Adds OnDisposed callback and swallows JSDisconnectedException during scroll-into-view in tag edit mode. |
| src/EventLogExpert.UI/FilterLibrary/FilterLibraryModal.razor.cs | Adds optimistic tag-selection snapshot/revert on bulk update failure; reconciles stale row refs; filters stale selected tags from affecting tag filtering. |
| src/EventLogExpert.UI/FilterLibrary/FilterLibraryModal.razor | Wires LibraryEntryRow.OnDisposed to modal handler. |
| src/EventLogExpert.Runtime/FilterLibrary/TagBulkUpdateFailedAction.cs | Adds explicit action to signal bulk tag update failure to the UI for rollback. |
| src/EventLogExpert.Runtime/FilterLibrary/IFilterLibraryStore.cs | Adds UpdateRange(...) API for atomic bulk persistence. |
| src/EventLogExpert.Runtime/FilterLibrary/FilterLibrarySqliteStore.cs | Implements UpdateRange transactionally; deletes unloadable known-kind rows on load with systemic-corruption guard and banner reporting; keeps unknown kinds untouched. |
| src/EventLogExpert.Runtime/FilterLibrary/FilterLibraryServiceCollectionExtensions.cs | Wires optional IErrorBannerService into the SQLite store. |
| src/EventLogExpert.Runtime/FilterLibrary/Effects.cs | Refactors tag rename/delete to use bulk persistence, reload behavior, failure signaling, and effect-driven announcements. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
1819edd to
a7689c3
Compare
Collaborator
Author
|
Addressed the performance feedback (force-pushed
Also folded in two related items from review:
Tests: Runtime unit 1194, UI 611, integration 175 — all green. |
a7689c3 to
d82481d
Compare
d82481d to
6666367
Compare
6666367 to
84af813
Compare
…ncile modal row refs
84af813 to
7bf44b2
Compare
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.
Stacked on
jschick/library-modal-redesign(#571) — implements five follow-ups deferred from that PR's review, plus the safety/resilience refinements surfaced during this PR's own review.Changes
Atomic, resilient bulk tag rename/delete. Tag rename and delete now persist all affected entries in a single transaction via a new
IFilterLibraryStore.UpdateRange, instead of per-entry writes. The effect dispatches success only for rows actually written, reloads the library if any row was already gone or the batch failed, and announces the affected-entry count (e.g. "Renamed tag 'X' to 'Y' in 3 entries") — the screen-reader announcement now comes from the effect with the real count. If the persist fails, a failure announcement is made and the tag-filter selection is rolled back to its pre-operation state, so a failed operation never leaves the list filtered to nothing.Clean up unloadable rows on load, with a safety net.
LoadAlldeletes library rows of a known kind whose payload can't be deserialized (logged), so a single corrupt row no longer blocks the rest of the library. Rows of an unknown kind are kept untouched as forward-version data — the kind is checked before any other column is parsed. If an unusually large share of rows are unreadable (a sign of a version mismatch rather than isolated corruption), deletion is withheld entirely and a persistent banner alerts the user, so a systemic problem can't silently delete the whole library.Reconcile stale row references.
LibraryEntryRownotifies the modal when it's disposed, and the modal reconciles its row-reference map against the live entries, so stale references don't accumulate after entries are removed.Tests
New coverage includes atomic/partial-update and failure-rollback paths, unloadable- and unknown-kind row handling, the systemic-corruption circuit breaker, the dispose-notification hook, scroll-interop disconnect handling, per-tab DOM id uniqueness, and tag-filter resilience.
Follow-up
The filter pane has a sibling row-reference/prune pattern that will be addressed in a separate change.