refactor: use Immer for immutable state updates

[joshribakoff]

Before Submitting This PR

Please confirm you have done the following:

• I have searched existing issues and pull requests (including closed ones) to ensure this isn't a duplicate
• I have read CONTRIBUTING.md

Human Written Description

The old code:

• converted to arrays downstream anyways
• made copies of sets/maps which negates their perf benefits.
• was less readable and more error prone
• failed to maintain immutability, despite jumping through hoops to try to do so

I will backfill missing automated tests, in separate PRs as discussed

See remaining AI generated description below.

Related Issues/Discussions

#525

Testing

Typescript compiles (I'd be confident on this alone, for these small changes).
Rust compiles
Compiled Tauri app runs
GUI manual smoke test seems fine to me.

---

Replace Set/Map with Record types and use Immer's produce() for immutable state updates. This fixes mutation "bugs" where .add()/.set() were mutating state before copying (e.g., new Set(prev.add(id))).

Changes:

• Add immer dependency
• Convert Set to Record<string, true> (sparse hash set pattern)
• Convert Map<K,V> to Record<K,V>
• Use produce() for all state mutations
• Update prop types in child components

🤖 Generated with Claude Code

[joshribakoff]

Not sure what I did wrong here, first time using bun.

[joshribakoff]

This is the only place where I feel I hurt readability, instead of improving it, but I find the original code to not be the most readable. I can make more PRs. We should add the tests first.

[joshribakoff]

🟢 The "correct" old pattern would have been:

const myNewSet = new Set(myOldSet)
// mutate myNewSet

🔴 But instead, the old code did this:

// mutate myOldSet
return new Set(myOldSet)

This is illustrative. This isn't stylistic. This is correctness.

[cjpais]

What I wonder about this: is it possible to just have a similar fix without adding a new dependency here?

I'm okay to add the dependency for what it's worth because I know Immer is fairly common, I just don't want to be adding things unnecessarily if there's smaller changes that would ultimately solve the same problem.

[joshribakoff]

Hey @cjpais there are potentially a few paths forward here:

• Fix the patterns with Set, and add lint rules if they seem error prone (feels heavier).
• ImmerJS (pretty commonly paired with React)
• VueJS avoids immutability altogether, most people don't actually need react. This would be a complete rewrite. Also pretty heavy handed.
• Do nothing -- might be okay, might not (feel free to close the PR ❤️ )
• Switching to a boilerplate heavy immutability pattern (kind of what we have now, but there's also the array/spread patterns): return [...oldArray, 'newItem'], but has the same foot guns (if not more foot guns).

Usually in my experience attempts to resist Immer and continue using immutability can start to lead to ... less than ideal patterns. I can empathize with the pain of adding a dependency though. If immutability or react were abandoned in the future, would this change make the code more portable?

[virenpepper]

ooh. i like this PR. think its pretty close to what i've been working on here #478

not to derail the conversation, but think your insight would be super helpful to get this over the finish line, since ive been relying on zustand

[cjpais]

Thanks for the extra detail @joshribakoff

I'll probably pull this in soon enough

but this is definitely not going to happen

VueJS avoids immutability altogether, most people don't actually need react. This would be a complete rewrite. Also pretty heavy handed.

[cjpais]

@joshribakoff would you mind rebasing the code to main, I removed useModels in favor of a Zustand store while I was doing some other work. I'll pull it in after that

[joshribakoff]

Fair enough on vueJS opposition, just answering what the alternatives are, my preference would also be React (with the caveat that it has footguns and i recommend using immerJs). Ill get this rebased shortly.

[joshribakoff]

It looks like Zuststand, which you've added a dependency on, also uses Immerjs under the hood, if you use certain parts of the lib.

I have resolved the merge conflict with rebase, as requested.

[cjpais]

thank you, merged!