fix(core): sync system prompt hash when overriding via runtime API#1702
fix(core): sync system prompt hash when overriding via runtime API#1702mvanhorn wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a mechanism to synchronize the system prompt hash when an override is present, ensuring that manual overrides are not immediately overwritten by the refresh logic. It refactors system prompt generation into a standalone method and adds unit tests to verify that overrides survive session refreshes. Review feedback suggests simplifying the hash synchronization logic using Option::map and highlights a potential limitation caused by hardcoding AppMode::Agent, which could lead to unexpected refreshes in other modes.
| self.session.last_system_prompt_hash = if self.session.system_prompt.is_some() { | ||
| let stable_prompt = self.stable_system_prompt_for_mode(AppMode::Agent); | ||
| Some(system_prompt_hash(stable_prompt.as_ref())) | ||
| } else { | ||
| Some(system_prompt_hash(None)) | ||
| }; |
There was a problem hiding this comment.
This logic can be simplified using Option::map. Additionally, using None for the last_system_prompt_hash when no override is present is more idiomatic and achieves the same result of forcing a refresh on the next turn (since None != Some(stable_hash) will be true).
Note that hardcoding AppMode::Agent means overrides will be lost if the session is used in Plan or Yolo mode, as refresh_system_prompt will detect a hash mismatch for those modes. Given the current architecture, this is a reasonable default for API-driven syncs, but it remains a limitation for TUI users who might be in a different mode when a sync occurs.
self.session.last_system_prompt_hash = self.session.system_prompt.as_ref().map(|_| {
let stable_prompt = self.stable_system_prompt_for_mode(AppMode::Agent);
system_prompt_hash(stable_prompt.as_ref())
});…t_hash with Option::map Use Option::map to avoid the if-else branch and let the no-override case fall through as None (which compares unequal to any Some(stable_hash) on the next refresh, so the override-clearing semantics are preserved without hashing None explicitly).
|
Addressed in e30e09a -- used Kept Tests: |
|
Thanks for the focused fix and tests. The runtime system_prompt override regression was harvested into v0.8.39 via #1734 and credited in the changelog, so I am closing this PR to keep the queue clean. If the latest release still drops thread-level system_prompt overrides, please reopen with the failing request payload. |
Summary
POST /v1/threadsaccepts asystem_promptfield and stores it on the session, but the nextrefresh_system_promptcall rebuilt the stable baseline and overwrote it becausesession.last_system_prompt_hashwas still the pre-override value. The model request fired with the rebuilt baseline, not the override.Add
sync_session_system_prompt_hashthat recomputeslast_system_prompt_hashafter a thread API override, and call it where the override is installed. Nowrefresh_system_prompt's hash-equality short-circuit fires and the override survives.Factor the stable-prompt construction into
stable_system_prompt_for_modeso the sync and the refresh paths share it.Why this matters
Reporter quote from #1688: "system_prompt on POST /v1/threads is accepted and stored but discarded before the model request." Root cause: hash drift between session.last_system_prompt_hash (pre-override baseline) and the new override. The new tests (
text_system_prompt_override_via_sync_session_survives_refreshand the Blocks variant) lock the regression in.Closes #1688