Skip to content

Apply attribute access rules for Model or SQLModel#5875

Merged
adhami3310 merged 1 commit intomainfrom
masenf/get-attribute-access-sqlmodel
Oct 10, 2025
Merged

Apply attribute access rules for Model or SQLModel#5875
adhami3310 merged 1 commit intomainfrom
masenf/get-attribute-access-sqlmodel

Conversation

@masenf
Copy link
Collaborator

@masenf masenf commented Oct 10, 2025

Fix #5872

@codspeed-hq
Copy link

codspeed-hq bot commented Oct 10, 2025

CodSpeed Performance Report

Merging #5875 will not alter performance

Comparing masenf/get-attribute-access-sqlmodel (fd56159) with main (af33d11)

Summary

✅ 8 untouched

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Greptile Overview

Summary

Extends the get_attribute_access_type function to support attribute access on sqlmodel.SQLModel instances in addition to rx.Model. This fixes the issue where accessing relationship attributes (like profile_sqlmodel_var.user.id) would fail with a VarAttributeError when using sqlmodel.SQLModel directly.

Key changes:

  • Added conditional import of sqlmodel.SQLModel when available
  • Created a tuple sqlmodel_types containing both Model and SQLModel (or just Model if sqlmodel is not installed)
  • Updated the issubclass check on line 496 from only checking Model to checking against sqlmodel_types

How it works:
The function has two code paths for SQLAlchemy models. The first checks DeclarativeBase directly using SQLAlchemy introspection, while the second checks for Model/SQLModel and uses get_type_hints() to extract types from annotations. The second path specifically handles the Mapped[T] wrapper by unwrapping it to get the inner type. This is crucial for sqlmodel.SQLModel classes where relationships are annotated with Mapped[...] types.

Confidence Score: 4/5

  • This PR is safe to merge with low risk
  • The change is minimal, well-scoped, and directly addresses the reported issue. It uses defensive programming by conditionally importing sqlmodel and falling back gracefully. The logic mirrors existing patterns in the codebase for handling Model classes.
  • No files require special attention. Consider adding a test case specifically for sqlmodel.SQLModel relationship access to prevent regression.

Important Files Changed

File Analysis

Filename Score Overview
reflex/utils/types.py 4/5 Extends attribute access type checking to support sqlmodel.SQLModel in addition to rx.Model, enabling relationship attribute access on SQLModel vars

Sequence Diagram

sequenceDiagram
    participant User as User Code
    participant Var as Var.__getattr__
    participant GAT as get_attribute_access_type
    participant SA as SQLAlchemy inspect
    participant TH as get_type_hints
    
    User->>Var: profile_sqlmodel_var.user.id
    Var->>GAT: get_attribute_access_type(ProfileSqlModel, "user")
    
    alt cls is DeclarativeBase
        GAT->>SA: inspect(ProfileSqlModel)
        SA-->>GAT: columns, relationships
        Note over GAT: Returns relationship type or column type
    else cls is Model or SQLModel
        GAT->>TH: get_type_hints(ProfileSqlModel)
        TH-->>GAT: {"user": Mapped[User | None]}
        Note over GAT: Unwraps Mapped[T] to T
        GAT-->>Var: User | None
    end
    
    Var->>GAT: get_attribute_access_type(User, "id")
    GAT->>SA: inspect(User)
    SA-->>GAT: id column type
    GAT-->>Var: int
    Var-->>User: Var[int]
Loading

1 file reviewed, no comments

Edit Code Review Agent Settings | Greptile

@adhami3310 adhami3310 merged commit ca1d33a into main Oct 10, 2025
47 checks passed
@adhami3310 adhami3310 deleted the masenf/get-attribute-access-sqlmodel branch October 10, 2025 20:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

access relationship through sqlmodel.SQLModel-typed Var: no attribute 'id' or may have been annotated wrongly.

2 participants