fix(grounding): preserve identity in ticker-keyed analysis metrics
Signed-off-by: zeus229 <69399886+zeus229@users.noreply.github.com>
This commit is contained in:
1 parent
54f4359559
commit
494033b509
3 files changed
+274
-10
No files matched your search
@@ -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,29 @@ def _metric_kind_for_path(path: str) -> str | None:
|
||||
return None
|
||||
|
||||
|
||||
def _keyed_metric_map_kind(path: str) -> str | None:
|
||||
"""Whether a field belongs to a per-instrument contribution or matrix map."""
|
||||
return next(
|
||||
(
|
||||
kind
|
||||
for part in str(path or "").split(".")
|
||||
if (kind := _ANALYSIS_KIND_ALIASES.get(part.casefold()))
|
||||
in {"contribution", "correlation"}
|
||||
),
|
||||
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 +734,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 +755,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
|
||||
|
||||
@@ -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
|
||||
@@ -2107,13 +2128,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)
|
||||
|
||||
@@ -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
|
||||
) == ([], [])
|
||||
Reference in new issue
Block a user