From 55c1cdaeac4bf61cf3c58dbb2ede711d20a195e8 Mon Sep 17 00:00:00 2001 From: jwgcurrie Date: Thu, 4 Jun 2026 15:19:41 +0100 Subject: [PATCH] attribute every LLMTurn to its resolved LLM config with content hash --- pepper_wizard/cli.py | 25 +++++++--- pepper_wizard/llm/client.py | 69 +++++++++++++------------ pepper_wizard/llm/identity.py | 55 ++++++++++++++++++++ tests/test_llm_client.py | 61 ++++++++++++++++++++++- tests/test_llm_identity.py | 94 +++++++++++++++++++++++++++++++++++ 5 files changed, 263 insertions(+), 41 deletions(-) create mode 100644 pepper_wizard/llm/identity.py create mode 100644 tests/test_llm_identity.py diff --git a/pepper_wizard/cli.py b/pepper_wizard/cli.py index 2c765cf..f005afc 100644 --- a/pepper_wizard/cli.py +++ b/pepper_wizard/cli.py @@ -782,6 +782,7 @@ def llm_talk_session(robot_client, config, verbose=False): from .stt_client import STTClient from .llm.client import LLMClient, LLMUnavailable from .llm.config_watcher import LLMConfigWatcher + from .llm.identity import build_config_snapshot logger = get_logger("LLMTalk") stt_config = config.stt_config @@ -799,6 +800,7 @@ def llm_talk_session(robot_client, config, verbose=False): ) return + logger.info("LLMConfigSnapshot", build_config_snapshot(watcher.current(), "session_start")) stt_client = STTClient(stt_config.get("zmq_address", "tcp://localhost:5562")) if not stt_client.ping(): print_formatted_text(HTML( @@ -963,20 +965,21 @@ def _announce_llm_reload(old_config: dict, new_config: dict, logger): return summary = "config reloaded: " + ", ".join(changed) print_formatted_text(HTML(f"{escape(summary)}")) - logger.info("LLMConfigReload", { - "changed": changed, - "old": old_config, - "new": new_config, - }) + from .llm.identity import build_config_snapshot + logger.info( + "LLMConfigSnapshot", + build_config_snapshot(new_config, "reload", changed=changed), + ) def _dispatch_to_llm(user_text, *, source, llm, stt, robot_client, logger): # Mute while Pepper is speaking so stt-service ignores self-hearing. - # TODO - for the person interrupting pepper this needs some neance. Outside the scope of current PR. + # TODO - for the person interrupting pepper this needs some neance. stt.mute() logger.info("MuteStart", {}) try: - reply = llm.reply(user_text) + result = llm.reply(user_text) + reply = result.text except Exception as e: print_formatted_text( HTML("LLM error: {}").format(str(e)) @@ -1005,4 +1008,10 @@ def _dispatch_to_llm(user_text, *, source, llm, stt, robot_client, logger): logger.error("TalkFailed", {"error": str(e)}) stt.unmute() logger.info("MuteEnd", {}) - logger.info("LLMTurn", {"user": user_text, "reply": reply, "source": source}) \ No newline at end of file + logger.info("LLMTurn", { + "user": user_text, + "reply": reply, + "source": source, + "config_hash": result.config_hash, + "config_name": result.config_name, + }) \ No newline at end of file diff --git a/pepper_wizard/llm/client.py b/pepper_wizard/llm/client.py index eceaa69..c0036a7 100644 --- a/pepper_wizard/llm/client.py +++ b/pepper_wizard/llm/client.py @@ -1,11 +1,16 @@ import os -from collections import deque +import threading +from collections import deque, namedtuple +from .identity import resolve_config, config_fingerprint from typing import TYPE_CHECKING if TYPE_CHECKING: from .config_watcher import LLMConfigWatcher +ReplyResult = namedtuple("ReplyResult", ["text", "config_hash", "config_name"]) + + class LLMUnavailable(Exception): pass @@ -20,11 +25,7 @@ class LLMClient: def __init__(self, watcher: "LLMConfigWatcher"): self._watcher = watcher - - # Initialise with the default maxlen; reply() will resize on first call - # if the watcher's history_turns differs. We do not call watcher.current() - # here so that the watcher call-count seen by callers reflects only - # active turns, not construction overhead. + self._lock = threading.Lock() self._history = deque(maxlen=10 * 2) api_key = os.environ.get("ANTHROPIC_API_KEY") @@ -48,33 +49,37 @@ def __init__(self, watcher: "LLMConfigWatcher"): def model(self) -> str: return self._watcher.current().get("model", "claude-haiku-4-5") - def reply(self, user_text: str) -> str: - """Send `user_text` with the rolling history and return the reply.""" - config = self._watcher.current() - - desired_maxlen = config.get("history_turns", 10) * 2 - if self._history.maxlen != desired_maxlen: - self._history = deque(self._history, maxlen=desired_maxlen) - - self._history.append({"role": "user", "content": user_text}) - - response = self._client.messages.create( - model=config.get("model", "claude-haiku-4-5"), - system=config.get( - "system_prompt", - "You are Pepper, a humanoid robot. Keep replies brief and conversational.", - ), - max_tokens=config.get("max_tokens", 256), - temperature=config.get("temperature", 0.7), - messages=list(self._history), - ) + def reply(self, user_text: str) -> "ReplyResult": + """Send user_text with the rolling history; return a ReplyResult with the + reply text plus the identity (config_hash, config_name) of the config used + for THIS turn. A per-client lock serializes concurrent callers (e.g. the + auto-dispatch VAD thread and typed input) so attribution is captured atomically + instead of read back from shared state.""" + with self._lock: + raw = self._watcher.current() + resolved = resolve_config(raw) + + desired_maxlen = resolved["history_turns"] * 2 + if self._history.maxlen != desired_maxlen: + self._history = deque(self._history, maxlen=desired_maxlen) + + self._history.append({"role": "user", "content": user_text}) + + response = self._client.messages.create( + model=resolved["model"], + system=resolved["system_prompt"], + max_tokens=resolved["max_tokens"], + temperature=resolved["temperature"], + messages=list(self._history), + ) - reply_text = "".join( - block.text for block in response.content if block.type == "text" - ).strip() + reply_text = "".join( + block.text for block in response.content if block.type == "text" + ).strip() - self._history.append({"role": "assistant", "content": reply_text}) - return reply_text + self._history.append({"role": "assistant", "content": reply_text}) + return ReplyResult(reply_text, config_fingerprint(resolved), raw.get("name")) def reset(self): - self._history.clear() + with self._lock: + self._history.clear() diff --git a/pepper_wizard/llm/identity.py b/pepper_wizard/llm/identity.py new file mode 100644 index 0000000..4558abd --- /dev/null +++ b/pepper_wizard/llm/identity.py @@ -0,0 +1,55 @@ +"""Source-agnostic identity for an LLM dialogue config. + +The identifier is a short content hash of the *resolved* config (the behavioural +fields actually sent to the model), so a turn can be attributed to its config +regardless of where the prompt came from: llm.json, a default, or a future +orchestrator. `name` is a human label, kept out of the hash so renaming a persona +does not change its behavioural fingerprint while editing a prompt does. +""" + +import hashlib +import json +from typing import List, Optional + +_DEFAULTS = { + "model": "claude-haiku-4-5", + "system_prompt": "You are Pepper, a humanoid robot. Keep replies brief and conversational.", + "max_tokens": 256, + "temperature": 0.7, + "history_turns": 10, +} + +# Fields that define model behaviour, and therefore the identity. +_BEHAVIOURAL_KEYS = ("model", "system_prompt", "max_tokens", "temperature", "history_turns") + + +def resolve_config(raw: dict) -> dict: + """Apply defaults, returning only the behavioural fields the model sees.""" + return {key: raw.get(key, _DEFAULTS[key]) for key in _BEHAVIOURAL_KEYS} + + +def config_fingerprint(config: dict) -> str: + """Short deterministic identity hash of a config. + + Resolves defaults first, so the hash always reflects what the model + receives whether a raw or already-resolved dict is passed (resolve_config + is idempotent). This removes any raw-vs-resolved mis-attribution surface. + """ + resolved = resolve_config(config) + canonical = json.dumps(resolved, sort_keys=True, separators=(",", ":")) + return hashlib.sha256(canonical.encode("utf-8")).hexdigest()[:12] + + +def build_config_snapshot(raw: dict, reason: str, *, source: str = "file", changed: Optional[List[str]] = None) -> dict: + """Assemble an LLMConfigSnapshot data dict from a raw config dict.""" + resolved = resolve_config(raw) + snapshot = { + "config_hash": config_fingerprint(resolved), + "config_name": raw.get("name"), + "config_source": source, + "reason": reason, + "config": resolved, + } + if changed is not None: + snapshot["changed"] = changed + return snapshot diff --git a/tests/test_llm_client.py b/tests/test_llm_client.py index 48df202..6e81a20 100644 --- a/tests/test_llm_client.py +++ b/tests/test_llm_client.py @@ -131,7 +131,7 @@ def test_reply_appends_user_and_assistant_turns(self): client = LLMClient(watcher) self._mock_messages.create.return_value = _make_anthropic_response("the reply") result = client.reply("the question") - self.assertEqual(result, "the reply") + self.assertEqual(result.text, "the reply") self.assertEqual(list(client._history), [ {"role": "user", "content": "the question"}, {"role": "assistant", "content": "the reply"}, @@ -159,6 +159,65 @@ def test_model_property_reflects_watcher(self): watcher.update(model="claude-sonnet-4-6") self.assertEqual(client.model, "claude-sonnet-4-6") + def test_reply_captures_config_hash_and_name(self): + from pepper_wizard.llm.client import LLMClient + from pepper_wizard.llm.identity import config_fingerprint, resolve_config + + cfg = { + "name": "persona-a", + "model": "claude-haiku-4-5", + "system_prompt": "be brief", + "max_tokens": 50, + "temperature": 0.3, + "history_turns": 4, + } + watcher = FakeWatcher(cfg) + client = LLMClient(watcher) + result = client.reply("hi") + self.assertEqual(result.config_hash, config_fingerprint(resolve_config(cfg))) + self.assertEqual(result.config_name, "persona-a") + + def test_config_name_none_when_absent(self): + from pepper_wizard.llm.client import LLMClient + + watcher = FakeWatcher({"history_turns": 4}) + client = LLMClient(watcher) + result = client.reply("hi") + self.assertIsNone(result.config_name) + + def test_config_hash_changes_when_config_changes(self): + from pepper_wizard.llm.client import LLMClient + + watcher = FakeWatcher({"system_prompt": "old", "history_turns": 4}) + client = LLMClient(watcher) + first_hash = client.reply("first").config_hash + watcher.update(system_prompt="new") + second_hash = client.reply("second").config_hash + self.assertNotEqual(second_hash, first_hash) + + def test_concurrent_replies_are_serialized(self): + import threading + import time + from pepper_wizard.llm.client import LLMClient + + watcher = FakeWatcher({"history_turns": 100}) + client = LLMClient(watcher) + + def slow_create(*args, **kwargs): + time.sleep(0.005) + return _make_anthropic_response("ok") + + self._mock_messages.create.side_effect = slow_create + threads = [threading.Thread(target=client.reply, args=("msg-%d" % i,)) for i in range(6)] + for t in threads: + t.start() + for t in threads: + t.join() + + history = list(client._history) + self.assertEqual(len(history), 12) + self.assertEqual([m["role"] for m in history], ["user", "assistant"] * 6) + if __name__ == "__main__": unittest.main() diff --git a/tests/test_llm_identity.py b/tests/test_llm_identity.py new file mode 100644 index 0000000..8271976 --- /dev/null +++ b/tests/test_llm_identity.py @@ -0,0 +1,94 @@ +import unittest + +from pepper_wizard.llm.identity import ( + resolve_config, + config_fingerprint, + build_config_snapshot, +) + +_BEHAVIOURAL = {"model", "system_prompt", "max_tokens", "temperature", "history_turns"} + + +class ResolveConfigTests(unittest.TestCase): + def test_applies_defaults_for_missing_fields(self): + resolved = resolve_config({"system_prompt": "hi"}) + self.assertEqual(resolved["model"], "claude-haiku-4-5") + self.assertEqual(resolved["max_tokens"], 256) + self.assertEqual(resolved["temperature"], 0.7) + self.assertEqual(resolved["history_turns"], 10) + self.assertEqual(resolved["system_prompt"], "hi") + + def test_drops_non_behavioural_keys(self): + resolved = resolve_config({"name": "persona-a", "model": "m", "extra": 1}) + self.assertNotIn("name", resolved) + self.assertNotIn("extra", resolved) + self.assertEqual(set(resolved), _BEHAVIOURAL) + + +class FingerprintTests(unittest.TestCase): + def test_deterministic_and_order_independent(self): + a = config_fingerprint( + {"model": "m", "system_prompt": "p", "max_tokens": 1, + "temperature": 0.1, "history_turns": 2} + ) + b = config_fingerprint( + {"history_turns": 2, "temperature": 0.1, "max_tokens": 1, + "system_prompt": "p", "model": "m"} + ) + self.assertEqual(a, b) + + def test_length_is_twelve_hex(self): + h = config_fingerprint(resolve_config({})) + self.assertEqual(len(h), 12) + int(h, 16) # raises if not hex + + def test_prompt_edit_changes_hash(self): + base = resolve_config({"system_prompt": "old"}) + edited = resolve_config({"system_prompt": "new"}) + self.assertNotEqual(config_fingerprint(base), config_fingerprint(edited)) + + def test_fingerprint_ignores_non_behavioural_keys(self): + # A raw config (with name + extras) and its resolved form fingerprint + # identically: config_fingerprint resolves internally, so there is no + # raw-vs-resolved mis-attribution surface. + raw = {"name": "persona-a", "model": "m", "system_prompt": "p", "extra": 1} + self.assertEqual(config_fingerprint(raw), config_fingerprint(resolve_config(raw))) + + +class BuildSnapshotTests(unittest.TestCase): + def test_session_start_shape(self): + snap = build_config_snapshot( + {"name": "a", "model": "m", "system_prompt": "p"}, "session_start" + ) + self.assertEqual(snap["reason"], "session_start") + self.assertEqual(snap["config_source"], "file") + self.assertEqual(snap["config_name"], "a") + self.assertEqual(len(snap["config_hash"]), 12) + self.assertEqual(set(snap["config"]), _BEHAVIOURAL) + self.assertNotIn("changed", snap) + + def test_config_name_none_when_absent(self): + snap = build_config_snapshot({"model": "m"}, "session_start") + self.assertIsNone(snap["config_name"]) + + def test_name_excluded_from_hash(self): + a = build_config_snapshot({"name": "a", "model": "m", "system_prompt": "p"}, "session_start") + b = build_config_snapshot({"name": "b", "model": "m", "system_prompt": "p"}, "session_start") + self.assertEqual(a["config_hash"], b["config_hash"]) + self.assertNotEqual(a["config_name"], b["config_name"]) + + def test_reload_includes_changed(self): + snap = build_config_snapshot( + {"model": "m"}, "reload", changed=["temperature 0.7→0.9"] + ) + self.assertEqual(snap["reason"], "reload") + self.assertEqual(snap["changed"], ["temperature 0.7→0.9"]) + + def test_hash_matches_fingerprint_of_resolved(self): + raw = {"model": "m", "system_prompt": "p"} + snap = build_config_snapshot(raw, "session_start") + self.assertEqual(snap["config_hash"], config_fingerprint(resolve_config(raw))) + + +if __name__ == "__main__": + unittest.main()