Allow computed vars to yield events and update state reliably #5990
Conversation
|
One more test done by actual dummy app : |
Greptile OverviewGreptile SummaryThis PR enables computed vars to yield events and modify state during computation. The implementation adds generator detection in Key changes:
Critical issues found:
Confidence Score: 0/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant User
participant EventHandler
participant State
participant ComputedVar
participant Frontend
User->>EventHandler: Trigger event (e.g., set count=5)
EventHandler->>State: Process event
State->>State: _get_resolved_delta()
State->>State: get_delta()
loop Stabilization (max 10 iterations)
State->>State: _mark_dirty_computed_vars()
State->>ComputedVar: get_value(computed_var)
ComputedVar->>ComputedVar: _get_value() executes fget
alt Generator detected
ComputedVar->>ComputedVar: Iterate generator
ComputedVar->>State: Collect yielded events to _computed_var_events
ComputedVar->>State: Return final value from StopIteration
else Regular value
ComputedVar->>State: Return value
end
alt Computed var modifies state
ComputedVar->>State: Modify base vars (e.g., side_effect_value = "Five")
State->>State: Mark modified vars dirty
Note over State: Continue loop if new dirty_vars detected
end
State->>State: Build subdelta with dirty vars
end
State->>State: _collect_computed_var_events()
State-->>EventHandler: Return delta + computed_var_events
EventHandler->>EventHandler: fix_events(computed_var_events)
EventHandler->>Frontend: Send StateUpdate(delta, events)
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| Returns: | ||
| The value of the computed var. | ||
| """ | ||
| print(f"DEBUG: Computing {self._name} for {type(instance).__name__}") |
There was a problem hiding this comment.
syntax: Debug print statement left in production code
| print(f"DEBUG: Computing {self._name} for {type(instance).__name__}") |
Context Used: Rule from dashboard - Remove commented-out code before merging PRs. (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: reflex/vars/base.py
Line: 2446:2446
Comment:
**syntax:** Debug print statement left in production code
```suggestion
```
**Context Used:** Rule from `dashboard` - Remove commented-out code before merging PRs. ([source](https://app.greptile.com/review/custom-context?memory=d49e2a0e-27a4-4cd6-b764-58c8a6fc4032))
How can I resolve this? If you propose a fix, please make it concise.| subdelta: dict[str, Any] = { | ||
| prop + FIELD_MARKER: self.get_value(prop) | ||
| for prop in delta_vars | ||
| if not types.is_backend_base_variable(prop, type(self)) | ||
| } |
There was a problem hiding this comment.
logic: subdelta is recreated in each loop iteration, so only the last iteration's changes will be included in the final delta. This will cause state updates from earlier iterations to be lost.
Move this outside the loop and update it incrementally:
| subdelta: dict[str, Any] = { | |
| prop + FIELD_MARKER: self.get_value(prop) | |
| for prop in delta_vars | |
| if not types.is_backend_base_variable(prop, type(self)) | |
| } | |
| # Only compute delta on first iteration or when there are new dirty vars | |
| if i == 0 or new_dirty_vars: | |
| for prop in delta_vars: | |
| if not types.is_backend_base_variable(prop, type(self)): | |
| subdelta[prop + FIELD_MARKER] = self.get_value(prop) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: reflex/state.py
Line: 2094:2098
Comment:
**logic:** `subdelta` is recreated in each loop iteration, so only the last iteration's changes will be included in the final delta. This will cause state updates from earlier iterations to be lost.
Move this outside the loop and update it incrementally:
```suggestion
# Only compute delta on first iteration or when there are new dirty vars
if i == 0 or new_dirty_vars:
for prop in delta_vars:
if not types.is_backend_base_variable(prop, type(self)):
subdelta[prop + FIELD_MARKER] = self.get_value(prop)
```
How can I resolve this? If you propose a fix, please make it concise.| Returns: | ||
| The delta for the state. | ||
| """ | ||
| delta = {} |
There was a problem hiding this comment.
logic: subdelta needs to be initialized outside the loop to accumulate changes across iterations
| delta = {} | |
| delta = {} | |
| subdelta: dict[str, Any] = {} |
Prompt To Fix With AI
This is a comment left during a code review.
Path: reflex/state.py
Line: 2067:2067
Comment:
**logic:** `subdelta` needs to be initialized outside the loop to accumulate changes across iterations
```suggestion
delta = {}
subdelta: dict[str, Any] = {}
```
How can I resolve this? If you propose a fix, please make it concise.
masenf
left a comment
masenf
left a comment
There was a problem hiding this comment.
this feels a bit hacky, but i'm willing to entertain it for now since my larger event processing refactor will take some more time to be ready.
greptile makes valid comments that should be addressed here, particularly the subdelta one
CodSpeed Performance ReportMerging #5990 will not alter performanceComparing Summary
|
Yeah I agree that got overlooked will try to make a new commit with those getting addressed |
2 participants
Tests
Added
tests/units/test_computed_var_side_effects.pywhich verifies:rx.window_alert).Closes #5956