Fix Var hashing to prevent silent data loss and enable hashability

[masenf]

Type of change

• Bug fix (non-breaking change which fixes an issue)

Description

This PR fixes critical bugs in Var hashing and equality that were causing hooks and imports to be silently dropped from compiled output, and making certain Var types unhashable.

Problems fixed:

1. Silent data loss on var interpolation: When two vars with identical values but different metadata (hooks, imports, dependencies) were interpolated into the same f-string, they would hash to the same value and collide in _global_vars, causing one var's metadata to be silently dropped from the compiled output.

2. VarTypeError in Var.equals(): The Var.equals() method was comparing VarData.deps element-wise, which invoked Var.__eq__(). Since Var.__eq__() returns a BooleanVar rather than a bool, this raised VarTypeError when containers tried to compare Vars.

3. NumberVar and BooleanVar unhashable: Defining __eq__ without explicitly preserving __hash__ made these types unhashable, breaking their use in sets and dicts.

Solution:

• Introduced Var._hash_key() method that returns a hashable tuple containing the var's canonical identity (expression, type, and var data). This key contains no Var objects, making it safe for containers to use.
• Updated Var.__hash__() and Var.equals() to use _hash_key() instead of direct field comparison, ensuring they never invoke Var.__eq__().
• Updated VarData._identity_key to convert deps to their hash keys, preventing Var objects from leaking into the identity tuple.
• Deduplicated deps in VarData.merge() using hash keys to avoid redundant dependencies.
• Cached VarData._identity_key and _cached_hash as properties to avoid recomputation during every var interpolation.
• Removed redundant __hash__ overrides from subclasses (NumberVar, BooleanVar, LiteralVar, etc.) that were inconsistent with the base implementation.
• Explicitly preserved __hash__ on NumberVar to prevent it from becoming unhashable when __eq__ is defined.

Testing

• Added comprehensive unit tests covering:
  • Var.equals() with vars carrying dependencies
  • VarData.merge() deduplication
  • Hash collision prevention for literal vars with different metadata
  • Hash consistency between .to() conversions and originals
  • Hash consistency with equals() across all var types
  • Verification that hash keys contain no Var objects
  • Hashability of NumberVar and BooleanVar
• All new tests pass; existing tests continue to pass.

Changelog

Added news fragment: news/+var-hash-identity.bugfix.md

https://claude.ai/code/session_01UbCysGoqoSYh53PNBGpHZQ

[greptile-apps]

Greptile Summary

This PR aligns Var hashing and structural equality around a metadata-aware identity key so interpolation retains required hooks, imports, and dependencies.

• Adds canonical hash keys for Vars and dependency metadata.
• Deduplicates equivalent Var dependencies without invoking reactive equality.
• Restores numeric and boolean Var hashability and adds regression coverage.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
packages/reflex-base/src/reflex_base/vars/base.py	Centralizes Var equality and hashing around a metadata-aware key, safely compares dependency Vars through their structural keys, and caches immutable VarData identity.
packages/reflex-base/src/reflex_base/vars/number.py	Preserves the inherited Var hash for NumberVar while removing redundant literal-specific hash implementations.
packages/reflex-base/src/reflex_base/vars/color.py	Removes the literal color hash override so hashing follows the canonical Var identity.
packages/reflex-base/src/reflex_base/vars/object.py	Removes the literal object hash override in favor of metadata-aware base hashing.
packages/reflex-base/src/reflex_base/vars/sequence.py	Removes literal sequence and string hash overrides so all affected types use the unified identity contract.
tests/units/test_var.py	Adds regression tests for dependency-safe equality, metadata-sensitive hashes, dependency deduplication, conversion identity, and numeric and boolean hashability.

_{Reviews (2): Last reviewed commit: "test: pin numeric literal hashability as..." | Re-trigger Greptile}

[codspeed-hq]

Merging this PR will not alter performance

✅ 32 untouched benchmarks
⏩ 8 skipped benchmarks¹

---

_{Comparing claude/var-eq-dict-keying-kab2pp (8336f9b) with main (3573364)}

Footnotes

1. 8 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

[cubic-dev-ai]

1 issue found and verified against the latest diff

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/reflex-base/src/reflex_base/vars/number.py">

<violation number="1" location="packages/reflex-base/src/reflex_base/vars/number.py:64">
P2: When two distinct numeric vars have the same Python hash, a set or dict invokes `NumberVar.__eq__` and then raises `VarTypeError` while boolifying its `BooleanVar` result. Do not expose these overloaded-equality objects as native hash keys without a Python-boolean equality path; use a dedicated structural key instead.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[masenf]

CI is red on 8336f9b7 with one failure, and I don't believe it belongs to this PR. Standing down on it rather than pushing a change, with the reasoning below.

Failing check: integration-app-harness (redis, 3.12, 1)
Failing test: tests/integration/test_dynamic_routes.py::test_on_load_navigate_non_dynamic[prod]
Error: selenium.common.exceptions.StaleElementReferenceException: stale element reference: stale element not found in the current frame (chrome 151.0.7922.173). The other 78 tests in the job passed.

Why it isn't this PR's:

1. The same check passed on the previous head. All 111 checks were green on daf6fdf0, including this exact job. The delta from daf6fdf0 to 8336f9b7 is a comment rewrite in vars/number.py and a new unit test in tests/units/test_var.py — nothing executable, and nothing touching tests/integration/ or any runtime path. The code under test is behaviourally identical between the green run and this red one.

2. The one mechanism by which this PR could plausibly affect compiled output doesn't apply. Since perf: deduplicate imports as they merge, not once at the end #7012 landed a fix for irreproducible compiler output, I checked whether this PR's Var.__hash__ change could feed a content hash. It can't: memo names come from md5(str(rendered_chain)) in memoize_helpers.py, and str()/!s route to Var.__str__, which returns _js_expr and never touches the hash. Only Var.__format__ embeds hash(self), and those markers are decoded away within the same compile process. So _global_vars keying stays process-local and compiled output is unaffected.

3. The error is a textbook Selenium race — an element reference captured before a navigation, dereferenced after it — in a test whose whole subject is on-load navigation.

Fix ported: none, because there's nothing to port — this isn't a failure with a known fix elsewhere, and it isn't reproducible from the diff. I have not skipped, disabled, or quarantined the test.

I'm re-running the failed job once to confirm. If it fails again on an unchanged commit I'll treat it as real and dig in rather than re-running further.

---

Generated by Claude Code

[masenf]

Correction to my previous comment: I said I was re-running the failed job. I attempted it and the API refused — 403 Resource not accessible by integration. So the re-run did not happen and the flake is unconfirmed. Apologies for stating it as done.

Since I couldn't re-run, I root-caused the test instead. The race is real and specific — tests/integration/test_dynamic_routes.py::test_on_load_navigate_non_dynamic, lines 353-357:

for _ in range(3):
    link = driver.find_element(By.ID, "link_page_x")
    link.click()
    assert urlsplit(driver.current_url).path.removesuffix("/") == "/static/x"

Every earlier click in this test is wrapped in with poll_for_navigation(driver):, but this final loop is not — presumably because it re-clicks the same link on the same URL, where poll_for_navigation would block waiting for a path change that never comes. That leaves nothing synchronising the next find_element against React's re-render from the previous click, so the returned node can be replaced between the find and the .click(). That is exactly the observed StaleElementReferenceException.

I'm not fixing it in this PR, for two reasons:

1. It's unrelated to this PR's subject (Var hashing) and lives in a different area with a different reviewer audience — hardening it here would widen the PR.
2. I can't reproduce a Selenium timing race in my environment, so any fix I pushed would be speculative. Pushing an unverified timing change to get a red job green is worse than leaving it visible.

The test is not skipped, disabled, or quarantined. I'll keep watching the PR and will re-check this job's state; if it goes green on an unchanged commit that confirms the flake, and if it fails again I'll treat it as real. A maintainer with re-run permission can settle it immediately.

---

Generated by Claude Code