Mutable attrs bypass _json_string cache, causing stale hashes and exports

[marcorudolphflex]

Problem:
_json_string is cached (tidy3d/components/base.py), but attrs stays mutable. If a model is hashed or serialized before mutating obj.attrs, the cached JSON is reused, so _hash_self() ignores the change and to_hdf5/JSON writes omit the new attrs.

Fixed:

_json_string is cached, but refreshed when attrs refresh

Greptile Overview

Updated On: 2025-10-02 09:22:33 UTC

Summary

This PR addresses a critical caching and hashing bug in `Tidy3dBaseModel` related to mutable `attrs` fields. The core issue was that the `_json_string` property was cached without considering changes to the mutable `attrs` dictionary, leading to stale cached values that could cause incorrect hashes and exports.

The solution introduces a sophisticated dual-caching mechanism:

1. Separated hashing and JSON concerns: A new _json_string_for_hashing property excludes attrs entirely to ensure hash stability, while the regular _json_string property includes attrs with proper cache invalidation.

2. Smart cache invalidation: A new cached_property_guarded decorator monitors changes to attrs through an _attrs_digest() method that creates stable fingerprints of the attrs field using JSON encoding and SHA256 hashing.

3. Consistent export behavior: The to_hdf5() method now uses the appropriate JSON string variant based on whether it's called for hashing purposes.

This change preserves the intended mutability of attrs (allowing obj.attrs['foo'] = bar) while ensuring that object hashes remain consistent for objects with the same core properties, regardless of metadata changes. The JSON serialization still properly reflects the current state including any attrs modifications through the guarded caching mechanism.

Important Files Changed

Changed Files

Filename	Score	Overview
tidy3d/components/base.py	4/5	Implements sophisticated dual-caching mechanism with guarded properties to fix mutable attrs cache invalidation issues
tests/test_components/test_base.py	5/5	Adds comprehensive tests validating hash stability and JSON cache invalidation for mutable attrs
CHANGELOG.md	4/5	Documents the bug fix, though categorized under 'Changed' rather than 'Fixed' section

Confidence score: 4/5

• This PR addresses a well-defined bug with a thoughtful solution that maintains backward compatibility while fixing caching issues
• Score reflects the complexity of the caching mechanism which requires careful review to ensure no edge cases are missed
• The CHANGELOG.md categorization should be reviewed as this appears to be a bug fix rather than a feature change

Sequence Diagram

sequenceDiagram
    participant User
    participant BaseModel
    participant Cache
    participant HashFunction
    
    User->>BaseModel: Create object with attrs
    BaseModel->>Cache: Store initial _json_string
    BaseModel->>HashFunction: Generate _hash_self (excludes attrs)
    
    User->>BaseModel: Modify attrs["foo"] = "new_value"
    BaseModel->>BaseModel: Call _attrs_digest()
    BaseModel->>Cache: Invalidate _json_string cache (key changed)
    BaseModel->>Cache: Generate new _json_string (includes updated attrs)
    
    User->>BaseModel: Call _hash_self()
    BaseModel->>HashFunction: Return same hash (attrs excluded)
    Note over BaseModel,HashFunction: Hash remains stable despite attrs change
    
    User->>BaseModel: Access _json_string
    BaseModel->>Cache: Return updated cached value
    Note over BaseModel,Cache: Cache reflects current attrs state

Loading

Context used:

Rule from dashboard - Use changelog categories correctly: "Fixed" for bug fixes, "Changed" for modifications to existing f... (source)

[greptile-apps]

_{3 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

[github-actions]

Diff Coverage

Diff: origin/develop...HEAD, staged and unstaged changes

• tidy3d/components/base.py (97.1%): Missing lines 260

Summary

• Total: 34 lines
• Missing: 1 line
• Coverage: 97%

tidy3d/components/base.py

  256         """Stable digest of `attrs` using the same JSON encoding rules as pydantic .json()."""
  257         encoders = getattr(self.__config__, "json_encoders", {}) or {}
  258 
  259         def _default(o):
! 260             return custom_pydantic_encoder(encoders, o)
  261 
  262         json_str = json.dumps(
  263             self.attrs,
  264             default=_default,

---

[yaugenst-flex]

Thanks @marcorudolphflex looks great! Just a minor nit on the changelog.

Oh and the commit should be tagged fix:, not feat: