Merge PR #1851: preserve instrument identity in keyed metrics

agent/src/agent/grounding/evidence.py

@@ -126,6 +126,11 @@ _ANALYSIS_KIND_ALIASES = {
     "annualized_return": "return",
     "return": "return",
     "returns": "return",
+    "correlation_matrix": "correlation",
+    "contribution": "contribution",
+    "contribution_pct": "contribution",
+    "risk_contribution": "contribution",
+    "risk_contribution_pct": "contribution",
     "ic_positive_ratio": "win_rate",
     "var": "tail_risk",
     "var_95": "tail_risk",
@@ -569,6 +574,17 @@ def _metric_kind_for_path(path: str) -> str | None:
     kind = _ANALYSIS_KIND_ALIASES.get(leaf)
     if kind is not None:
         return kind
+    # Recognize explicitly named metric maps with ticker-keyed numeric leaves.
+    # A dotted venue suffix (AAPL.US) occupies multiple path components;
+    # only uppercase symbol-shaped components can follow the metric field.
+    parts = str(path or "").split(".")
+    for index, part in enumerate(parts[:-1]):
+        parent_kind = _ANALYSIS_KIND_ALIASES.get(part.casefold())
+        if parent_kind not in {"contribution", "correlation"}:
+            continue
+        labels = parts[index + 1 :]
+        if all(re.fullmatch(r"[A-Z][A-Z0-9]{0,14}", label) for label in labels):
+            return parent_kind
     # Compound leaves ("strategy_max_drawdown"): scan tokens from the right,
     # where English puts the head noun, so "return_vol" is vol, not return.
     # Only the head of a compound leaf says what it measures (#1426):
@@ -590,6 +606,32 @@ def _metric_kind_for_path(path: str) -> str | None:
     return None
 
 
+def _keyed_metric_map_kind(path: str) -> str | None:
+    """Identify metric maps with instrument-shaped keys after the metric name.
+
+    Scalar fields such as ``positions[0].contribution_pct`` are not keyed
+    maps and must keep their ordinary indexed evidence references.
+    """
+    parts = str(path or "").split(".")
+    for index, part in enumerate(parts[:-1]):
+        kind = _ANALYSIS_KIND_ALIASES.get(part.casefold())
+        if kind not in {"contribution", "correlation"}:
+            continue
+        if all(re.fullmatch(r"[A-Z][A-Z0-9]{0,14}", key) for key in parts[index + 1 :]):
+            return kind
+    return None
+
+
+def _analysis_symbol_key(key: str) -> str | None:
+    """Return a plausible instrument key, excluding common aggregate labels."""
+    if _CANONICAL_SYMBOL_RE.fullmatch(key) or (
+        re.fullmatch(r"[A-Z][A-Z0-9]{0,14}", key)
+        and key not in {"ALL", "PORTFOLIO", "TOTAL", "UNKNOWN"}
+    ):
+        return _normalize_symbol(key)
+    return None
+
+
 @dataclass(frozen=True)
 class EvidenceRecord:
     """One observed, unavailable, or derived numeric evidence item.
@@ -695,12 +737,20 @@ class _EvidenceMixin:
         """Record nested numeric leaves whose key names a metric kind."""
         recorded = 0
 
-        def visit(item: Any, path: str) -> None:
+        def visit(
+            item: Any,
+            path: str,
+            *,
+            metric_map: str | None = None,
+            keyed_symbol: str | None = None,
+            keyed_symbols: tuple[str, ...] = (),
+        ) -> None:
             nonlocal recorded
             if _is_number(item):
                 kind = _metric_kind_for_path(path)
                 if kind is None:
                     return
+                symbol = keyed_symbol if metric_map == "contribution" else None
                 self._analysis_metrics.append(
                     {
                         "metric": kind,
@@ -708,16 +758,48 @@ class _EvidenceMixin:
                         "tool": tool_name,
                         "call_id": call_id,
                         "field": path,
+                        "instrument_scoped": metric_map is not None,
+                        "symbol": symbol,
+                        "instrument_symbols": list(keyed_symbols),
                     }
                 )
                 recorded += 1
                 return
             if isinstance(item, dict):
                 for key, child in item.items():
-                    visit(child, f"{path}.{key}" if path else str(key))
+                    key_text = str(key)
+                    child_path = f"{path}.{key_text}" if path else key_text
+                    parent_kind = _ANALYSIS_KIND_ALIASES.get(key_text.casefold())
+                    child_metric_map = (
+                        parent_kind
+                        if parent_kind in {"contribution", "correlation"}
+                        else metric_map
+                    )
+                    child_symbol = keyed_symbol
+                    child_symbols = keyed_symbols
+                    if metric_map == "contribution":
+                        child_symbol = _analysis_symbol_key(key_text)
+                    elif metric_map == "correlation":
+                        child_symbol = None
+                        axis_symbol = _analysis_symbol_key(key_text)
+                        if axis_symbol and axis_symbol not in child_symbols:
+                            child_symbols = (*child_symbols, axis_symbol)
+                    visit(
+                        child,
+                        child_path,
+                        metric_map=child_metric_map,
+                        keyed_symbol=child_symbol,
+                        keyed_symbols=child_symbols,
+                    )
             elif isinstance(item, list):
                 for index, child in enumerate(item):
-                    visit(child, f"{path}[{index}]")
+                    visit(
+                        child,
+                        f"{path}[{index}]",
+                        metric_map=metric_map,
+                        keyed_symbol=keyed_symbol,
+                        keyed_symbols=keyed_symbols,
+                    )
 
         visit(value, field_prefix or "")
         return recorded

agent/src/agent/grounding/policies.py

@@ -28,6 +28,7 @@ from src.agent.grounding.evidence import (
     _is_metadata_count_leaf,
     _is_number,
     _is_price_kind,
+    _keyed_metric_map_kind,
     _metric_kind_for_path,
     _price_field_for_path,
     _timestamp_matches_claim_date,
@@ -718,13 +719,24 @@ class _PolicyMixin:
         segment precedes the line because a comparison report names both
         instruments on one header line and then writes about one of them.
         """
+        declared = None
         if declaration is not None:
             declared = self._symbol_for_claim(
                 f"{declaration.ref} {declaration.note}", records
             )
-            if declared:
-                return declared
-        return self._written_symbol(content, figure, line_symbols, records) or document_symbol
+        written = (
+            self._written_symbol(content, figure, line_symbols, records)
+            or document_symbol
+        )
+        if (
+            written
+            and declared
+            and written != declared
+            and declaration is not None
+            and _keyed_metric_map_kind(declaration.ref) is not None
+        ):
+            return written
+        return declared or written
 
     def _written_symbol(
         self,
@@ -871,9 +883,16 @@ class _PolicyMixin:
             if record.symbol and record.status == "observed":
                 call_symbols.setdefault(record.call_id, set()).add(record.symbol)
         return [
-            entry for entry in self._analysis_metrics
-            if not call_symbols.get(entry.get("call_id"))
-            or call_symbols[entry.get("call_id")] == {symbol}
+            entry
+            for entry in self._analysis_metrics
+            if (
+                entry.get("symbol") == symbol
+                if entry.get("instrument_scoped")
+                else (
+                    not call_symbols.get(entry.get("call_id"))
+                    or call_symbols[entry.get("call_id")] == {symbol}
+                )
+            )
         ]
 
     def _field_sources(
@@ -902,6 +921,7 @@ class _PolicyMixin:
             if named(record.field)
             and record.status == "observed"
             and record.value is not None
+            and _keyed_metric_map_kind(record.field) is None
             and (not symbol or not record.symbol or record.symbol == symbol)
         ]
         entries = [
@@ -1490,6 +1510,7 @@ class _PolicyMixin:
             if record.status == "observed"
             and record.value is not None
             and _metric_kind_for_path(record.field) is not None
+            and _keyed_metric_map_kind(record.field) is None
             and (not symbol or not record.symbol or record.symbol == symbol)
         )
         return values
@@ -2141,13 +2162,23 @@ class _PolicyMixin:
             ]
         return []
 
-    @staticmethod
     def _symbol_for_claim(
+        self,
         content: str,
         records: Sequence[EvidenceRecord],
     ) -> str | None:
         """Return one canonical evidence symbol explicitly named in a claim."""
         known = {record.symbol for record in records if record.symbol}
+        known.update(
+            entry.get("symbol")
+            for entry in self._analysis_metrics
+            if entry.get("symbol")
+        )
+        known.update(
+            symbol
+            for entry in self._analysis_metrics
+            for symbol in entry.get("instrument_symbols", ())
+        )
         matches = {
             _normalize_symbol(match.group(0))
             for match in _CANONICAL_SYMBOL_RE.finditer(content)

agent/tests/test_grounding_keyed_metric_paths.py

@@ -0,0 +1,154 @@
+"""Symbol-keyed analysis metric paths are classified without guessing generic maps."""
+
+from __future__ import annotations
+
+import json
+
+import pytest
+
+from src.agent.grounding import GroundingLedger
+from src.agent.grounding.evidence import _metric_kind_for_path
+
+
+@pytest.mark.parametrize(
+    ("path", "expected"),
+    [
+        ("risk_contribution_pct.AAPL.US", "contribution"),
+        ("risk_contribution_pct.ALPHA", "contribution"),
+        ("contribution.MSFT.US", "contribution"),
+        ("correlation_matrix.AAPL.US.MSFT.US", "correlation"),
+        ("correlation_matrix.ALPHA.BETA", "correlation"),
+        ("other_data.AAPL.US", None),
+        ("unrelated_map.ALPHA", None),
+        ("risk_contribution_pct.unknown", None),
+    ],
+)
+def test_symbol_keyed_metric_paths(path: str, expected: str | None) -> None:
+    assert _metric_kind_for_path(path) == expected
+
+
+def _ledger(tmp_path, payload):
+    ledger = GroundingLedger(
+        run_dir=tmp_path,
+        user_message="Compare instrument-level risk contributions across venues",
+    )
+    ledger.ingest_tool_result(
+        tool_name="factor_analysis",
+        arguments={},
+        result=json.dumps({"status": "ok", **payload}),
+        call_id="metrics-call",
+        success=True,
+    )
+    return ledger
+
+
+@pytest.mark.parametrize(
+    ("key", "symbol"),
+    [("ALPHA", "ALPHA"), ("AAPL.US", "AAPL.US")],
+)
+def test_contribution_map_keeps_exact_instrument_identity(tmp_path, key, symbol):
+    ledger = _ledger(tmp_path, {"risk_contribution_pct": {key: 0.125}})
+    (entry,) = ledger._analysis_metrics
+    assert entry["symbol"] == symbol
+    assert entry["instrument_scoped"] is True
+    assert ledger._metric_pool(symbol) == [0.125]
+    assert ledger._metric_pool("MSFT.US") == []
+
+
+def test_correlation_matrix_cells_have_two_axes_and_no_single_symbol(tmp_path):
+    ledger = _ledger(
+        tmp_path,
+        {"correlation_matrix": {"AAPL.US": {"MSFT.US": 0.42}}},
+    )
+    (entry,) = ledger._analysis_metrics
+    assert entry["metric"] == "correlation"
+    assert entry["symbol"] is None
+    assert entry["instrument_scoped"] is True
+    assert ledger._metric_pool("AAPL.US") == []
+    assert ledger._metric_pool("MSFT.US") == []
+    pair_claim = ledger.validate_final_answer(
+        "AAPL.US y MSFT.US have correlation 0.42.\n\n```figures\n"
+        "0.42 | observed | correlation | "
+        "metrics-call::correlation_matrix.AAPL.US.MSFT.US\n```"
+    )
+    assert pair_claim.valid, pair_claim.issues
+    single_instrument_claim = ledger.validate_final_answer(
+        "AAPL.US has correlation 0.42.\n\n```figures\n"
+        "0.42 | observed | correlation | "
+        "metrics-call::correlation_matrix.AAPL.US.MSFT.US\n```"
+    )
+    assert not single_instrument_claim.valid
+
+
+def test_equal_values_cannot_cross_instrument_reference(tmp_path):
+    ledger = _ledger(
+        tmp_path,
+        {"contribution_pct": {"ALPHA": 0.125, "AAPL.US": 0.125}},
+    )
+    assert ledger._metric_pool("ALPHA") == [0.125]
+    assert ledger._metric_pool("AAPL.US") == [0.125]
+    assert ledger._referenced(
+        "metrics-call::contribution_pct.ALPHA", "AAPL.US", None
+    ) == ([], [])
+    wrong_instrument_ref = ledger.validate_final_answer(
+        "AAPL.US contributes 0.125%.\n\n```figures\n"
+        "0.125% | observed | contribution | "
+        "metrics-call::contribution_pct.ALPHA\n```"
+    )
+    assert not wrong_instrument_ref.valid
+
+
+@pytest.mark.parametrize(
+    "key", ["TOTAL", "UNKNOWN", "AAPL.US.extra", "AAPL.US/MSFT.US"]
+)
+def test_ambiguous_or_aggregate_keys_do_not_get_identity(tmp_path, key):
+    ledger = _ledger(tmp_path, {"risk_contribution_pct": {key: 0.125}})
+    assert all(entry["symbol"] is None for entry in ledger._analysis_metrics)
+    assert all(entry["instrument_scoped"] for entry in ledger._analysis_metrics)
+    assert ledger._metric_pool("AAPL.US") == []
+
+
+def test_valid_final_answer_passes_and_wrong_symbol_value_fails(tmp_path):
+    ledger = _ledger(
+        tmp_path,
+        {"risk_contribution_pct": {"ALPHA": 0.125, "AAPL.US": 0.25}},
+    )
+    valid = ledger.validate_final_answer(
+        "ALPHA contributes 0.125%.\n\n```figures\n"
+        "0.125% | observed | contribution | "
+        "metrics-call::risk_contribution_pct.ALPHA\n```"
+    )
+    false = ledger.validate_final_answer(
+        "AAPL.US contributes 0.125%.\n\n```figures\n"
+        "0.125% | observed | contribution | "
+        "metrics-call::risk_contribution_pct.ALPHA\n```"
+    )
+    assert valid.valid, valid.issues
+    assert not false.valid
+
+
+def test_same_ticker_different_markets_remain_distinct(tmp_path):
+    """Market-qualified identifiers cannot lend evidence to each other."""
+    ledger = _ledger(
+        tmp_path,
+        {"risk_contribution_pct": {"ABC.US": 0.125, "ABC.BA": 0.125}},
+    )
+    assert ledger._metric_pool("ABC.US") == [0.125]
+    assert ledger._metric_pool("ABC.BA") == [0.125]
+    assert ledger._metric_pool("ABC") == []
+    assert ledger._referenced(
+        "metrics-call::risk_contribution_pct.ABC.US", "ABC.BA", None
+    ) == ([], [])
+
+
+def test_bare_ticker_not_automatically_equivalent_to_market_qualified(tmp_path):
+    """Do not infer an identifier alias or an exchange from the same stem."""
+    ledger = _ledger(
+        tmp_path,
+        {"risk_contribution_pct": {"ABC": 0.2, "ABC.US": 0.125}},
+    )
+    assert ledger._metric_pool("ABC") == [0.2]
+    assert ledger._metric_pool("ABC.US") == [0.125]
+    assert ledger._referenced(
+        "metrics-call::risk_contribution_pct.ABC", "ABC.US", None
+    ) == ([], [])