feat: implement Prompt.save() with automatic dirty tracking for name changes #504
feat: implement Prompt.save() with automatic dirty tracking for name changes #504thiagobomfin-galileo wants to merge 2 commits intomainfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #504 +/- ##
==========================================
+ Coverage 82.08% 82.20% +0.12%
==========================================
Files 96 96
Lines 9297 9288 -9
==========================================
+ Hits 7631 7635 +4
+ Misses 1666 1653 -13 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![baz-reviewer[bot]](https://avatars.githubusercontent.com/in/933528?s=60&v=4){kind=small}
| def __setattr__(self, attr_name: str, value: Any) -> None: | ||
| """Track mutations to name and transition to DIRTY state when synced.""" | ||
| if attr_name in self._TRACKED_FIELDS and hasattr(self, "_sync_state") and self._sync_state == SyncState.SYNCED: | ||
| self._set_state(SyncState.DIRTY) | ||
| object.__setattr__(self, attr_name, value) | ||
|
|
There was a problem hiding this comment.
Prompt.setattr flips a synced object's state to DIRTY on any assignment to tracked fields without checking equality, so prompt.name = prompt.name becomes DIRTY and save() (lines 775–788) then calls update(name=self.name), issuing an unnecessary API rename. Can we compare the new value to the current one before flipping the sync state so redundant assignments don't trigger updates?
Finding type: Logical Bugs | Severity: 🟠 Medium
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents:
In src/galileo/__future__/prompt.py around lines 209-214, the __setattr__ method
unconditionally marks tracked fields DIRTY when synced. Change it to first obtain the
current value via object.__getattribute__(self, attr_name) (or use a unique sentinel
with getattr) and compare it to the new value; only call
self._set_state(SyncState.DIRTY) if the values differ. Ensure this comparison handles
the missing-attribute case safely (treat missing as different) and then delegate to
object.__setattr__ to assign the value.
| """ | ||
| self.id = retrieved_prompt.id | ||
| self.name = retrieved_prompt.name | ||
| object.__setattr__(self, "name", retrieved_prompt.name) | ||
|
|
There was a problem hiding this comment.
What's wrong: Both _update_from_api_response and update() now manually call object.__setattr__(self, "name", …) to bypass the new dirty-tracking override when syncing the name. The same three-line pattern appears twice (lines 301‑304 and again around 506‑511), so any future tracked field or additional sync flow will need the same boilerplate.
Impact: We risk forgetting to bypass the overridden __setattr__ in new code paths, which would incorrectly mark the prompt as DIRTY and block save()/update() operations or cause redundant API calls.
Ask: Can we extract a helper (e.g., _set_synced_name(self, name: str) or a generic _assign_tracked_attr(attr, value)) and call it from both _update_from_api_response and update() so the bypass logic lives in one place and isn’t duplicated?
Finding type: Code Dedup and Conventions | Severity: 🟢 Low
Want Baz to fix this for you? Activate Fixer
| # DIRTY or FAILED_SYNC: name was changed, persist it | ||
| if self.id is None: | ||
| raise ValueError("Prompt ID is not set. Cannot update a prompt without an ID.") | ||
| return self.update(name=self.name) |
There was a problem hiding this comment.
Prompt.save() treats any FAILED_SYNC as a name-only dirty state and unconditionally calls self.update(name=self.name) (e.g. after a failed create_version). This can hide the original failure and leave the prompt incorrectly marked SYNCED even though the failed operation never completed. Can we restrict save() to retry only name-related DIRTY changes or record/distinguish the failure source before invoking self.update(name=...)?
Finding type: Logical Bugs | Severity: 🔴 High
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents:
In src/galileo/__future__/prompt.py around lines 785 to 788, the save() method currently
treats FAILED_SYNC the same as DIRTY and unconditionally calls update(name=self.name),
which can mask unrelated failures. Change this logic so that only SyncState.DIRTY
triggers the update retry: if self.sync_state == SyncState.DIRTY, validate self.id and
call return self.update(name=self.name); if self.sync_state == SyncState.FAILED_SYNC, do
not call update — instead re-raise or surface the stored error (e.g., raise the saved
error from the failed sync state or raise a ValueError instructing the caller to retry
the original operation), preserving the original failure information and avoiding
marking the prompt as SYNCED incorrectly.
1 participant
User description
Shortcut:
Implement Missing CRUD Operations in future Package
Description:
object automatically transitions its state to DIRTY
by delegating to the existing update() method
from an API response, bypassing dirty tracking to avoid false DIRTY transitions during internal
syncs
logic, error handling, and state transitions are already in update()
Design decision: name only
Prompt has two mutable attributes that map to different API endpoints:
Only name is tracked for dirty state. Changing messages requires an explicit create_version() call — making version creation an intentional action rather than a silent side effect of save().
Tests:
Generated description
Below is a concise technical summary of the changes proposed in this PR:
Introduce dirty-state tracking for
Promptname assignments so synced objects automatically go DIRTY and API syncs bypass it. ImplementPrompt.save()to delegate DIRTY/FAILED_SYNC persistence toGlobalPromptTemplates.update()while reusing existing state transitions and tests covering failure and ID guards.Latest Contributors(1)