CODAP-1117: fix MobX constraint violations in DataSet extend block#2401
Draft
CODAP-1117: fix MobX constraint violations in DataSet extend block#2401
Conversation
Replace observable.map cache with observable version counters (per-key and global) paired with a plain Map, so the getter never writes to an observable. This follows the same pattern as observableCachedFnFactory from PR #2377, avoiding the anti-pattern of modifying observables during computed/view evaluation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Simplify cache-miss check (lint fix) and add note about running git commands from the repo root to avoid v3/v3 path errors. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…roups Replace 5 observable boxes with plain variables and a single version counter to eliminate observable writes from within a computed/view. Add invalidateCaseGroups() and invalidateCaseGroupsForNewCases() actions so that the version counter is only incremented from action contexts. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When undo restores deleted cases in multiple batches (insert + append), the first batch triggers invalidateCases() but the second batch's validateCasesForNewItems() would run completeCaseGroups() with incomplete volatile state, signaling observers before data was fully rebuilt. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…Groups When undoing "delete unselected cases", updateCaseGroups() can return an empty newCaseIds array (because groupKeyCaseIds still has old mappings). The APPEND path condition treated [] as truthy, appending nothing instead of falling through to the REBUILD path. Check newCaseIds?.length instead. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The early return in validateCasesForNewItems (which skips additive processing when a full rebuild is pending) means groupKeyCaseIds may not be populated when prepareSnapshot() is called. Adding a validateCases() call in prepareSnapshot() ensures caches are current. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…Cases - Collection: test that invalidateCaseGroupsForNewCases([]) falls through to the REBUILD path instead of the APPEND path (which would append nothing) - DataSet: test that addCases called while isValidCases is false (simulating undo of removeCases) correctly defers to full rebuild via validateCases() Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace 3 observable boxes (_validationCount, _isValidCases, _isValidItemIds) with plain variables and a single _caseValidationVersion observable counter, following the same pattern used in the Collection fix. Move invalidateCases() and invalidateItemIds() from views to actions, wrap selection cleanup in runInAction(), and fix use-rows.ts reaction to maintain MobX dependency tracking when isValidCases is no longer observable. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## CODAP-1117-collection-mobx-compliance #2401 +/- ##
==========================================================================
- Coverage 85.51% 57.18% -28.33%
==========================================================================
Files 753 748 -5
Lines 41521 41443 -78
Branches 10247 10238 -9
==========================================================================
- Hits 35507 23700 -11807
- Misses 6001 17709 +11708
- Partials 13 34 +21
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
kswenson
force-pushed
the
CODAP-1117-collection-mobx-compliance
branch
from
69dc918 to
8f98f5c
Compare
Base automatically changed from
CODAP-1117-collection-mobx-compliance
to
main
February 25, 2026 20:50
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
observable.boxvariables (_validationCount,_isValidCases,_isValidItemIds) in DataSet's.extend()block with plain variables + a single_caseValidationVersionobservable counter (same pattern as Collection's_cacheVersionfix in PR CODAP-1117: fix MobX constraint violation in Collection.completeCaseGroups #2398)invalidateCases()andinvalidateItemIds()from views to actions, eliminating theirrunInAction()wrappers_validateItemIds()by removinguntracked()andrunInAction()— plain variable check/setself.selection.delete()invalidateCases()withrunInAction()sinceselectionis anobservable.setuse-rows.tsreaction to always evaluatevalidationCountso MobX dependency tracking is maintained whenisValidCasesis no longer observableBuilds on PR #2398. Fixes CODAP-1117.
Test plan
🤖 Generated with Claude Code