fix: remove redundant system reminder wrappers
This commit is contained in:
@@ -14,7 +14,7 @@ Workerの状態から純粋に再現可能で、且つ揮発性の無い操作
|
|||||||
|
|
||||||
**禁止**: ターンを跨ぐことができない情報に基づいて、history に記録せずに context だけにコンテンツを差し込むこと。これをやると LLM はそれに反応して生成を行う一方、次以降のターンでhistoryに残らないため、「自分がなぜその発言/tool call をしたか」の根拠が消えるうえ、prompt cache のヒット率も低下させることになる。
|
**禁止**: ターンを跨ぐことができない情報に基づいて、history に記録せずに context だけにコンテンツを差し込むこと。これをやると LLM はそれに反応して生成を行う一方、次以降のターンでhistoryに残らないため、「自分がなぜその発言/tool call をしたか」の根拠が消えるうえ、prompt cache のヒット率も低下させることになる。
|
||||||
|
|
||||||
新しい input を context に乗せたいなら、必ず先に `worker.history` に append して commit すること。`history.json` への永続化はそこから自動的についてくる。Notify / WorkerEvent / `<system-reminder>` 系はこの原則で扱う。
|
新しい input を context に乗せたいなら、必ず先に `worker.history` に append して commit すること。`history.json` への永続化はそこから自動的についてくる。Notify / WorkerEvent / typed `SystemItem` reminder はこの原則で扱う。
|
||||||
また、キャッシュを破壊するタイミングは正確にコントロールされる必要があり、キャッシュ破壊とトークン消費のトレードオフに基づいて慎重に設計されるべきである。
|
また、キャッシュを破壊するタイミングは正確にコントロールされる必要があり、キャッシュ破壊とトークン消費のトレードオフに基づいて慎重に設計されるべきである。
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|||||||
@@ -358,7 +358,7 @@ pub enum Event {
|
|||||||
/// `Method::Run` (kind=`UserSend`), `Method::Notify` (kind=`Notify`),
|
/// `Method::Run` (kind=`UserSend`), `Method::Notify` (kind=`Notify`),
|
||||||
/// `Method::WorkerEvent` re-injection (kind=`WorkerEvent`), and any other
|
/// `Method::WorkerEvent` re-injection (kind=`WorkerEvent`), and any other
|
||||||
/// IDLE-breaking trigger. Mid-run interrupts (e.g. hook output,
|
/// IDLE-breaking trigger. Mid-run interrupts (e.g. hook output,
|
||||||
/// `<system-reminder>` injection that doesn't break IDLE) do not
|
/// typed system reminder insertion that doesn't break IDLE) do not
|
||||||
/// emit `InvokeStart` — they appear as `SystemItem` only.
|
/// emit `InvokeStart` — they appear as `SystemItem` only.
|
||||||
///
|
///
|
||||||
/// Carries `kind` only; the payload (user text / notify message /
|
/// Carries `kind` only; the payload (user text / notify message /
|
||||||
@@ -833,7 +833,7 @@ pub enum InvokeKind {
|
|||||||
Notify,
|
Notify,
|
||||||
/// `Method::WorkerEvent` — typed lifecycle report from a child Worker.
|
/// `Method::WorkerEvent` — typed lifecycle report from a child Worker.
|
||||||
WorkerEvent,
|
WorkerEvent,
|
||||||
/// `<system-reminder>` etc. that crosses an IDLE boundary (mid-run
|
/// A typed system reminder that crosses an IDLE boundary (mid-run
|
||||||
/// reminders that don't break IDLE are SystemItem-only and do not
|
/// reminders that don't break IDLE are SystemItem-only and do not
|
||||||
/// open a new Invoke).
|
/// open a new Invoke).
|
||||||
SystemReminder,
|
SystemReminder,
|
||||||
|
|||||||
@@ -2,8 +2,8 @@
|
|||||||
//!
|
//!
|
||||||
//! Items in worker history with `role:system` are never produced by the
|
//! Items in worker history with `role:system` are never produced by the
|
||||||
//! LLM — they are always inserted by the Worker itself (notifications,
|
//! LLM — they are always inserted by the Worker itself (notifications,
|
||||||
//! file ref resolutions, child-worker lifecycle events,
|
//! file ref resolutions, child-worker lifecycle events, reminders, …).
|
||||||
//! future `<system-reminder>` tags, …). [`SystemItem`] carries the
|
//! [`SystemItem`] carries the
|
||||||
//! typed shape of each such injection so clients can dispatch on
|
//! typed shape of each such injection so clients can dispatch on
|
||||||
//! `kind` instead of parsing text prefixes like `[Notification] …` or
|
//! `kind` instead of parsing text prefixes like `[Notification] …` or
|
||||||
//! `[File: …]`.
|
//! `[File: …]`.
|
||||||
@@ -22,10 +22,7 @@ use llm_engine::llm_client::types::Item;
|
|||||||
use protocol::WorkerEvent;
|
use protocol::WorkerEvent;
|
||||||
use serde::{Deserialize, Serialize};
|
use serde::{Deserialize, Serialize};
|
||||||
|
|
||||||
const SYSTEM_REMINDER_OPEN: &str = "<system-reminder>";
|
/// Source policy that produced a durable system reminder input.
|
||||||
const SYSTEM_REMINDER_CLOSE: &str = "</system-reminder>";
|
|
||||||
|
|
||||||
/// Source policy that produced a durable `<system-reminder>` input.
|
|
||||||
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
|
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
|
||||||
#[serde(rename_all = "snake_case")]
|
#[serde(rename_all = "snake_case")]
|
||||||
pub enum SystemReminderSource {
|
pub enum SystemReminderSource {
|
||||||
@@ -52,57 +49,30 @@ pub struct SystemReminder {
|
|||||||
}
|
}
|
||||||
|
|
||||||
impl SystemReminder {
|
impl SystemReminder {
|
||||||
/// Build a task-inactivity reminder from an unwrapped body.
|
/// Build a task-inactivity reminder from its plain system-message body.
|
||||||
pub fn task_inactivity(body: impl Into<String>) -> Self {
|
pub fn task_inactivity(body: impl Into<String>) -> Self {
|
||||||
Self::new(SystemReminderSource::TaskInactivity, body)
|
Self::new(SystemReminderSource::TaskInactivity, body)
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Build a reminder from an unwrapped body. If a caller passes a body that
|
/// Build a reminder whose body is committed verbatim as a system message.
|
||||||
/// is already exactly wrapped in `<system-reminder>` tags, normalize it back
|
|
||||||
/// to the inner body so rendering still wraps exactly once.
|
|
||||||
pub fn new(source: SystemReminderSource, body: impl Into<String>) -> Self {
|
pub fn new(source: SystemReminderSource, body: impl Into<String>) -> Self {
|
||||||
let body = normalize_unwrapped_system_reminder_body(body.into());
|
Self {
|
||||||
Self { source, body }
|
source,
|
||||||
}
|
body: body.into(),
|
||||||
|
}
|
||||||
pub fn source(&self) -> SystemReminderSource {
|
|
||||||
self.source
|
|
||||||
}
|
|
||||||
|
|
||||||
pub fn body(&self) -> &str {
|
|
||||||
&self.body
|
|
||||||
}
|
|
||||||
|
|
||||||
pub fn rendered_body(&self) -> String {
|
|
||||||
render_system_reminder(&self.body)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
pub fn into_system_item(self) -> SystemItem {
|
pub fn into_system_item(self) -> SystemItem {
|
||||||
match self.source {
|
match self.source {
|
||||||
SystemReminderSource::TaskInactivity => SystemItem::TaskReminder {
|
SystemReminderSource::TaskInactivity => SystemItem::TaskReminder {
|
||||||
source: self.source,
|
source: self.source,
|
||||||
body: self.rendered_body(),
|
body: self.body,
|
||||||
prompt_provenance: None,
|
prompt_provenance: None,
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
fn normalize_unwrapped_system_reminder_body(body: String) -> String {
|
|
||||||
let trimmed = body.trim();
|
|
||||||
if let Some(inner) = trimmed
|
|
||||||
.strip_prefix(SYSTEM_REMINDER_OPEN)
|
|
||||||
.and_then(|rest| rest.strip_suffix(SYSTEM_REMINDER_CLOSE))
|
|
||||||
{
|
|
||||||
return inner.trim_matches('\n').to_string();
|
|
||||||
}
|
|
||||||
body
|
|
||||||
}
|
|
||||||
|
|
||||||
fn render_system_reminder(body: &str) -> String {
|
|
||||||
format!("{SYSTEM_REMINDER_OPEN}\n{body}\n{SYSTEM_REMINDER_CLOSE}")
|
|
||||||
}
|
|
||||||
|
|
||||||
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
|
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
|
||||||
pub struct PromptRenderProvenance {
|
pub struct PromptRenderProvenance {
|
||||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||||
@@ -178,7 +148,7 @@ pub enum SystemItem {
|
|||||||
|
|
||||||
/// Task-management inactivity reminder inserted before an LLM request.
|
/// Task-management inactivity reminder inserted before an LLM request.
|
||||||
/// `source` is the policy that produced this durable reminder; `body` is
|
/// `source` is the policy that produced this durable reminder; `body` is
|
||||||
/// the exact LLM-context text wrapped in a `<system-reminder>` block.
|
/// the exact plain system-message text committed to LLM context.
|
||||||
TaskReminder {
|
TaskReminder {
|
||||||
#[serde(default = "default_task_reminder_source")]
|
#[serde(default = "default_task_reminder_source")]
|
||||||
source: SystemReminderSource,
|
source: SystemReminderSource,
|
||||||
@@ -324,21 +294,9 @@ mod tests {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn system_reminder_renders_body_once() {
|
fn system_reminder_preserves_plain_body() {
|
||||||
let reminder = SystemReminder::task_inactivity("remember tasks");
|
let item = SystemReminder::task_inactivity("remember tasks").into_system_item();
|
||||||
assert_eq!(
|
assert_eq!(item.history_text(), "remember tasks");
|
||||||
reminder.rendered_body(),
|
|
||||||
"<system-reminder>\nremember tasks\n</system-reminder>"
|
|
||||||
);
|
|
||||||
|
|
||||||
let already_wrapped = SystemReminder::task_inactivity(
|
|
||||||
"<system-reminder>\nremember tasks\n</system-reminder>",
|
|
||||||
);
|
|
||||||
assert_eq!(already_wrapped.body(), "remember tasks");
|
|
||||||
assert_eq!(
|
|
||||||
already_wrapped.rendered_body(),
|
|
||||||
"<system-reminder>\nremember tasks\n</system-reminder>"
|
|
||||||
);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
@@ -347,10 +305,7 @@ mod tests {
|
|||||||
match item {
|
match item {
|
||||||
SystemItem::TaskReminder { source, body, .. } => {
|
SystemItem::TaskReminder { source, body, .. } => {
|
||||||
assert_eq!(source, SystemReminderSource::TaskInactivity);
|
assert_eq!(source, SystemReminderSource::TaskInactivity);
|
||||||
assert_eq!(
|
assert_eq!(body, "remember tasks");
|
||||||
body,
|
|
||||||
"<system-reminder>\nremember tasks\n</system-reminder>"
|
|
||||||
);
|
|
||||||
}
|
}
|
||||||
other => panic!("unexpected: {other:?}"),
|
other => panic!("unexpected: {other:?}"),
|
||||||
}
|
}
|
||||||
@@ -358,10 +313,8 @@ mod tests {
|
|||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn task_reminder_deserialization_defaults_legacy_source() {
|
fn task_reminder_deserialization_defaults_legacy_source() {
|
||||||
let parsed: SystemItem = serde_json::from_str(
|
let parsed: SystemItem =
|
||||||
r#"{"kind":"task_reminder","body":"<system-reminder>\nbody\n</system-reminder>"}"#,
|
serde_json::from_str(r#"{"kind":"task_reminder","body":"legacy body"}"#).unwrap();
|
||||||
)
|
|
||||||
.unwrap();
|
|
||||||
match parsed {
|
match parsed {
|
||||||
SystemItem::TaskReminder { source, .. } => {
|
SystemItem::TaskReminder { source, .. } => {
|
||||||
assert_eq!(source, SystemReminderSource::TaskInactivity);
|
assert_eq!(source, SystemReminderSource::TaskInactivity);
|
||||||
|
|||||||
@@ -2980,6 +2980,12 @@ fn idle_orchestrator_gets_bounded_attention_for_new_queued_work() {
|
|||||||
.expect("idle orchestrator should receive queued-work attention");
|
.expect("idle orchestrator should receive queued-work attention");
|
||||||
|
|
||||||
assert_eq!(request.worker_name, "test-orchestrator");
|
assert_eq!(request.worker_name, "test-orchestrator");
|
||||||
|
assert!(
|
||||||
|
request
|
||||||
|
.notice
|
||||||
|
.message
|
||||||
|
.starts_with("Workspace Dashboard observed")
|
||||||
|
);
|
||||||
assert!(request.notice.message.contains("00001QUEUE"));
|
assert!(request.notice.message.contains("00001QUEUE"));
|
||||||
assert!(request.notice.message.contains("new_queued"));
|
assert!(request.notice.message.contains("new_queued"));
|
||||||
assert!(request.notice.message.contains("queued -> inprogress"));
|
assert!(request.notice.message.contains("queued -> inprogress"));
|
||||||
|
|||||||
@@ -312,8 +312,7 @@ mod tests {
|
|||||||
let SystemItem::TaskReminder { body, .. } = &queued[0] else {
|
let SystemItem::TaskReminder { body, .. } = &queued[0] else {
|
||||||
panic!("unexpected system item: {:?}", queued[0]);
|
panic!("unexpected system item: {:?}", queued[0]);
|
||||||
};
|
};
|
||||||
assert_eq!(body.matches("<system-reminder>").count(), 1);
|
assert!(body.starts_with("Current session steps are listed below."));
|
||||||
assert_eq!(body.matches("</system-reminder>").count(), 1);
|
|
||||||
assert!(body.contains("taskid 1"));
|
assert!(body.contains("taskid 1"));
|
||||||
assert!(body.contains("pending"));
|
assert!(body.contains("pending"));
|
||||||
assert!(body.contains("keep going"));
|
assert!(body.contains("keep going"));
|
||||||
@@ -338,19 +337,17 @@ mod tests {
|
|||||||
panic!("unexpected system item: {:?}", queued[0]);
|
panic!("unexpected system item: {:?}", queued[0]);
|
||||||
};
|
};
|
||||||
assert_eq!(*source, SystemReminderSource::TaskInactivity);
|
assert_eq!(*source, SystemReminderSource::TaskInactivity);
|
||||||
assert_eq!(body.matches("<system-reminder>").count(), 1);
|
assert!(body.starts_with("Current session steps are listed below."));
|
||||||
assert_eq!(body.matches("</system-reminder>").count(), 1);
|
|
||||||
assert!(body.contains("typed"));
|
assert!(body.contains("typed"));
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn render_task_reminder_body_is_unwrapped_for_system_reminder_helper() {
|
fn render_task_reminder_body_is_plain_system_text() {
|
||||||
let feature = TaskFeature::new();
|
let feature = TaskFeature::new();
|
||||||
let task = feature.task_store().create("body".into(), String::new());
|
let task = feature.task_store().create("body".into(), String::new());
|
||||||
let body = render_task_reminder_body(&[task]);
|
let body = render_task_reminder_body(&[task]);
|
||||||
|
|
||||||
assert!(!body.contains("<system-reminder>"));
|
assert!(body.starts_with("Current session steps are listed below."));
|
||||||
assert!(!body.contains("</system-reminder>"));
|
|
||||||
assert!(body.contains("TaskUpdate"));
|
assert!(body.contains("TaskUpdate"));
|
||||||
assert!(body.contains("taskid 1"));
|
assert!(body.contains("taskid 1"));
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -176,9 +176,7 @@ impl SystemItemAppendHandle {
|
|||||||
|
|
||||||
/// Queue a task-inactivity reminder for durable model-visible append.
|
/// Queue a task-inactivity reminder for durable model-visible append.
|
||||||
///
|
///
|
||||||
/// The body should be the unwrapped reminder text; the host-side
|
/// The body is committed verbatim as the typed item's system-message text.
|
||||||
/// `SystemReminder` renderer wraps it exactly once in `<system-reminder>`
|
|
||||||
/// tags before commit.
|
|
||||||
pub fn append_task_reminder(&self, body: impl Into<String>) {
|
pub fn append_task_reminder(&self, body: impl Into<String>) {
|
||||||
let item = SystemReminder::task_inactivity(body).into_system_item();
|
let item = SystemReminder::task_inactivity(body).into_system_item();
|
||||||
self.pending
|
self.pending
|
||||||
@@ -452,8 +450,7 @@ mod tests {
|
|||||||
assert_eq!(queued.len(), 1);
|
assert_eq!(queued.len(), 1);
|
||||||
match &queued[0] {
|
match &queued[0] {
|
||||||
SystemItem::TaskReminder { body, .. } => {
|
SystemItem::TaskReminder { body, .. } => {
|
||||||
assert_eq!(body.matches("<system-reminder>").count(), 1);
|
assert_eq!(body, "remember tasks");
|
||||||
assert!(body.contains("remember tasks"));
|
|
||||||
}
|
}
|
||||||
other => panic!("unexpected system item: {other:?}"),
|
other => panic!("unexpected system item: {other:?}"),
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -12,8 +12,8 @@
|
|||||||
//!
|
//!
|
||||||
//! This is the **single lane** for "system messages produced by Worker
|
//! This is the **single lane** for "system messages produced by Worker
|
||||||
//! state that should land in the next LLM request": Notify,
|
//! state that should land in the next LLM request": Notify,
|
||||||
//! agent-visible WorkerEvent variants, and any future `<system-reminder>`
|
//! agent-visible WorkerEvent variants, and any future typed system reminder
|
||||||
//! injection all ride this queue.
|
//! insertion all ride this queue.
|
||||||
//! Per `tickets/notify-history-persist.md` and `AGENTS.md` (LLM
|
//! Per `tickets/notify-history-persist.md` and `AGENTS.md` (LLM
|
||||||
//! context の加工原則), there is **no** "transient, history-skipping"
|
//! context の加工原則), there is **no** "transient, history-skipping"
|
||||||
//! lane — everything injected into a request is also committed to
|
//! lane — everything injected into a request is also committed to
|
||||||
|
|||||||
@@ -24,7 +24,7 @@ Do not insert turn-crossing information directly into context without first appe
|
|||||||
Forbidden examples:
|
Forbidden examples:
|
||||||
|
|
||||||
- Delivering a `Notify` or `WorkerEvent` only as a temporary context note.
|
- Delivering a `Notify` or `WorkerEvent` only as a temporary context note.
|
||||||
- Adding a `<system-reminder>` that explains behavior but is not persisted.
|
- Adding a system reminder that explains behavior but is not persisted.
|
||||||
- Rewriting old messages to include new facts.
|
- Rewriting old messages to include new facts.
|
||||||
- Letting UI/controller-only state become model-visible without a committed record.
|
- Letting UI/controller-only state become model-visible without a committed record.
|
||||||
|
|
||||||
|
|||||||
@@ -1,4 +1,3 @@
|
|||||||
<system-reminder>
|
|
||||||
Workspace Dashboard observed that this Orchestrator Worker is idle while queued Ticket work is present.
|
Workspace Dashboard observed that this Orchestrator Worker is idle while queued Ticket work is present.
|
||||||
|
|
||||||
This is bounded attention only, not scheduler authority. Do not drain the queue automatically. Verify the Ticket is still `queued`, then use the guarded `SpawnTicketCoder` operation without a separate state transition; that operation records `queued -> inprogress` only after Worker creation, initial input, assignment, and Workdir finalization are durably accepted.
|
This is bounded attention only, not scheduler authority. Do not drain the queue automatically. Verify the Ticket is still `queued`, then use the guarded `SpawnTicketCoder` operation without a separate state transition; that operation records `queued -> inprogress` only after Worker creation, initial input, assignment, and Workdir finalization are durably accepted.
|
||||||
@@ -21,4 +20,3 @@ Additional queued Tickets omitted from this bounded notice: {{ omitted_ticket_co
|
|||||||
{% endif -%}
|
{% endif -%}
|
||||||
|
|
||||||
Preserve the existing human gate, dependency/conflict/capacity/dirty-workspace checks, and duplicate-start checks using actual Ticket state, role/session claims, visible Workers, and worktrees.
|
Preserve the existing human gate, dependency/conflict/capacity/dirty-workspace checks, and duplicate-start checks using actual Ticket state, role/session claims, visible Workers, and worktrees.
|
||||||
</system-reminder>
|
|
||||||
|
|||||||
Reference in New Issue
Block a user