diff --git a/README.md b/README.md index 0450687..4a928e9 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # Sentinel -**AI on-call copilot.** Sentinel ingests production alerts (Sentry, PagerDuty, Datadog, generic webhooks), assembles incident context in parallel from your existing tooling, and produces a structured diagnosis — with confidence scoring, evidence citations against the supplied context, and suggested remediations. Single-shot, schema-validated, deterministic. Learns from resolutions via a pgvector incident memory. +**AI on-call copilot.** Sentinel ingests production alerts (Sentry, PagerDuty, Datadog, generic webhooks), assembles incident context in parallel from your existing tooling, and produces a structured diagnosis — with confidence scoring, evidence citations against the supplied context, and suggested remediations. Single-shot, schema-validated, and run at temperature 0. Learns from resolutions via a pgvector incident memory. Public engineering portfolio. Held to a production bar — every external call has a timeout and a circuit breaker, every LLM response is schema-validated, every evidence citation is verified, and the eval harness blocks regressions in CI. diff --git a/sentinel/diagnosis/agent.py b/sentinel/diagnosis/agent.py index 2631786..c4bfd35 100644 --- a/sentinel/diagnosis/agent.py +++ b/sentinel/diagnosis/agent.py @@ -15,6 +15,7 @@ from sentinel.diagnosis.deps import DiagnosisDeps from sentinel.diagnosis.errors import DiagnosisInvalid +from sentinel.diagnosis.llm_client import RepairTurn from sentinel.diagnosis.persisted import PersistedDiagnosis from sentinel.diagnosis.prompt import serialize from sentinel.diagnosis.truncation import truncate_for_budget @@ -62,12 +63,14 @@ async def diagnose( diagnosis_obj: Diagnosis | None = None last_result = None + repair: RepairTurn | None = None for attempt in (1, 2): last_result = await deps.llm.diagnose_call( system=system, user=user, tool_schema=tool_schema, tool_name=_TOOL_NAME, + repair=repair, ) # Meter tokens and cost per attempt: a retried attempt still spent # tokens against the Anthropic bill, so all attempts must be counted @@ -94,10 +97,16 @@ async def diagnose( except ValidationError as e: if attempt == 2: raise DiagnosisInvalid(str(e)) from e - user = ( - user - + f"\n\nYour previous response was invalid: {e}.\n" - + f"Return a valid {_TOOL_NAME} call." + # Repair as a proper multi-turn exchange: replay the model's invalid + # tool_use turn and answer it with a tool_result error, rather than + # appending prose to the user message. + repair = RepairTurn( + tool_use_id=last_result.tool_use_id, + tool_input=last_result.tool_input, + error=( + f"Your previous response failed schema validation: {e}. " + f"Return a corrected {_TOOL_NAME} call." + ), ) # Both variables are always set after the loop: the loop runs at least once diff --git a/sentinel/diagnosis/llm_client.py b/sentinel/diagnosis/llm_client.py index 019d2e4..1bf15d1 100644 --- a/sentinel/diagnosis/llm_client.py +++ b/sentinel/diagnosis/llm_client.py @@ -15,7 +15,7 @@ import anthropic import httpx from anthropic import AsyncAnthropic -from anthropic.types import ToolParam +from anthropic.types import MessageParam, ToolParam from pydantic import SecretStr from sentinel.diagnosis.errors import LLMNoToolCall, LLMTimeout, LLMTransport @@ -30,6 +30,23 @@ class LLMResult: output_tokens: int stop_reason: str latency_ms: int + # id of the returned tool_use block; needed to replay the turn in a repair + # exchange (see RepairTurn). Empty only for hand-built test fixtures. + tool_use_id: str = "" + + +@dataclass(frozen=True, slots=True) +class RepairTurn: + """One round of schema-repair context for a follow-up diagnosis call. + + Replayed as a proper multi-turn exchange — the prior assistant tool_use + turn plus a tool_result describing the validation error — rather than prose + appended to the user message. + """ + + tool_use_id: str + tool_input: dict[str, Any] + error: str class AnthropicClient: @@ -63,6 +80,7 @@ async def diagnose_call( user: str, tool_schema: dict[str, Any], tool_name: str, + repair: RepairTurn | None = None, ) -> LLMResult: try: return await asyncio.wait_for( @@ -71,6 +89,7 @@ async def diagnose_call( user=user, tool_schema=tool_schema, tool_name=tool_name, + repair=repair, ), timeout=self.timeout_s, ) @@ -84,6 +103,7 @@ async def _call_with_retry( user: str, tool_schema: dict[str, Any], tool_name: str, + repair: RepairTurn | None, ) -> LLMResult: try: return await self._stream_once( @@ -91,6 +111,7 @@ async def _call_with_retry( user=user, tool_schema=tool_schema, tool_name=tool_name, + repair=repair, ) except anthropic.APIStatusError: await asyncio.sleep(_BACKOFF_S) @@ -100,10 +121,47 @@ async def _call_with_retry( user=user, tool_schema=tool_schema, tool_name=tool_name, + repair=repair, ) except anthropic.APIStatusError as inner: raise LLMTransport(str(inner)) from inner + @staticmethod + def _build_messages(user: str, tool_name: str, repair: RepairTurn | None) -> list[MessageParam]: + if repair is None: + return [{"role": "user", "content": user}] + # Replay the original prompt, the model's prior (invalid) tool_use turn, + # and a tool_result carrying the validation error — a real multi-turn + # repair exchange rather than prose appended to the user message. + return cast( + list[MessageParam], + [ + {"role": "user", "content": user}, + { + "role": "assistant", + "content": [ + { + "type": "tool_use", + "id": repair.tool_use_id, + "name": tool_name, + "input": repair.tool_input, + } + ], + }, + { + "role": "user", + "content": [ + { + "type": "tool_result", + "tool_use_id": repair.tool_use_id, + "content": repair.error, + "is_error": True, + } + ], + }, + ], + ) + async def _stream_once( self, *, @@ -111,13 +169,17 @@ async def _stream_once( user: str, tool_schema: dict[str, Any], tool_name: str, + repair: RepairTurn | None, ) -> LLMResult: start = time.monotonic() stream = self._client.messages.stream( model=self.model, max_tokens=self.max_output_tokens, + # temperature=0 for run-to-run stability; the API exposes no seed, so + # this is as deterministic as a single-shot call can be. + temperature=0, system=system, - messages=[{"role": "user", "content": user}], + messages=self._build_messages(user, tool_name, repair), tools=[cast(ToolParam, tool_schema)], tool_choice={"type": "tool", "name": tool_name}, ) @@ -137,5 +199,6 @@ async def _stream_once( output_tokens=getattr(usage, "output_tokens", 0), stop_reason=getattr(final, "stop_reason", "tool_use"), latency_ms=latency_ms, + tool_use_id=getattr(block, "id", "") or "", ) raise LLMNoToolCall("model did not call the required tool") diff --git a/tests/unit/diagnosis/test_agent.py b/tests/unit/diagnosis/test_agent.py index acd90a2..9440cec 100644 --- a/tests/unit/diagnosis/test_agent.py +++ b/tests/unit/diagnosis/test_agent.py @@ -223,6 +223,61 @@ async def test_schema_invalid_then_valid_succeeds_after_one_retry() -> None: ) +async def test_retry_passes_repair_turn_with_prior_tool_use_id() -> None: + """On schema-invalid → retry, the agent must hand the client a RepairTurn + carrying the prior tool_use id + input (so the client can replay a proper + tool_result exchange), not mutate the user string (issue #63).""" + from sentinel.diagnosis.llm_client import RepairTurn + + ctx = make_context(deploys=[make_deploy("abc")]) + bad = dict( + hypothesis="", # fails min_length → triggers repair + confidence=0.5, + reasoning="r", + evidence=[{"kind": "deploy", "id": "deploy:abc", "note": "n"}], + suggested_actions=[], + likely_category="deploy", + ) + good = dict( + hypothesis="h", + confidence=0.5, + reasoning="r", + evidence=[{"kind": "deploy", "id": "deploy:abc", "note": "n"}], + suggested_actions=[], + likely_category="deploy", + ) + llm = AsyncMock() + llm.model = "claude-sonnet-4-5" + llm.diagnose_call.side_effect = [ + LLMResult( + tool_input=bad, + input_tokens=10, + output_tokens=5, + stop_reason="tool_use", + latency_ms=5, + tool_use_id="toolu_first", + ), + LLMResult( + tool_input=good, + input_tokens=20, + output_tokens=7, + stop_reason="tool_use", + latency_ms=5, + tool_use_id="toolu_second", + ), + ] + + await diagnose(_incident(ctx.incident_id), ctx, _deps(llm)) + + calls = llm.diagnose_call.call_args_list + assert calls[0].kwargs["repair"] is None # first attempt: no repair + repair = calls[1].kwargs["repair"] + assert isinstance(repair, RepairTurn) + assert repair.tool_use_id == "toolu_first" + assert repair.tool_input == bad + assert "validation" in repair.error.lower() + + async def test_audit_logger_records_one_line_per_call_attempt() -> None: deploy = make_deploy("abc") ctx = make_context(deploys=[deploy]) diff --git a/tests/unit/diagnosis/test_llm_client.py b/tests/unit/diagnosis/test_llm_client.py index 6018ac8..22a5ffd 100644 --- a/tests/unit/diagnosis/test_llm_client.py +++ b/tests/unit/diagnosis/test_llm_client.py @@ -42,6 +42,7 @@ def _make_fake_message( tool_name: str = _TOOL_NAME, tool_input: dict[str, Any] | None = None, include_tool_block: bool = True, + tool_use_id: str = "toolu_abc123", input_tokens: int = 100, output_tokens: int = 200, stop_reason: str = "tool_use", @@ -56,6 +57,7 @@ def _make_fake_message( block = MagicMock() block.type = "tool_use" block.name = tool_name + block.id = tool_use_id block.input = tool_input if tool_input is not None else _TOOL_INPUT msg.content = [block] else: @@ -131,6 +133,24 @@ async def test_happy_path_returns_llm_result() -> None: assert result.latency_ms >= 0 +async def test_stream_call_sets_temperature_zero() -> None: + """The diagnosis call must pin temperature=0 for run-to-run stability + (issue #63: previously unset → API default 1.0).""" + msg = _make_fake_message() + stream = _FakeStream(msg) + client = _make_client(stream_result=stream) + + await client.diagnose_call( + system="sys", + user="user", + tool_schema=_TOOL_SCHEMA, + tool_name=_TOOL_NAME, + ) + + _, kwargs = client._client.messages.stream.call_args # type: ignore[attr-defined] + assert kwargs["temperature"] == 0 + + async def test_timeout_raises_llm_timeout() -> None: """Stream that sleeps past timeout_s raises LLMTimeout.""" msg = _make_fake_message() @@ -225,6 +245,57 @@ async def test_http_client_passthrough() -> None: await fake_http_client.aclose() +async def test_llm_result_carries_tool_use_id() -> None: + """The tool_use block id must be captured so a repair turn can reference it.""" + msg = _make_fake_message(tool_use_id="toolu_xyz") + client = _make_client(stream_result=_FakeStream(msg)) + result = await client.diagnose_call( + system="sys", user="user", tool_schema=_TOOL_SCHEMA, tool_name=_TOOL_NAME + ) + assert result.tool_use_id == "toolu_xyz" + + +async def test_repair_builds_tool_result_turn() -> None: + """On repair, the request replays the prior assistant tool_use turn plus a + tool_result describing the error — a real multi-turn exchange, not prose + appended to the user string (issue #63).""" + from sentinel.diagnosis.llm_client import RepairTurn + + msg = _make_fake_message() + client = _make_client(stream_result=_FakeStream(msg)) + repair = RepairTurn( + tool_use_id="toolu_prev", + tool_input={"hypothesis": ""}, + error="hypothesis must be non-empty", + ) + + await client.diagnose_call( + system="sys", + user="original-user", + tool_schema=_TOOL_SCHEMA, + tool_name=_TOOL_NAME, + repair=repair, + ) + + _, kwargs = client._client.messages.stream.call_args # type: ignore[attr-defined] + messages = kwargs["messages"] + assert [m["role"] for m in messages] == ["user", "assistant", "user"] + # Original user prompt preserved as the first turn. + assert messages[0]["content"] == "original-user" + # Assistant turn replays the exact prior tool_use block (id + name + input). + tool_use = messages[1]["content"][0] + assert tool_use["type"] == "tool_use" + assert tool_use["id"] == "toolu_prev" + assert tool_use["name"] == _TOOL_NAME + assert tool_use["input"] == {"hypothesis": ""} + # Final user turn is a tool_result error referencing that tool_use id. + tool_result = messages[2]["content"][0] + assert tool_result["type"] == "tool_result" + assert tool_result["tool_use_id"] == "toolu_prev" + assert tool_result["is_error"] is True + assert "hypothesis must be non-empty" in tool_result["content"] + + async def test_no_tool_call_raises_llm_no_tool_call() -> None: """Message with no matching tool_use block raises LLMNoToolCall.""" msg = _make_fake_message(include_tool_block=False)