[Repo Assist] fix: PersistentHashMap.containsKey returns true for None/unit/null values

[github-actions]

🤖 This PR was created by Repo Assist, an automated AI assistant.

Fixes the long-standing bug reported in #85: PersistentHashMap.containsKey (and the TransientHashMap counterpart) returned false for keys whose associated value is null at the .NET level — including None, unit, and explicit null.

Root Cause

ContainsKey called INode.find, which returns the stored value as obj. The result was compared to null to detect presence:

else this.root.find(0, hash(key), key) <> null

When the stored value is null (as it is for None, unit, or any null-valued reference type), this comparison yields false even though the key is present.

Fix

INode already had a tryFind method on all node types, returning obj option. Some null is correctly distinguished from None, so we change ContainsKey to use it:

else this.root.tryFind(0, hash(key), key).IsSome

Applied to both PersistentHashMap<'T,'S>.ContainsKey and TransientHashMap<'T,'S>.ContainsKey.

Tests

• Activated the two previously-ptest (pending) tests for None values in both PersistentHashMapTest.fs and TransientHashMapTest.fs.
• Added three new tests to PersistentHashMapTest.fs:
  • containsKey with unit value
  • containsKey after multiple adds mixing None and Some
  • Original "None value in empty map" test (activated)

Trade-offs

tryFind allocates an option discriminated union on every ContainsKey call. In the common case (non-null values), this is a minor allocation cost. The alternative — adding a dedicated containsKey method to INode returning bool — would be more performant but requires more changes; this minimal fix is correct and low-risk.

Test Status

✅ All 710 tests pass (dotnet test — Passed: 710, Skipped: 4, Failed: 0). The skipped tests are pre-existing skips unrelated to this change.

Closes #85

Generated by Repo Assist · ◷

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@30f2254f2a7a944da1224df45d181a3f8faefd0d

[dsyme]

/repo-assist format the code according to AGENTS.md

[dsyme]

/repo-assist This is not the right tradeoff:

tryFind allocates an option discriminated union on every ContainsKey call. In the common case (non-null values), this is a minor allocation cost. The alternative — adding a dedicated containsKey method to INode returning bool — would be more performant but requires more changes; this minimal fix is correct and low-risk.

Instead you should indeed have a dedicated containsKey returning bool. Performance matters for collections!

[github-actions]

Commit pushed: 4cd402e

Generated by Repo Assist

[github-actions]

Commit pushed: 4cd402e

Generated by Repo Assist

[github-actions]

🤖 This is an automated response from Repo Assist.

Done! I've also addressed your performance feedback. I've added a dedicated containsKey: int * int * obj -> bool method to the INode interface and implemented it on all three node types (HashCollisionNode, ArrayNode, BitmapIndexedNode). Both ContainsKey methods now call this.root.containsKey(...) directly — no option allocation.

All 710 tests pass (same as before, 4 pre-existing skips).

Generated by Repo Assist for issue #238 · ◷

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@30f2254f2a7a944da1224df45d181a3f8faefd0d