refs(#2264): Phase 3 — emit PrefixCacheChange events from FrozenPrefix layer#2576
refs(#2264): Phase 3 — emit PrefixCacheChange events from FrozenPrefix layer#2576encyc wants to merge 2 commits into
Conversation
…nPrefix layer Upgrades the three-zone diagnostic layer from silent tracing::debug! to user-visible Event::PrefixCacheChange events. - First turn: emits 'frozen: <short_id>' event (stability 100%, no change) - Drift detected: emits PrefixDrift description event (stability 0%, changed) - Stable turns: no event (existing PrefixStabilityManager handles this) Phase 2→3: only turn_loop.rs, +18 lines.
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Thanks @encyc for taking the time to contribute. This repository is currently observing a maintainer-managed contribution gate in dry-run mode, so this pull request is staying open. When enforcement is enabled, pull requests from contributors who are not listed in Please read |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a8cfbbf5cc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| changed: true, | ||
| pinned_combined_hash: frozen.hash().to_string(), |
There was a problem hiding this comment.
Don't count frozen-prefix drift as a second change
In normal engine sessions prefix_stability is initialized and the block immediately above this one already sends a PrefixCacheChange for the same turn after check_and_update detects and re-pins a drift. Emitting this second event with changed: true makes the UI handler increment prefix_change_count again and overwrite the displayed stability/hash, so a single system/tool drift is reported in /cache stats as two changes (and this event uses the pre-refreeze hash). Use a distinct event or avoid marking this diagnostic emission as another cache change.
Useful? React with 👍 / 👎.
…freeze The existing PrefixStabilityManager::check_and_update() block already emits PrefixCacheChange on every check (stable + drift). The three-zone layer was re-using the same event type, doubling prefix_change_count and prefix_checks_total. Fix: emit only the one-shot 'frozen: <id>' event on first turn. Drift is still verified and logged (tracing::debug!) but not re-emitted — check_and_update already surfaces the change.
Refs #2264 (Phase 3 — user-visible events).
Summary
Upgrades the three-zone diagnostic layer from silent
tracing::debug!to user-visible
Event::PrefixCacheChangeevents. The TUI footer nowshows three-zone prefix status alongside the existing PrefixStabilityManager
data.
"frozen: a1b2c3d4e5f6"(stability 100%, not a change)"prefix drift: system prompt changed (frozen=..., current=...)"Files
crates/tui/src/core/engine/turn_loop.rsTesting
Greptile Summary
Phase 3 upgrades the three-zone diagnostic layer to emit a one-shot
PrefixCacheChangeevent when the frozen baseline is first established, and correctly silences the duplicate drift event (relying onPrefixStabilityManagerfor drift reporting from that point on).Nonebranch) computespinned_combined_hashviaFrozenPrefix::combined_hash, which hashes tools using fullserde_jsonserialization. All subsequent PM events usePrefixFingerprint::combined_sha256, which hashes only API-visible fields viatool_to_api_json. These two algorithms produce different digests for the same tool set, solast_pinned_prefix_hashsilently flips to a new value on turn 2, making/cache statsoutput inconsistent.prefix_change_countthat would have occurred in Phase 2.Confidence Score: 3/5
The frozen-init event introduces a hash computed with a different tool-serialization algorithm than all subsequent PM events, causing
last_pinned_prefix_hashto show an inconsistent value between turn 1 and turn 2 even when nothing has changed.The
pinned_combined_hashfield in the frozen-init event is computed byFrozenPrefix::combined_hash(fullserde_jsonserialization of every tool field), while every subsequentPrefixCacheChangeevent fromPrefixStabilityManagerusesPrefixFingerprint::combined_sha256(API-only fields viatool_to_api_json). These two digests diverge for the same tool set, so/cache statsshows a hash on turn 1 that never matches what PM reports from turn 2 onward.crates/tui/src/core/engine/turn_loop.rs — the frozen-init event block (None branch, ~line 352) needs
pinned_combined_hashcomputed viaPrefixFingerprint::compute(the same path PM uses) rather thanFrozenPrefix::hash().Important Files Changed
pinned_combined_hashis computed with a different tool-serialization algorithm than all subsequent PM events, causinglast_pinned_prefix_hashto show an inconsistent hash between turn 1 and turn 2.Sequence Diagram
sequenceDiagram participant TL as TurnLoop participant PM as PrefixStabilityManager participant TZ as ThreeZoneLayer participant TX as tx_event participant UI as TUI (ui.rs) Note over TL: Turn 1 (frozen_prefix = None) TL->>PM: check_and_update(system, tools) PM-->>TX: "PrefixCacheChange { changed:false, stability_pct:X%, hash:PM_hash }" TX-->>UI: "prefix_checks_total++, prefix_stability_pct=X%" TL->>TZ: frozen_prefix is None, freeze() TZ-->>TX: "PrefixCacheChange { changed:false, stability_pct:100, hash:FrozenPrefix_hash }" TX-->>UI: "prefix_checks_total++ now 2, prefix_stability_pct=100 overwritten" Note right of UI: PM_hash != FrozenPrefix_hash different tool serialization Note over TL: Turn 2+ stable TL->>PM: check_and_update Ok PM-->>TX: "PrefixCacheChange { changed:false, stability_pct:Y%, hash:PM_hash }" TX-->>UI: "prefix_checks_total++, prefix_stability_pct=Y%" Note over TZ: No event emitted Note over TL: Turn N drift TL->>PM: check_and_update Err PM-->>TX: "PrefixCacheChange { changed:true, stability_pct:Z%, hash:PM_hash }" TX-->>UI: prefix_checks_total++, prefix_change_count++ TL->>TZ: frozen.verify() Err drift TZ->>TZ: tracing debug only, re-freeze silently Note over TZ: No duplicate event Phase 3 fixReviews (2): Last reviewed commit: "fix: don't double-count drift — only emi..." | Re-trigger Greptile