fix: stabilize persisted run transition identities
This commit is contained in:
@@ -117,11 +117,128 @@ pub async fn dispatch_worker_protocol_method(
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use session_store::Store;
|
||||
|
||||
use crate::segment_log_sink::SegmentLogSink;
|
||||
use crate::worker::{LogWriterHandle, SegmentState};
|
||||
|
||||
fn committed_entry(event: Event) -> protocol::SessionSnapshotEntry {
|
||||
match event {
|
||||
Event::SessionEntryCommitted { entry } => entry,
|
||||
other => panic!("expected SessionEntryCommitted, got {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn persisted_run_transitions_keep_identity_across_live_and_snapshot_projection() {
|
||||
let temp = tempfile::tempdir().unwrap();
|
||||
let store = session_store::FsStore::new(temp.path()).unwrap();
|
||||
let session_id = session_store::new_session_id();
|
||||
let segment_id = session_store::new_segment_id();
|
||||
let start = LogEntry::AnnotatedSegmentStart {
|
||||
ts: 1,
|
||||
session_id,
|
||||
system_prompt: None,
|
||||
config: agen::llm_client::RequestConfig::default(),
|
||||
history: Vec::new(),
|
||||
forked_from: None,
|
||||
compacted_from: None,
|
||||
};
|
||||
store
|
||||
.create_segment(session_id, segment_id, std::slice::from_ref(&start))
|
||||
.unwrap();
|
||||
let sink = SegmentLogSink::with_initial(vec![start]);
|
||||
let writer = LogWriterHandle {
|
||||
store: store.clone(),
|
||||
state: SegmentState::new(session_id, segment_id, 1),
|
||||
sink: sink.clone(),
|
||||
in_flight: None,
|
||||
};
|
||||
let (_, mut receiver) = sink.subscribe_with_snapshot();
|
||||
let transitions = [
|
||||
LogEntry::RunYielded {
|
||||
ts: 10,
|
||||
entry_id: None,
|
||||
reason: protocol::RunYieldReason::Compaction,
|
||||
active_run_turn_count: 2,
|
||||
},
|
||||
LogEntry::RunResumed {
|
||||
ts: 10,
|
||||
entry_id: None,
|
||||
source: protocol::RunResumeSource::Compaction,
|
||||
active_run_turn_count: 2,
|
||||
},
|
||||
LogEntry::RunErrored {
|
||||
ts: 10,
|
||||
entry_id: None,
|
||||
interrupted: false,
|
||||
message: "same compaction failure".into(),
|
||||
failure: Some(protocol::RunFailureKind::Compaction),
|
||||
},
|
||||
LogEntry::RunErrored {
|
||||
ts: 10,
|
||||
entry_id: None,
|
||||
interrupted: false,
|
||||
message: "same compaction failure".into(),
|
||||
failure: Some(protocol::RunFailureKind::Compaction),
|
||||
},
|
||||
LogEntry::RunCancelled {
|
||||
ts: 10,
|
||||
entry_id: None,
|
||||
},
|
||||
];
|
||||
|
||||
let mut live_ids = Vec::new();
|
||||
for transition in transitions {
|
||||
writer.append_entry(transition).unwrap();
|
||||
let committed = receiver.try_recv().expect("committed transition broadcast");
|
||||
live_ids.push(
|
||||
committed_entry(live_log_entry_event(committed).expect("live projection")).entry_id,
|
||||
);
|
||||
}
|
||||
assert_eq!(
|
||||
live_ids
|
||||
.iter()
|
||||
.collect::<std::collections::HashSet<_>>()
|
||||
.len(),
|
||||
5
|
||||
);
|
||||
|
||||
let (mirror, _) = sink.subscribe_with_snapshot();
|
||||
let snapshot = session_store::public_snapshot::project_current_session_snapshot(&mirror);
|
||||
let snapshot_ids: Vec<_> = snapshot
|
||||
.entries
|
||||
.iter()
|
||||
.filter(|entry| {
|
||||
matches!(
|
||||
entry.data,
|
||||
protocol::SessionSnapshotEntryData::RunYielded { .. }
|
||||
| protocol::SessionSnapshotEntryData::RunResumed { .. }
|
||||
| protocol::SessionSnapshotEntryData::RunError { .. }
|
||||
| protocol::SessionSnapshotEntryData::RunCancelled
|
||||
)
|
||||
})
|
||||
.map(|entry| entry.entry_id.clone())
|
||||
.collect();
|
||||
assert_eq!(snapshot_ids, live_ids);
|
||||
|
||||
let persisted = store.read_all(session_id, segment_id).unwrap();
|
||||
let persisted_snapshot =
|
||||
session_store::public_snapshot::project_current_session_snapshot(&persisted);
|
||||
let persisted_ids: Vec<_> = persisted_snapshot
|
||||
.entries
|
||||
.iter()
|
||||
.filter(|entry| snapshot_ids.contains(&entry.entry_id))
|
||||
.map(|entry| entry.entry_id.clone())
|
||||
.collect();
|
||||
assert_eq!(persisted_ids, live_ids);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn durable_run_transition_maps_to_session_entry_committed() {
|
||||
let event = live_log_entry_event(LogEntry::RunYielded {
|
||||
ts: 42,
|
||||
entry_id: Some(session_store::LoggedSessionHistoryEntryId::new()),
|
||||
reason: protocol::RunYieldReason::Compaction,
|
||||
active_run_turn_count: 3,
|
||||
})
|
||||
|
||||
@@ -330,6 +330,7 @@ mod tests {
|
||||
// Durable logical-Run transitions share the committed-entry lane.
|
||||
sink.publish(LogEntry::RunResumed {
|
||||
ts: now_millis(),
|
||||
entry_id: None,
|
||||
source: protocol::RunResumeSource::Compaction,
|
||||
active_run_turn_count: 1,
|
||||
});
|
||||
|
||||
@@ -1426,7 +1426,8 @@ where
|
||||
/// Append `entry` to the log: disk write → counter bump → in-memory
|
||||
/// mirror push → broadcast. The Store owns physical write ordering and
|
||||
/// partial-write recovery; publication happens only after it returns Ok.
|
||||
pub fn append_entry(&self, entry: LogEntry) -> Result<(), StoreError> {
|
||||
pub fn append_entry(&self, mut entry: LogEntry) -> Result<(), StoreError> {
|
||||
entry.ensure_session_entry_id();
|
||||
let _append_guard = self
|
||||
.state
|
||||
.append_lock
|
||||
@@ -3055,7 +3056,8 @@ impl<C: LlmClient + 'static, St: Store> Worker<C, St> {
|
||||
/// Append `entry` to the session log AND publish it through the
|
||||
/// broadcast sink. The Store is the commit boundary: a failed write is
|
||||
/// never counted or published.
|
||||
pub(crate) fn commit_entry(&self, entry: LogEntry) -> Result<(), StoreError> {
|
||||
pub(crate) fn commit_entry(&self, mut entry: LogEntry) -> Result<(), StoreError> {
|
||||
entry.ensure_session_entry_id();
|
||||
let loc = self.segment_state.location();
|
||||
self.store.append(loc.session_id, loc.segment_id, &entry)?;
|
||||
self.segment_state.increment_entries();
|
||||
@@ -4707,6 +4709,7 @@ impl<C: LlmClient + 'static, St: Store> Worker<C, St> {
|
||||
.expect("pause/yield resume must retain the logical-run turn budget");
|
||||
self.commit_entry(LogEntry::RunResumed {
|
||||
ts: segment_log::now_millis(),
|
||||
entry_id: None,
|
||||
source,
|
||||
active_run_turn_count,
|
||||
})?;
|
||||
@@ -5024,6 +5027,7 @@ impl<C: LlmClient + 'static, St: Store> Worker<C, St> {
|
||||
// marked as already delivered to clients through the session-log sink.
|
||||
self.commit_entry(LogEntry::RunErrored {
|
||||
ts: segment_log::now_millis(),
|
||||
entry_id: None,
|
||||
interrupted: false,
|
||||
message,
|
||||
failure: Some(RunFailureKind::Compaction),
|
||||
@@ -5435,6 +5439,7 @@ impl<C: LlmClient + 'static, St: Store> Worker<C, St> {
|
||||
{
|
||||
self.commit_entry(LogEntry::RunErrored {
|
||||
ts: segment_log::now_millis(),
|
||||
entry_id: None,
|
||||
interrupted: false,
|
||||
message: message.to_string(),
|
||||
failure: Some(RunFailureKind::Compaction),
|
||||
@@ -5458,6 +5463,7 @@ impl<C: LlmClient + 'static, St: Store> Worker<C, St> {
|
||||
EngineRunExit::Yielded => {
|
||||
self.commit_entry(LogEntry::RunYielded {
|
||||
ts: segment_log::now_millis(),
|
||||
entry_id: None,
|
||||
reason: RunYieldReason::Compaction,
|
||||
active_run_turn_count: active_run_turn_count
|
||||
.expect("yielded logical run must retain its turn budget"),
|
||||
@@ -5474,11 +5480,13 @@ impl<C: LlmClient + 'static, St: Store> Worker<C, St> {
|
||||
EngineRunExit::Interrupted(RunInterruptionReason::Cancelled) => {
|
||||
self.commit_entry(LogEntry::RunCancelled {
|
||||
ts: segment_log::now_millis(),
|
||||
entry_id: None,
|
||||
})?;
|
||||
}
|
||||
EngineRunExit::Interrupted(reason) => {
|
||||
self.commit_entry(LogEntry::RunErrored {
|
||||
ts: segment_log::now_millis(),
|
||||
entry_id: None,
|
||||
interrupted,
|
||||
message: run_interruption_reason_message(reason),
|
||||
failure: Some(RunFailureKind::Engine),
|
||||
@@ -11442,6 +11450,7 @@ mod build_summary_prompt_tests {
|
||||
worker
|
||||
.commit_entry(LogEntry::RunErrored {
|
||||
ts: segment_log::now_millis(),
|
||||
entry_id: None,
|
||||
interrupted: true,
|
||||
message: "provider failed".to_string(),
|
||||
failure: Some(RunFailureKind::Engine),
|
||||
@@ -11472,6 +11481,7 @@ mod build_summary_prompt_tests {
|
||||
worker
|
||||
.commit_entry(LogEntry::RunCancelled {
|
||||
ts: segment_log::now_millis(),
|
||||
entry_id: None,
|
||||
})
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
@@ -11482,6 +11492,7 @@ mod build_summary_prompt_tests {
|
||||
worker
|
||||
.commit_entry(LogEntry::RunResumed {
|
||||
ts: segment_log::now_millis(),
|
||||
entry_id: None,
|
||||
source: RunResumeSource::Compaction,
|
||||
active_run_turn_count: 1,
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user