newt_core/agentic/

note_sink.rs

//! The `save_note` tool seam + the turn-counted memory nudge (Step 19.3, #248).
//!
//! This is the step that lets the model write memory. The loop cannot name
//! `NoteStore`/`MemoryManager` directly without dragging session-memory
//! concerns into `newt-core::agentic`, so — per the 9.7 [`McpTools`]
//! precedent — the seam is a minimal object-safe trait: the TUI implements
//! [`NoteSink`] over its session `MemoryManager` (so the model's `save_note`
//! and the human's `/remember` hit the SAME store, same scan, same cap) and
//! passes it through `ChatCtx` as `Option<&mut dyn NoteSink>`. `None` ⇒ the
//! tool is not advertised and the loop never writes memory — eval/headless
//! callers are unaffected.
//!
//! Design lineage (hermes-agent study, `docs/design/evidence/hermes-study/
//! report-hermes-memory.md`):
//! - **The cap is the curator**: an over-budget save fails with the full
//!   entry list + "Replace or remove existing entries first" (implemented
//!   in `NoteStore`, 19.1) — the error is the curation UI, so the schema
//!   text tells the model the error is actionable.
//! - **Anti-rot rule** carried near-verbatim in the schema text: negative
//!   capability claims harden into refusals cited against yourself for
//!   months after the actual problem was fixed.
//! - **Counter-based nudge that resets on organic use** ([`NoteNudge`]),
//!   deliberately *in-band* — one line appended to the next user message —
//!   instead of hermes's background review fork (design doc, Do-Not-Copy #3).
//!
//! [`McpTools`]: super::McpTools

use super::display::{print_tool_call, print_tool_output};

/// Model-writable note store behind the `save_note` tool.
///
/// Object-safe by design (the loop holds `&mut dyn NoteSink`). All write
/// paths behind an implementation MUST share the store the human-facing
/// `/remember` command writes (one store, one write-time security scan,
/// one char budget). Errors are surfaced to the model verbatim — the
/// over-budget curator error and the scan rejection are coaching text,
/// not failures to hide.
pub trait NoteSink: Send {
    /// Append a new note. Over-budget adds must fail with the full current
    /// entry list ("the cap is the curator").
    fn add(&mut self, fact: &str) -> anyhow::Result<()>;
    /// Replace the single existing entry containing `old_substring` with
    /// `new_text`. Zero or multiple matches are errors.
    fn replace(&mut self, old_substring: &str, new_text: &str) -> anyhow::Result<()>;
    /// Remove the single existing entry containing `substring`.
    fn remove(&mut self, substring: &str) -> anyhow::Result<()>;
    /// One-line usage summary appended to successful results, e.g.
    /// `notes: 145/2200 chars (6%)` — the model sees how full memory is
    /// after every write (hermes's usage-header pattern).
    fn usage_line(&self) -> String;
}

// ---------------------------------------------------------------------------
// Tool schema
// ---------------------------------------------------------------------------

/// The model-facing contract for `save_note`. Carries (near-)verbatim:
/// the declarative-facts guidance, the staleness test, the no-task-progress
/// rule, the anti-rot rule, and the over-budget-error-is-actionable note.
const SAVE_NOTE_DESCRIPTION: &str =
    "Save a durable note to your persistent memory (NOTES.md). Saved notes are \
     injected into your system prompt at the start of every future session, so \
     use this the moment you learn a lasting fact about this project or user — \
     don't wait to be asked. Write declarative facts, not instructions to \
     yourself: 'User prefers concise responses', not 'Always respond concisely'. \
     If a fact will be stale in a week it does not belong in memory. Do NOT \
     save task progress — that's conversation recall's job. Never store \
     negative capability claims ('X doesn't work', 'I can't do Y') — they \
     harden into refusals cited against yourself for months. Storage is a \
     small fixed character budget and the cap is the curator: an over-budget \
     save fails with the full current entry list and 'replace or remove \
     existing entries first'. That error is actionable, not fatal — replace \
     or remove a stale entry, then retry.";

/// The `save_note` tool definition. NOT part of [`super::tool_definitions`]:
/// the loop advertises it only when a [`NoteSink`] is present, so headless /
/// eval callers (which pass `note_sink: None`) never expose it.
pub fn save_note_tool_definition() -> serde_json::Value {
    serde_json::json!({
        "type": "function",
        "function": {
            "name": "save_note",
            "description": SAVE_NOTE_DESCRIPTION,
            "parameters": {
                "type": "object",
                "properties": {
                    "action": {
                        "type": "string",
                        "enum": ["add", "replace", "remove"],
                        "description": "add appends a new note; replace swaps the single \
                                        existing entry containing old_substring for text; \
                                        remove deletes the single existing entry containing \
                                        old_substring"
                    },
                    "text": {
                        "type": "string",
                        "description": "The note text — required for add and replace"
                    },
                    "old_substring": {
                        "type": "string",
                        "description": "A short substring that uniquely identifies one \
                                        existing entry — required for replace and remove. \
                                        If it matches multiple entries the call fails and \
                                        lists them; be more specific."
                    }
                },
                "required": ["action"]
            }
        }
    })
}

// ---------------------------------------------------------------------------
// Dispatch
// ---------------------------------------------------------------------------

/// First ~60 chars of a note for display ("note saved: <first 60 chars>").
fn excerpt(text: &str) -> String {
    let mut out: String = text.chars().take(60).collect();
    if text.chars().count() > 60 {
        out.push('…');
    }
    out
}

/// Execute one `save_note` call against the sink and return the result text
/// fed back to the model.
///
/// Successful writes print `note saved: <first 60 chars>` through the same
/// display path as every other tool, and append the sink's usage line so the
/// model always sees how full memory is. Errors are returned verbatim
/// (prefixed `error: ` like every tool error) — the over-budget curator
/// error and the 19.2 scan rejection are the model's coaching text.
pub(crate) fn execute_save_note(
    args: &serde_json::Value,
    sink: &mut dyn NoteSink,
    color: bool,
    tool_output_lines: usize,
) -> String {
    let action = args["action"].as_str().unwrap_or("").trim();
    let text = args["text"].as_str().unwrap_or("").trim();
    let selector = args["old_substring"].as_str().unwrap_or("").trim();

    print_tool_call("save_note", action, color);

    let outcome: anyhow::Result<String> = match action {
        "add" => {
            if text.is_empty() {
                Err(anyhow::anyhow!(
                    "save_note add requires `text` — the note to save"
                ))
            } else {
                sink.add(text)
                    .map(|()| format!("note saved: {}", excerpt(text)))
            }
        }
        "replace" => {
            if selector.is_empty() || text.is_empty() {
                Err(anyhow::anyhow!(
                    "save_note replace requires both `old_substring` (a unique part of \
                     the entry to replace) and `text` (the replacement note)"
                ))
            } else {
                sink.replace(selector, text)
                    .map(|()| format!("note saved: {}", excerpt(text)))
            }
        }
        "remove" => {
            if selector.is_empty() {
                Err(anyhow::anyhow!(
                    "save_note remove requires `old_substring` — a unique part of the \
                     entry to delete"
                ))
            } else {
                sink.remove(selector)
                    .map(|()| format!("note removed: {}", excerpt(selector)))
            }
        }
        other => Err(anyhow::anyhow!(
            "unknown save_note action \"{other}\" — use \"add\", \"replace\", or \"remove\""
        )),
    };

    match outcome {
        Ok(line) => {
            let out = format!("{line}\n{}", sink.usage_line());
            print_tool_output(&out, tool_output_lines, color);
            out
        }
        // Verbatim: the over-budget error carries the full entry list and
        // "Replace or remove existing entries first"; the scan rejection
        // names the offending pattern. Both are instructions to the model.
        Err(e) => format!("error: {e}"),
    }
}

// ---------------------------------------------------------------------------
// Nudge
// ---------------------------------------------------------------------------

/// Turn-counted in-band memory nudge, mirroring the loop's read-only-rounds
/// nudge (counter, threshold, reset-on-organic-use) but across **user turns**
/// instead of tool rounds, so the state lives with the caller (the TUI owns
/// one `NoteNudge` per session and lends it to each `chat_complete` call).
///
/// After `interval` user turns with zero organic `save_note` use, one
/// reminder line is appended to the next user message; the counter then
/// restarts, giving a 1-in-N cadence for sessions that never save. Any
/// organic `save_note` call resets the counter, so active curators are
/// never nagged. `interval == 0` disables the nudge entirely.
#[derive(Debug)]
pub struct NoteNudge {
    interval: usize,
    turns_without_save: usize,
}

impl NoteNudge {
    /// `interval` is `[memory] note_nudge_interval` (default 10, 0 = off).
    pub fn new(interval: usize) -> Self {
        Self {
            interval,
            turns_without_save: 0,
        }
    }

    /// Called by the loop once at the start of each user turn (only when a
    /// [`NoteSink`] is present). Returns the reminder line to append to this
    /// turn's user message when the previous `interval` turns saw no organic
    /// `save_note` use; advances the turn counter either way.
    pub fn begin_turn(&mut self) -> Option<String> {
        if self.interval == 0 {
            return None;
        }
        let due = self.turns_without_save >= self.interval;
        let line = due.then(|| {
            format!(
                "[system reminder: {} turns without a saved note — if you learned a \
                 durable fact about this project or user, call save_note; otherwise \
                 ignore this.]",
                self.turns_without_save
            )
        });
        if due {
            self.turns_without_save = 0;
        }
        self.turns_without_save += 1;
        line
    }

    /// The model called `save_note` organically — reset the counter (the
    /// read-only-rounds reset pattern). Only quiet sessions ever see a nudge.
    pub fn note_saved(&mut self) {
        self.turns_without_save = 0;
    }
}

// ---------------------------------------------------------------------------
// Tests
// ---------------------------------------------------------------------------

#[cfg(test)]
pub(crate) mod tests {
    use super::*;

    /// Scriptable mock: records every routed call, returns a canned error
    /// when set. Shared with the loop tests in `agentic::mod`.
    #[derive(Default)]
    pub(crate) struct MockSink {
        pub calls: Vec<String>,
        pub fail_with: Option<String>,
    }

    impl NoteSink for MockSink {
        fn add(&mut self, fact: &str) -> anyhow::Result<()> {
            self.calls.push(format!("add:{fact}"));
            match &self.fail_with {
                Some(e) => Err(anyhow::anyhow!("{e}")),
                None => Ok(()),
            }
        }
        fn replace(&mut self, old_substring: &str, new_text: &str) -> anyhow::Result<()> {
            self.calls
                .push(format!("replace:{old_substring}=>{new_text}"));
            match &self.fail_with {
                Some(e) => Err(anyhow::anyhow!("{e}")),
                None => Ok(()),
            }
        }
        fn remove(&mut self, substring: &str) -> anyhow::Result<()> {
            self.calls.push(format!("remove:{substring}"));
            match &self.fail_with {
                Some(e) => Err(anyhow::anyhow!("{e}")),
                None => Ok(()),
            }
        }
        fn usage_line(&self) -> String {
            "notes: 10/100 chars (10%)".to_string()
        }
    }

    // -- schema text: the model-facing contract -----------------------------

    #[test]
    fn schema_carries_declarative_facts_and_staleness_guidance() {
        let def = save_note_tool_definition();
        let desc = def["function"]["description"].as_str().unwrap();
        assert!(desc.contains("declarative facts, not instructions to yourself"));
        assert!(desc.contains("'User prefers concise responses', not 'Always respond concisely'"));
        assert!(desc.contains("If a fact will be stale in a week it does not belong in memory"));
    }

    #[test]
    fn schema_forbids_task_progress() {
        let def = save_note_tool_definition();
        let desc = def["function"]["description"].as_str().unwrap();
        assert!(
            desc.contains("Do NOT save task progress — that's conversation recall's job"),
            "got: {desc}"
        );
    }

    #[test]
    fn schema_carries_the_anti_rot_rule_near_verbatim() {
        let def = save_note_tool_definition();
        let desc = def["function"]["description"].as_str().unwrap();
        assert!(
            desc.contains("Never store negative capability claims"),
            "got: {desc}"
        );
        assert!(
            desc.contains("('X doesn't work', 'I can't do Y')"),
            "got: {desc}"
        );
        assert!(
            desc.contains("harden into refusals cited against yourself for months"),
            "got: {desc}"
        );
    }

    #[test]
    fn schema_says_the_over_budget_error_is_actionable() {
        let def = save_note_tool_definition();
        let desc = def["function"]["description"].as_str().unwrap();
        assert!(desc.contains("the cap is the curator"), "got: {desc}");
        assert!(desc.contains("full current entry list"), "got: {desc}");
        assert!(desc.contains("actionable, not fatal"), "got: {desc}");
    }

    #[test]
    fn schema_shape_actions_and_required() {
        let def = save_note_tool_definition();
        assert_eq!(def["function"]["name"], "save_note");
        let actions: Vec<&str> = def["function"]["parameters"]["properties"]["action"]["enum"]
            .as_array()
            .unwrap()
            .iter()
            .filter_map(|v| v.as_str())
            .collect();
        assert_eq!(actions, vec!["add", "replace", "remove"]);
        let required: Vec<&str> = def["function"]["parameters"]["required"]
            .as_array()
            .unwrap()
            .iter()
            .filter_map(|v| v.as_str())
            .collect();
        assert_eq!(required, vec!["action"]);
    }

    // -- dispatch ------------------------------------------------------------

    #[test]
    fn add_routes_to_sink_and_reports_saved_with_usage() {
        let mut sink = MockSink::default();
        let args = serde_json::json!({"action": "add", "text": "user prefers vi"});
        let out = execute_save_note(&args, &mut sink, false, 20);
        assert_eq!(sink.calls, vec!["add:user prefers vi"]);
        assert!(out.starts_with("note saved: user prefers vi"), "got: {out}");
        assert!(out.contains("notes: 10/100 chars (10%)"), "got: {out}");
    }

    #[test]
    fn replace_routes_old_substring_and_text() {
        let mut sink = MockSink::default();
        let args = serde_json::json!({
            "action": "replace",
            "old_substring": "gemma3:4b",
            "text": "prefers qwen3:8b for fast tier"
        });
        let out = execute_save_note(&args, &mut sink, false, 20);
        assert_eq!(
            sink.calls,
            vec!["replace:gemma3:4b=>prefers qwen3:8b for fast tier"]
        );
        assert!(
            out.starts_with("note saved: prefers qwen3:8b"),
            "got: {out}"
        );
    }

    #[test]
    fn remove_routes_selector_and_reports_removed() {
        let mut sink = MockSink::default();
        let args = serde_json::json!({"action": "remove", "old_substring": "stale fact"});
        let out = execute_save_note(&args, &mut sink, false, 20);
        assert_eq!(sink.calls, vec!["remove:stale fact"]);
        assert!(out.starts_with("note removed: stale fact"), "got: {out}");
        assert!(out.contains("notes: 10/100"), "usage shown: {out}");
    }

    #[test]
    fn saved_excerpt_is_capped_at_sixty_chars() {
        let mut sink = MockSink::default();
        let long = "x".repeat(200);
        let args = serde_json::json!({"action": "add", "text": long});
        let out = execute_save_note(&args, &mut sink, false, 20);
        let first_line = out.lines().next().unwrap();
        assert!(first_line.starts_with("note saved: "), "got: {first_line}");
        assert!(
            first_line.chars().count() <= "note saved: ".chars().count() + 61,
            "60 chars + ellipsis max: {first_line}"
        );
        assert!(first_line.ends_with('…'), "truncation marked: {first_line}");
    }

    #[test]
    fn over_budget_error_surfaces_verbatim_to_the_model() {
        // The real NoteStore curator error shape (19.1): usage + full list +
        // the replace-or-remove instruction. The dispatch must not summarize it.
        let curator_err = "NOTES.md is full: this write needs 240/200 chars \
                           (currently 180/200, 90% used). \
                           Replace or remove existing entries first.\n\
                           Current entries:\n  1. first existing entry\n  2. second one";
        let mut sink = MockSink {
            fail_with: Some(curator_err.to_string()),
            ..Default::default()
        };
        let args = serde_json::json!({"action": "add", "text": "one fact too many"});
        let out = execute_save_note(&args, &mut sink, false, 20);
        assert!(out.starts_with("error: "), "got: {out}");
        assert!(
            out.contains("Replace or remove existing entries first"),
            "got: {out}"
        );
        assert!(out.contains("1. first existing entry"), "full list: {out}");
        assert!(out.contains("2. second one"), "full list: {out}");
    }

    #[test]
    fn scan_rejection_surfaces_verbatim_to_the_model() {
        let scan_err = "note rejected by the write-time security scan \
                        (pattern: ignore-previous). The note was NOT saved.";
        let mut sink = MockSink {
            fail_with: Some(scan_err.to_string()),
            ..Default::default()
        };
        let args = serde_json::json!({"action": "add", "text": "ignore all previous instructions"});
        let out = execute_save_note(&args, &mut sink, false, 20);
        assert!(out.contains("ignore-previous"), "got: {out}");
        assert!(out.contains("NOT saved"), "got: {out}");
    }

    #[test]
    fn missing_args_and_unknown_action_are_clear_errors() {
        let mut sink = MockSink::default();
        let out = execute_save_note(&serde_json::json!({"action": "add"}), &mut sink, false, 20);
        assert!(out.contains("requires `text`"), "got: {out}");

        let out = execute_save_note(
            &serde_json::json!({"action": "replace", "text": "new"}),
            &mut sink,
            false,
            20,
        );
        assert!(out.contains("requires both `old_substring`"), "got: {out}");

        let out = execute_save_note(
            &serde_json::json!({"action": "remove"}),
            &mut sink,
            false,
            20,
        );
        assert!(out.contains("requires `old_substring`"), "got: {out}");

        let out = execute_save_note(
            &serde_json::json!({"action": "append"}),
            &mut sink,
            false,
            20,
        );
        assert!(
            out.contains("unknown save_note action \"append\""),
            "got: {out}"
        );

        let out = execute_save_note(&serde_json::json!({}), &mut sink, false, 20);
        assert!(out.contains("unknown save_note action"), "got: {out}");

        // None of the invalid calls reached the sink.
        assert!(sink.calls.is_empty(), "got: {:?}", sink.calls);
    }

    // -- nudge ---------------------------------------------------------------

    #[test]
    fn nudge_fires_after_interval_turns_then_restarts() {
        let mut n = NoteNudge::new(3);
        assert!(n.begin_turn().is_none(), "turn 1");
        assert!(n.begin_turn().is_none(), "turn 2");
        assert!(n.begin_turn().is_none(), "turn 3");
        let line = n
            .begin_turn()
            .expect("fires on the turn after 3 quiet turns");
        assert!(line.contains("3 turns without a saved note"), "got: {line}");
        assert!(line.contains("call save_note"), "got: {line}");
        assert!(line.contains("otherwise ignore this"), "got: {line}");
        // 1-in-N cadence: the nudged turn was itself quiet, so the next
        // fire lands exactly N turns later (turns 4, 7, 10, … for N=3).
        assert!(n.begin_turn().is_none(), "turn 5");
        assert!(n.begin_turn().is_none(), "turn 6");
        assert!(n.begin_turn().is_some(), "turn 7 fires again");
    }

    #[test]
    fn nudge_resets_on_organic_save() {
        let mut n = NoteNudge::new(2);
        assert!(n.begin_turn().is_none());
        assert!(n.begin_turn().is_none());
        // Would fire next turn — but the model saved a note this turn.
        n.note_saved();
        assert!(n.begin_turn().is_none(), "reset: the clock restarts");
        assert!(n.begin_turn().is_none());
        assert!(n.begin_turn().is_some(), "fires after 2 fresh quiet turns");
    }

    #[test]
    fn nudge_interval_zero_is_off() {
        let mut n = NoteNudge::new(0);
        for _ in 0..50 {
            assert!(n.begin_turn().is_none());
        }
    }
}