From fc7650e6adeab2e16b8d45ed1d98d94247dcda61 Mon Sep 17 00:00:00 2001 From: kina0630 Date: Sat, 23 May 2026 22:16:20 +0800 Subject: [PATCH] feat: add recall eval observability --- ...oss-topic-memory-and-topic-range-search.md | 3 +- .../docs/cross_topic_memory/GOAL_PROGRESS.md | 61 +++++ .../src/coding_agent/observability.py | 225 +++++++++++++++++- .../src/coding_agent/recall_evaluation.py | 176 ++++++++++++++ .../tests/coding_agent/test_observability.py | 139 +++++++++++ .../coding_agent/test_recall_evaluation.py | 132 ++++++++++ 6 files changed, 734 insertions(+), 2 deletions(-) create mode 100644 coding-agent/src/coding_agent/recall_evaluation.py create mode 100644 coding-agent/tests/coding_agent/test_recall_evaluation.py diff --git a/coding-agent/docs/adr/0046-cross-topic-memory-and-topic-range-search.md b/coding-agent/docs/adr/0046-cross-topic-memory-and-topic-range-search.md index fa42a85e7..18dbd9afc 100644 --- a/coding-agent/docs/adr/0046-cross-topic-memory-and-topic-range-search.md +++ b/coding-agent/docs/adr/0046-cross-topic-memory-and-topic-range-search.md @@ -169,7 +169,8 @@ multi-agent task graph work are out of scope. - [x] `test_accepted_memory_context_pack_is_reference_only` - [x] `test_recall_plan_records_anchor_and_topic_recall_links` - [x] `test_recall_context_pack_contains_reference_evidence` -- [ ] `test_recall_metrics_omit_high_cardinality_labels` +- [x] `test_recall_eval_report_compares_all_recall_variants` +- [x] `test_recall_metrics_omit_high_cardinality_labels` - [ ] `test_console_memory_recall_renders_safe_provenance` - [ ] `test_cross_topic_memory_e2e_smoke` - [ ] `uv run pytest tests/coding_agent/test_topic_layer_smoke.py -v` diff --git a/coding-agent/docs/cross_topic_memory/GOAL_PROGRESS.md b/coding-agent/docs/cross_topic_memory/GOAL_PROGRESS.md index c0b43b466..ddeb0d551 100644 --- a/coding-agent/docs/cross_topic_memory/GOAL_PROGRESS.md +++ b/coding-agent/docs/cross_topic_memory/GOAL_PROGRESS.md @@ -279,3 +279,64 @@ phase. - Added disabled mode and no-leak query validation. - Remaining risks: - G142 still needs recall evaluation and low-cardinality recall/memory metrics. + +## G142_RECALL_EVAL_AND_OBSERVABILITY + +### Before + +- Goal id: G142_RECALL_EVAL_AND_OBSERVABILITY +- Intended files: + - `docs/cross_topic_memory/GOAL_PROGRESS.md` + - `docs/adr/0046-cross-topic-memory-and-topic-range-search.md` + - `src/coding_agent/recall_evaluation.py` + - `src/coding_agent/observability.py` + - `tests/coding_agent/test_recall_evaluation.py` + - `tests/coding_agent/test_observability.py` +- Verification commands: + - `uv run pytest tests/coding_agent/test_recall_evaluation.py tests/coding_agent/test_observability.py -v` + - `uv run pytest tests/coding_agent/test_recall_context.py tests/coding_agent/test_memory_review.py tests/coding_agent/evaluation/ -v` + - `uv run ruff format --check --preview docs/cross_topic_memory/GOAL_PROGRESS.md docs/adr/0046-cross-topic-memory-and-topic-range-search.md src/coding_agent/recall_evaluation.py src/coding_agent/observability.py tests/coding_agent/test_recall_evaluation.py tests/coding_agent/test_observability.py` + - `uv run ruff check src/coding_agent/recall_evaluation.py src/coding_agent/observability.py tests/coding_agent/test_recall_evaluation.py tests/coding_agent/test_observability.py` + - `git diff --check -- .` +- Stop criteria: + - Eval helper compares no recall, accepted-memory recall, topic-range recall, + and combined topic+memory recall deterministically. + - Recall/memory metrics emit only low-cardinality labels. + - Metrics omit `memory_id`, `topic_id`, `task_id`, `run_id`, and + `session_id` labels. + - Recall evaluation reports and metrics avoid raw evidence/log content. + +### After + +- Changed files: + - `docs/cross_topic_memory/GOAL_PROGRESS.md` + - `docs/adr/0046-cross-topic-memory-and-topic-range-search.md` + - `src/coding_agent/recall_evaluation.py` + - `src/coding_agent/observability.py` + - `tests/coding_agent/test_recall_evaluation.py` + - `tests/coding_agent/test_observability.py` +- Tests run: + - `uv run pytest tests/coding_agent/test_recall_evaluation.py tests/coding_agent/test_observability.py -q` + - `uv run pytest tests/coding_agent/test_recall_evaluation.py tests/coding_agent/test_observability.py -v` + - `uv run pytest tests/coding_agent/test_recall_context.py tests/coding_agent/test_memory_review.py tests/coding_agent/evaluation/ -v` + - `uv run pytest tests/coding_agent/test_observability_platform_smoke.py tests/coding_agent/test_context_system_smoke.py -v` + - `uv run ruff format --check --preview docs/cross_topic_memory/GOAL_PROGRESS.md docs/adr/0046-cross-topic-memory-and-topic-range-search.md src/coding_agent/recall_evaluation.py src/coding_agent/observability.py tests/coding_agent/test_recall_evaluation.py tests/coding_agent/test_observability.py` + - `uv run ruff check src/coding_agent/recall_evaluation.py src/coding_agent/observability.py tests/coding_agent/test_recall_evaluation.py tests/coding_agent/test_observability.py` + - `git diff --check -- .` +- Results: + - Red check before implementation failed on missing + `coding_agent.recall_evaluation` and missing recall/memory metric wrappers, + confirming the target tests covered the new behavior. + - Added deterministic recall evaluation reports for no-recall, + accepted-memory, topic-range, and combined recall variants. + - Added recall and memory metrics: + `topic_recall_runs_total`, `topic_recall_candidates`, + `memory_candidates_total`, and `memory_reviews_total`. + - Added no-leak and cardinality tests for recall/memory metrics and recall + evaluation report serialization. + - Local review found an OTLP trace value leak for raw recall/memory attribute + values; fixed by filtering sensitive string values before OTLP + serialization and adding a regression test. +- Remaining risks: + - G143 still needs Developer Console memory/recall rendering and safe review + surface coverage. diff --git a/coding-agent/src/coding_agent/observability.py b/coding-agent/src/coding_agent/observability.py index 2f091aff3..90fb3551e 100644 --- a/coding-agent/src/coding_agent/observability.py +++ b/coding-agent/src/coding_agent/observability.py @@ -36,14 +36,20 @@ _SENSITIVE_PROMETHEUS_VALUE_PARTS = frozenset({ "content", "env", + "key", + "log", "message", "output", + "password", + "private", "prompt", + "raw", "result", "secret", "stderr", "stdout", "text", + "token", }) _FORBIDDEN_PROMETHEUS_LABELS = frozenset({ "run_id", @@ -61,6 +67,7 @@ "interaction_id", "job_name", "launch_id", + "memory_id", "pod_name", "tool_call_id", "workflow_name", @@ -79,10 +86,15 @@ "executor_kind", "eval_status", "hitl_status", + "memory_kind", + "memory_review_status", + "memory_status", "model", "operation", "policy_decision", "provider", + "recall_source", + "recall_status", "risk_level", "schedule_kind", "schedule_status", @@ -170,6 +182,17 @@ }), "eval_status": frozenset({"failed", "passed", "skipped"}), "hitl_status": frozenset({"approved", "rejected", "requested", "timed_out"}), + "memory_kind": frozenset({ + "command_memory", + "decision", + "fact", + "incident", + "procedure", + "project_convention", + "unknown", + }), + "memory_review_status": frozenset({"accepted", "archived", "rejected"}), + "memory_status": frozenset({"accepted", "archived", "candidate", "rejected"}), "operation": frozenset({ "checkpoint_load", "checkpoint_save", @@ -179,6 +202,20 @@ "tape_load", }), "policy_decision": frozenset({"allow", "approval_required", "deny"}), + "recall_source": frozenset({ + "accepted_memory", + "none", + "topic_and_memory", + "topic_range", + "unknown", + }), + "recall_status": frozenset({ + "disabled", + "empty", + "failed", + "matched", + "unknown", + }), "risk_level": frozenset({"low", "medium", "high"}), "schedule_kind": frozenset({"interval", "manual", "once", "unknown"}), "schedule_status": frozenset({"active", "completed", "disabled", "paused"}), @@ -261,6 +298,40 @@ "unavailable", "unknown", }) +_TOPIC_RECALL_SOURCES: Final[frozenset[str]] = frozenset({ + "accepted_memory", + "none", + "topic_and_memory", + "topic_range", + "unknown", +}) +_TOPIC_RECALL_STATUSES: Final[frozenset[str]] = frozenset({ + "disabled", + "empty", + "failed", + "matched", + "unknown", +}) +_MEMORY_KINDS: Final[frozenset[str]] = frozenset({ + "command_memory", + "decision", + "fact", + "incident", + "procedure", + "project_convention", + "unknown", +}) +_MEMORY_STATUSES: Final[frozenset[str]] = frozenset({ + "accepted", + "archived", + "candidate", + "rejected", +}) +_MEMORY_REVIEW_STATUSES: Final[frozenset[str]] = frozenset({ + "accepted", + "archived", + "rejected", +}) _PROMETHEUS_HISTOGRAM_BUCKETS = (5.0, 10.0, 50.0, 100.0, 250.0, 500.0, 1000.0, 5000.0) _PROMETHEUS_HTTP_HISTOGRAM_BUCKETS = ( 1.0, @@ -397,10 +468,19 @@ def _otlp_attributes(attributes: Mapping[str, Any]) -> list[dict[str, Any]]: return [ {"key": key, "value": _otlp_value(value)} for key, value in attributes.items() - if _attribute_allowed(key) + if _attribute_allowed(key) and _attribute_value_allowed(value) ] +def _attribute_value_allowed(value: Any) -> bool: + if not isinstance(value, str): + return True + normalized_tokens = { + token for token in re.split(r"[^a-zA-Z0-9]+", value.casefold()) if token + } + return not bool(normalized_tokens & _SENSITIVE_PROMETHEUS_VALUE_PARTS) + + def _resource_attributes(attributes: Mapping[str, Any]) -> list[dict[str, Any]]: resource: dict[str, Any] = {"service.name": "coding-agent"} session_id = attributes.get("session_id") @@ -648,6 +728,39 @@ def record_executor_capability( with self._lock: self._set("executor_capability_status", labels, 1.0) + def record_topic_recall_run( + self, + *, + source: str, + status: str, + candidate_count: int, + ) -> None: + labels = { + "source": _prometheus_metric_part(source, _TOPIC_RECALL_SOURCES), + "status": _prometheus_metric_part(status, _TOPIC_RECALL_STATUSES), + } + histogram_labels = {"source": labels["source"]} + with self._lock: + self._inc("topic_recall_runs_total", labels) + self._observe( + "topic_recall_candidates", + histogram_labels, + max(0.0, float(candidate_count)), + ) + + def record_memory_candidate(self, *, kind: str, status: str) -> None: + labels = { + "kind": _prometheus_metric_part(kind, _MEMORY_KINDS), + "status": _prometheus_metric_part(status, _MEMORY_STATUSES), + } + with self._lock: + self._inc("memory_candidates_total", labels) + + def record_memory_review(self, *, status: str) -> None: + labels = {"status": _prometheus_metric_part(status, _MEMORY_REVIEW_STATUSES)} + with self._lock: + self._inc("memory_reviews_total", labels) + def exposition_text(self) -> str: lines: list[str] = [] with self._lock: @@ -838,6 +951,53 @@ def exposition_text(self) -> str: executor_capabilities, ) ) + topic_recall_counters = self._topic_recall_counters + topic_recall_histograms = self._topic_recall_histograms + if topic_recall_counters: + lines.extend(( + "# HELP topic_recall_runs_total Topic recall runs by source and status.", + "# TYPE topic_recall_runs_total counter", + )) + lines.extend( + self._format_metric( + "topic_recall_runs_total", + topic_recall_counters, + ) + ) + if topic_recall_histograms: + lines.extend(( + "# HELP topic_recall_candidates Recall candidates returned by source.", + "# TYPE topic_recall_candidates histogram", + )) + lines.extend( + self._format_histograms( + histograms=topic_recall_histograms, + metric_name="topic_recall_candidates", + buckets=_PROMETHEUS_HISTOGRAM_BUCKETS, + ) + ) + memory_counters = self._memory_counters + if memory_counters: + lines.extend(( + "# HELP memory_candidates_total Topic-derived memory candidates by kind and status.", + "# TYPE memory_candidates_total counter", + )) + lines.extend( + self._format_metric( + "memory_candidates_total", + memory_counters, + ) + ) + lines.extend(( + "# HELP memory_reviews_total Memory review transitions by status.", + "# TYPE memory_reviews_total counter", + )) + lines.extend( + self._format_metric( + "memory_reviews_total", + memory_counters, + ) + ) return "\n".join(lines) + ("\n" if lines else "") def reset(self) -> None: @@ -972,6 +1132,36 @@ def _executor_capability_gauges( if key[0] == "executor_capability_status" } + @property + def _topic_recall_counters( + self, + ) -> dict[tuple[str, tuple[tuple[str, str], ...]], float]: + return { + key: value + for key, value in self._counters.items() + if key[0] == "topic_recall_runs_total" + } + + @property + def _topic_recall_histograms( + self, + ) -> dict[tuple[str, tuple[tuple[str, str], ...]], list[float]]: + return { + key: value + for key, value in self._histograms.items() + if key[0] == "topic_recall_candidates" + } + + @property + def _memory_counters( + self, + ) -> dict[tuple[str, tuple[tuple[str, str], ...]], float]: + return { + key: value + for key, value in self._counters.items() + if key[0] in {"memory_candidates_total", "memory_reviews_total"} + } + def _inc(self, metric: str, labels: Mapping[str, str], amount: float = 1.0) -> None: key = (metric, tuple(sorted(labels.items()))) self._counters[key] = self._counters.get(key, 0.0) + amount @@ -1149,6 +1339,39 @@ def record_executor_capability_metric( return +def record_topic_recall_metric( + *, + source: str, + status: str, + candidate_count: int, +) -> None: + try: + _DEFAULT_PROMETHEUS_RECORDER.record_topic_recall_run( + source=source, + status=status, + candidate_count=candidate_count, + ) + except Exception: + return + + +def record_memory_candidate_metric(*, kind: str, status: str) -> None: + try: + _DEFAULT_PROMETHEUS_RECORDER.record_memory_candidate( + kind=kind, + status=status, + ) + except Exception: + return + + +def record_memory_review_metric(*, status: str) -> None: + try: + _DEFAULT_PROMETHEUS_RECORDER.record_memory_review(status=status) + except Exception: + return + + def reset_prometheus_metrics() -> None: _DEFAULT_PROMETHEUS_RECORDER.reset() diff --git a/coding-agent/src/coding_agent/recall_evaluation.py b/coding-agent/src/coding_agent/recall_evaluation.py new file mode 100644 index 000000000..d4bba9feb --- /dev/null +++ b/coding-agent/src/coding_agent/recall_evaluation.py @@ -0,0 +1,176 @@ +"""Deterministic evaluation helpers for cross-topic recall variants.""" + +from __future__ import annotations + +from dataclasses import dataclass +from enum import StrEnum + +from coding_agent.recall_context import ( + TopicRecallPlanner, + TopicRecallPlannerInput, +) +from coding_agent.topic_memory import ReviewedMemoryRecord +from coding_agent.topic_range_index import TopicRangeIndex, require_recall_safe_text +from coding_agent.topic_store import JSONObject, TopicRecord + + +class RecallEvalVariant(StrEnum): + NO_RECALL = "no_recall" + ACCEPTED_MEMORY = "accepted_memory" + TOPIC_RANGE = "topic_range" + TOPIC_AND_MEMORY = "topic_and_memory" + + +@dataclass(frozen=True) +class RecallEvalCase: + case_id: str + source_topic: TopicRecord + query_text: str + profile: str | None = None + bee_template_id: str | None = None + tags: tuple[str, ...] = () + expected_topic_ids: tuple[str, ...] = () + expected_memory_ids: tuple[str, ...] = () + limit: int = 5 + + def __post_init__(self) -> None: + _require_safe_identifier("case_id", self.case_id) + require_recall_safe_text("query_text", self.query_text) + if self.profile is not None: + _require_safe_identifier("profile", self.profile) + if self.bee_template_id is not None: + _require_safe_identifier("bee_template_id", self.bee_template_id) + for index, tag in enumerate(self.tags): + _require_safe_identifier(f"tags[{index}]", tag) + for index, topic_id in enumerate(self.expected_topic_ids): + _require_safe_identifier(f"expected_topic_ids[{index}]", topic_id) + for index, memory_id in enumerate(self.expected_memory_ids): + _require_safe_identifier(f"expected_memory_ids[{index}]", memory_id) + if self.limit <= 0: + raise ValueError("limit must be positive") + object.__setattr__(self, "tags", tuple(sorted(set(self.tags)))) + + +@dataclass(frozen=True) +class RecallEvalVariantResult: + variant: RecallEvalVariant + status: str + topic_ids: tuple[str, ...] = () + memory_ids: tuple[str, ...] = () + candidate_count: int = 0 + matched_expected: int = 0 + + def to_dict(self) -> JSONObject: + return { + "variant": self.variant.value, + "status": self.status, + "topic_ids": list(self.topic_ids), + "memory_ids": list(self.memory_ids), + "candidate_count": self.candidate_count, + "matched_expected": self.matched_expected, + } + + +@dataclass(frozen=True) +class RecallEvalReport: + case_id: str + results: tuple[RecallEvalVariantResult, ...] + + def result_for(self, variant: RecallEvalVariant) -> RecallEvalVariantResult: + for result in self.results: + if result.variant == variant: + return result + raise KeyError(f"recall eval variant not found: {variant.value}") + + def to_dict(self) -> JSONObject: + return { + "case_id": self.case_id, + "results": [result.to_dict() for result in self.results], + } + + +def evaluate_recall_variants( + case: RecallEvalCase, + *, + topic_index: TopicRangeIndex, + accepted_memories: tuple[ReviewedMemoryRecord, ...] = (), +) -> RecallEvalReport: + """Compare recall-disabled, memory-only, topic-only, and combined recall.""" + + results = ( + _evaluate_variant( + case, + variant=RecallEvalVariant.NO_RECALL, + topic_index=TopicRangeIndex(), + accepted_memories=(), + enabled=False, + ), + _evaluate_variant( + case, + variant=RecallEvalVariant.ACCEPTED_MEMORY, + topic_index=TopicRangeIndex(), + accepted_memories=accepted_memories, + enabled=True, + ), + _evaluate_variant( + case, + variant=RecallEvalVariant.TOPIC_RANGE, + topic_index=topic_index, + accepted_memories=(), + enabled=True, + ), + _evaluate_variant( + case, + variant=RecallEvalVariant.TOPIC_AND_MEMORY, + topic_index=topic_index, + accepted_memories=accepted_memories, + enabled=True, + ), + ) + return RecallEvalReport(case_id=case.case_id, results=results) + + +def _evaluate_variant( + case: RecallEvalCase, + *, + variant: RecallEvalVariant, + topic_index: TopicRangeIndex, + accepted_memories: tuple[ReviewedMemoryRecord, ...], + enabled: bool, +) -> RecallEvalVariantResult: + plan = TopicRecallPlanner( + topic_index=topic_index, + accepted_memories=accepted_memories, + ).plan( + TopicRecallPlannerInput( + source_topic=case.source_topic, + text=case.query_text, + profile=case.profile, + bee_template_id=case.bee_template_id, + tags=case.tags, + limit=case.limit, + enabled=enabled, + ) + ) + topic_ids = tuple(result.topic_id for result in plan.topic_results) + memory_ids = tuple( + memory.candidate.candidate_id or "" for memory in plan.accepted_memories + ) + candidate_count = len(topic_ids) + len(memory_ids) + matched_expected = len(set(topic_ids) & set(case.expected_topic_ids)) + len( + set(memory_ids) & set(case.expected_memory_ids) + ) + return RecallEvalVariantResult( + variant=variant, + status="matched" if candidate_count else "empty", + topic_ids=topic_ids, + memory_ids=memory_ids, + candidate_count=candidate_count, + matched_expected=matched_expected, + ) + + +def _require_safe_identifier(field_name: str, value: str) -> None: + require_recall_safe_text(field_name, value) + if any(char.isspace() for char in value): + raise ValueError(f"{field_name} must not contain whitespace") diff --git a/coding-agent/tests/coding_agent/test_observability.py b/coding-agent/tests/coding_agent/test_observability.py index 72ee5aca7..483dbd874 100644 --- a/coding-agent/tests/coding_agent/test_observability.py +++ b/coding-agent/tests/coding_agent/test_observability.py @@ -19,6 +19,9 @@ record_evaluation_case_metric, record_hitl_interaction_metric, record_http_request_metric, + record_memory_candidate_metric, + record_memory_review_metric, + record_topic_recall_metric, record_storage_operation_metric, reset_prometheus_metrics, ) @@ -535,6 +538,106 @@ def test_executor_metrics_record_on_default_recorder() -> None: ) +def test_recall_metrics_omit_high_cardinality_labels() -> None: + recorder = PrometheusMetricsRecorder() + sink = PrometheusMetricsObservationSink(recorder=recorder) + + sink.record_span( + SpanRecord( + name="context_pack.build", + status="ok", + attributes={ + "memory_id": "memory-alpha", + "topic_id": "topic-alpha", + "task_id": "task-alpha", + "run_id": "run-alpha", + "session_id": "session-alpha", + "recall_source": "topic_range", + "recall_status": "matched", + "memory_kind": "procedure", + "memory_status": "accepted", + }, + duration_ms=1, + ) + ) + recorder.record_topic_recall_run( + source="topic_and_memory", + status="matched", + candidate_count=2, + ) + recorder.record_memory_candidate(kind="procedure", status="candidate") + recorder.record_memory_review(status="accepted") + + text = recorder.exposition_text() + + assert 'recall_source="topic_range"' in text + assert 'recall_status="matched"' in text + assert 'memory_kind="procedure"' in text + assert 'memory_status="accepted"' in text + assert ( + 'topic_recall_runs_total{source="topic_and_memory",status="matched"} 1' in text + ) + assert 'topic_recall_candidates_bucket{le="5",source="topic_and_memory"} 1' in text + assert 'memory_candidates_total{kind="procedure",status="candidate"} 1' in text + assert 'memory_reviews_total{status="accepted"} 1' in text + for forbidden in ( + "memory_id", + "topic_id", + "task_id", + "run_id", + "session_id", + "memory-alpha", + "topic-alpha", + "task-alpha", + "run-alpha", + "session-alpha", + ): + assert forbidden not in text + + +def test_recall_metrics_normalize_unlisted_values_and_raw_evidence_text() -> None: + recorder = PrometheusMetricsRecorder() + + recorder.record_topic_recall_run( + source="customer_topic_alpha", + status="raw secret status", + candidate_count=3, + ) + recorder.record_memory_candidate(kind="raw_prompt_memory", status="candidate") + recorder.record_memory_review(status="custom_review") + + text = recorder.exposition_text() + + assert 'source="unknown"' in text + assert 'status="unknown"' in text + assert 'kind="unknown"' in text + assert "customer_topic_alpha" not in text + assert "raw_prompt_memory" not in text + assert "raw_secret_status" not in text + assert "custom_review" not in text + + +def test_recall_metrics_record_on_default_recorder() -> None: + reset_prometheus_metrics() + + record_topic_recall_metric( + source="accepted_memory", + status="matched", + candidate_count=1, + ) + record_memory_candidate_metric(kind="decision", status="candidate") + record_memory_review_metric(status="accepted") + + text = prometheus_metrics_text() + + assert ( + 'topic_recall_runs_total{source="accepted_memory",status="matched"} 1' in text + ) + assert 'topic_recall_candidates_count{source="accepted_memory"} 1' in text + assert 'memory_candidates_total{kind="decision",status="candidate"} 1' in text + assert 'memory_reviews_total{status="accepted"} 1' in text + + def test_prometheus_metrics_normalize_unsafe_allowed_values_and_reserved_labels() -> ( None ): @@ -792,6 +895,42 @@ def test_otlp_sink_posts_span_without_prompt_or_output_content() -> None: ) +def test_otlp_sink_drops_raw_recall_and_memory_attribute_values() -> None: + transport = RecordingTransport() + sink = OtlpHttpObservationSink( + endpoint="https://otel.example.test/v1/traces", + client=httpx.Client(transport=httpx.MockTransport(transport.handler)), + ) + + sink.record_span( + SpanRecord( + name="context_pack.build", + status="ok", + attributes={ + "recall_source": "secret: value", + "recall_status": "matched", + "memory_kind": "stdout: raw output", + "memory_status": "accepted", + }, + ) + ) + + body = transport.requests[0].read().decode() + span = _first_span(json.loads(body)) + exported_keys = { + item["key"] + for item in span.get("attributes", []) + if isinstance(item, dict) and isinstance(item.get("key"), str) + } + + assert "secret: value" not in body + assert "stdout: raw output" not in body + assert "recall_source" not in exported_keys + assert "memory_kind" not in exported_keys + assert "recall_status" in exported_keys + assert "memory_status" in exported_keys + + def test_otlp_sink_groups_spans_by_session_and_run() -> None: transport = RecordingTransport() sink = OtlpHttpObservationSink( diff --git a/coding-agent/tests/coding_agent/test_recall_evaluation.py b/coding-agent/tests/coding_agent/test_recall_evaluation.py new file mode 100644 index 000000000..2efb78cfe --- /dev/null +++ b/coding-agent/tests/coding_agent/test_recall_evaluation.py @@ -0,0 +1,132 @@ +from __future__ import annotations + +import json +from datetime import UTC, datetime + +from coding_agent.recall_evaluation import ( + RecallEvalCase, + RecallEvalVariant, + evaluate_recall_variants, +) +from coding_agent.topic_memory import ( + MemoryReviewStore, + propose_memory_candidate_from_topic, +) +from coding_agent.topic_range_index import TopicRangeIndex +from coding_agent.topic_store import TopicRecord + + +def test_recall_eval_report_compares_all_recall_variants() -> None: + index = TopicRangeIndex() + topic = _topic( + "topic-auth", + title="Auth migration", + summary="JWT validation moved to shared middleware", + ) + index.index_topic(topic, profile="local", tags=("auth",)) + review_store = MemoryReviewStore() + candidate = propose_memory_candidate_from_topic(topic, tags=("auth",)) + assert candidate is not None + review_store.add_candidate(candidate) + accepted = review_store.accept_candidate(candidate.candidate_id or "") + + report = evaluate_recall_variants( + RecallEvalCase( + case_id="recall-auth", + source_topic=_topic("topic-new", status="open", summary=None, end=None), + query_text="jwt middleware auth", + profile="local", + tags=("auth",), + expected_topic_ids=("topic-auth",), + expected_memory_ids=(accepted.candidate.candidate_id or "",), + ), + topic_index=index, + accepted_memories=review_store.accepted_memories(), + ) + + assert [result.variant for result in report.results] == [ + RecallEvalVariant.NO_RECALL, + RecallEvalVariant.ACCEPTED_MEMORY, + RecallEvalVariant.TOPIC_RANGE, + RecallEvalVariant.TOPIC_AND_MEMORY, + ] + assert report.result_for(RecallEvalVariant.NO_RECALL).candidate_count == 0 + accepted_result = report.result_for(RecallEvalVariant.ACCEPTED_MEMORY) + assert accepted_result.memory_ids == (accepted.candidate.candidate_id,) + assert accepted_result.matched_expected == 1 + topic_result = report.result_for(RecallEvalVariant.TOPIC_RANGE) + assert topic_result.topic_ids == ("topic-auth",) + assert topic_result.matched_expected == 1 + both_result = report.result_for(RecallEvalVariant.TOPIC_AND_MEMORY) + assert both_result.candidate_count == 2 + assert both_result.matched_expected == 2 + + +def test_recall_eval_report_serialization_omits_raw_summaries() -> None: + index = TopicRangeIndex() + index.index_topic( + _topic( + "topic-auth", + title="Auth migration", + summary="JWT validation moved to shared middleware", + ), + profile="local", + ) + + report = evaluate_recall_variants( + RecallEvalCase( + case_id="recall-auth", + source_topic=_topic("topic-new", status="open", summary=None, end=None), + query_text="jwt validation", + profile="local", + ), + topic_index=index, + ) + + payload = json.dumps(report.to_dict(), sort_keys=True) + + assert "topic-auth" in payload + assert "JWT validation moved to shared middleware" not in payload + assert "query_text" not in payload + + +def test_recall_eval_case_rejects_raw_query_text() -> None: + try: + RecallEvalCase( + case_id="bad", + source_topic=_topic("topic-new", status="open", summary=None, end=None), + query_text="stdout: raw command output", + ) + except ValueError as exc: + assert "forbidden raw content marker" in str(exc) + else: + raise AssertionError("raw recall query text should be rejected") + + +def _topic( + topic_id: str, + *, + title: str | None = "Topic", + summary: str | None, + kind: str = "coding", + status: str = "finalized", + start: int = 2, + end: int | None = 9, +) -> TopicRecord: + return TopicRecord( + topic_id=topic_id, + tape_id="tape-1", + session_id="session-1", + kind=kind, + status=status, + title=title, + summary=summary, + owner="local", + topic_initial_seq=start, + topic_finalized_seq=end, + created_at=datetime(2026, 5, 23, 9, 1, tzinfo=UTC), + finalized_at=datetime(2026, 5, 23, 9, 2, tzinfo=UTC) + if status == "finalized" + else None, + metadata={"profile": "local"}, + )