CODAP-1117: fix MobX constraint violation in Collection.completeCaseGroups by kswenson · Pull Request #2398 · concord-consortium/codap

kswenson · 2026-02-17T04:26:04Z

Summary

Fixes the MobX constraint violation in Collection.completeCaseGroups() where 5 observable boxes were written from within a computed/view via runInAction
Replaces the 5 observable boxes with plain (non-observable) variables and a single _caseVersion observable counter that is only incremented from action contexts (invalidateCaseGroups, invalidateCaseGroupsForNewCases)
Removes dead code: newCaseIdsForCollections map in DataSet.validateCasesForNewItems() was populated but never read

Fixes CODAP-1117 (in part)

Test plan

All existing Jest tests pass (2127/2127)
TypeScript type check passes
New tests verify invalidation guard scenarios (full invalidation overrides additive; additive appends correctly)
Verify data operations in CODAP: adding/removing cases, grouping/ungrouping, plugin data insertion

🤖 Generated with Claude Code

codecov · 2026-02-17T04:28:51Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.53%. Comparing base (7e3dd37) to head (8f98f5c).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2398      +/-   ##
==========================================
+ Coverage   85.52%   85.53%   +0.01%     
==========================================
  Files         758      758              
  Lines       42157    42169      +12     
  Branches    10439    10048     -391     
==========================================
+ Hits        36054    36069      +15     
+ Misses       6091     6088       -3     
  Partials       12       12

Flag	Coverage Δ
cypress	`69.21% <95.55%> (+0.02%)`	⬆️
jest	`57.32% <100.00%> (+0.26%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

cypress · 2026-02-17T04:30:21Z

codap-v3 Run #10453

Run Properties: Passed #10453 • ca6b8580a9: CODAP-1117: fix MobX constraint violation in Collection.completeCaseGroups (#239...

Project	`codap-v3`
Branch Review	`main`
Run status	`Passed #10453`
Run duration	`05m 21s`
Commit	`ca6b8580a9: CODAP-1117: fix MobX constraint violation in Collection.completeCaseGroups (#239...`
Committer	`Kirk Swenson`
View all properties for this run ↗︎

Test results
Failures	`0`
Flaky	`0`
Pending	`73`
Skipped	`0`
Passing	`300`
View all changes introduced in this branch ↗︎

Copilot

Pull request overview

This PR fixes a MobX constraint violation in Collection.completeCaseGroups() by replacing 5 observable boxes with plain variables and a single observable version counter. The change maintains the same functionality while ensuring MobX constraints are respected by only writing to observables from action contexts.

Changes:

Replaced 5 observable boxes (_caseGroups, _cases, _nonEmptyCases, _caseIdsHash, _caseIdsOrderedHash) with plain variables and a single _cacheVersion observable counter
Added two new action methods (invalidateCaseGroups(), invalidateCaseGroupsForNewCases()) that increment the version counter from action contexts
Removed dead code: newCaseIdsForCollections map that was populated but never read

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
v3/src/models/data/collection.ts	Core refactoring: replaced observable boxes with plain variables + version counter; added invalidation actions; removed `runInAction` from `completeCaseGroups()`
v3/src/models/data/data-set.ts	Added `invalidateCaseGroups()` call in `invalidateCases()`; replaced dead code with `invalidateCaseGroupsForNewCases()` calls
v3/src/models/data/collection.test.ts	Updated test helper to use new invalidation API; added tests for invalidation guard scenarios

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

scytacki

Looks good to me.
I suspect there is some cleaner pattern, but I don't have the time to figure it out now.

scytacki

My last approval didn't seem to go through, trying again.

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>

kswenson added the v3 CODAP v3 label Feb 17, 2026

github-actions bot temporarily deployed to development February 17, 2026 04:29 Inactive

kswenson force-pushed the CODAP-1117-collection-mobx-compliance branch from 2d60331 to 8b22e94 Compare February 17, 2026 04:36

github-actions bot temporarily deployed to development February 17, 2026 04:41 Inactive

kswenson requested a review from Copilot February 17, 2026 04:41

Copilot started reviewing on behalf of kswenson February 17, 2026 04:41 View session

Copilot AI reviewed Feb 17, 2026

View reviewed changes

kswenson added the run regression label Feb 17, 2026

kswenson force-pushed the CODAP-1117-collection-mobx-compliance branch from 8b22e94 to 28cdc31 Compare February 17, 2026 05:22

github-actions bot temporarily deployed to development February 17, 2026 05:26 Inactive

github-actions bot temporarily deployed to development February 17, 2026 15:39 Inactive

kswenson requested a review from Copilot February 17, 2026 15:42

Copilot started reviewing on behalf of kswenson February 17, 2026 15:43 View session

Copilot AI reviewed Feb 17, 2026

View reviewed changes

github-actions bot temporarily deployed to development February 17, 2026 15:54 Inactive

kswenson force-pushed the CODAP-1117-collection-mobx-compliance branch from dc7c097 to 69dc918 Compare February 17, 2026 16:31

github-actions bot temporarily deployed to development February 17, 2026 16:35 Inactive

kswenson requested review from scytacki February 17, 2026 16:50

kswenson mentioned this pull request Feb 17, 2026

CODAP-1117: fix MobX constraint violations in DataSet extend block #2401

Draft

3 tasks

scytacki approved these changes Feb 25, 2026

View reviewed changes

Base automatically changed from CODAP-1117-cachedFnWithArgsFactory to main February 25, 2026 19:37

kswenson and others added 5 commits February 25, 2026 12:26

CODAP-1117: fix lint in cachedFnWithArgsFactory; update CLAUDE.md

763043b

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>

kswenson and others added 2 commits February 25, 2026 12:26

kswenson force-pushed the CODAP-1117-collection-mobx-compliance branch from 69dc918 to 8f98f5c Compare February 25, 2026 20:33

github-actions bot temporarily deployed to development February 25, 2026 20:37 Inactive

kswenson merged commit ca6b858 into main Feb 25, 2026
26 of 27 checks passed

kswenson deleted the CODAP-1117-collection-mobx-compliance branch February 25, 2026 20:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CODAP-1117: fix MobX constraint violation in Collection.completeCaseGroups#2398

CODAP-1117: fix MobX constraint violation in Collection.completeCaseGroups#2398
kswenson merged 7 commits intomainfrom
CODAP-1117-collection-mobx-compliance

kswenson commented Feb 17, 2026 •

edited

Loading

Uh oh!

codecov bot commented Feb 17, 2026 •

edited

Loading

Uh oh!

cypress bot commented Feb 17, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

scytacki left a comment

Uh oh!

scytacki left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

kswenson commented Feb 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

codecov bot commented Feb 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

cypress bot commented Feb 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

codap-v3 Run #10453

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

scytacki left a comment

Choose a reason for hiding this comment

Uh oh!

scytacki left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kswenson commented Feb 17, 2026 •

edited

Loading

codecov bot commented Feb 17, 2026 •

edited

Loading

cypress bot commented Feb 17, 2026 •

edited

Loading