mirror of
https://github.com/superdesigndev/treg.git
synced 2026-10-02 03:24:35 +08:00
fix(routing): one reader of the miss block for router and arena; validate miss.when
Independent review findings on #573: - arena.classify read only miss.status, so a prospeo 400 INVALID_DATAPOINTS was a "miss" in compare mode and an "error" on the routed path. `declared_miss` / `miss_status` now live in routing.contracts and both callers use them. - A misspelt `when` evaluated False on every body and silently reverted the endpoint to "every 4xx is an error"; catalog_validate.py now requires a comparison or call, and the fixture test evaluates the predicate against the NO_MATCH and INVALID_DATAPOINTS bodies instead of grepping the string. - linkedin_url() lower-cases the host it matched so linkedin_handle() can derive from it (its regex is case-sensitive). - endpoint_view no longer exposes the internal `when` expression to agents. - vendor-listing skill: INVALID_DATA -> INVALID_DATAPOINTS (the observed code). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5.1
parent
b3e8c99a9e
commit
229baeb53c
@@ -168,7 +168,7 @@ uv run --frozen python -m pytest -q
|
||||
(#573: 64% of three days of `treg.people.email.find` 502s were exactly this).
|
||||
- LimaData answered 404 for "no email"; the cost `note` said "a 404 miss is free" but no
|
||||
`miss:` block existed. Prose in `note` is documentation; only the `miss:` block is read.
|
||||
- Prospeo answers 400 for BOTH `NO_MATCH` (a miss) and `INVALID_DATA` (a real error). When
|
||||
- Prospeo answers 400 for BOTH `NO_MATCH` (a miss) and `INVALID_DATAPOINTS` (a real error). When
|
||||
status alone cannot separate them the declaration needs a body predicate, not a
|
||||
`provider == "x"` branch in `route.py` — provider knowledge lives in YAML and adapters only.
|
||||
- The hit-only `example_response` hides the miss shape from every later reader; capture the
|
||||
|
||||
@@ -1956,10 +1956,15 @@ Five rules worth keeping:
|
||||
same). Endpoints with evidenced miss behaviour carry a `miss: {status, means}` block in their
|
||||
YAML, surfaced through `endpoint_view` — so an agent reads "404 = no match, don't retry" instead
|
||||
of treating an expected empty answer as a failure. Only annotate what the wire has demonstrated.
|
||||
**The router reads the same block** (`route._declared_miss`): a child answering the declared
|
||||
4xx is a MISS — the waterfall goes on and a fully-missed call ends as a 200 miss, never
|
||||
`route_failed`. Where one status carries both a miss and a fault, `when:` adds a body predicate
|
||||
in the adapter expression language, evaluated only on a JSON-object body: prospeo answers 400
|
||||
**The router and the arena read the same block through one function**
|
||||
(`routing.contracts.declared_miss`, wrapped by `route._declared_miss` and called by
|
||||
`arena.classify`): a child answering the declared 4xx is a MISS — the waterfall goes on and a
|
||||
fully-missed call ends as a 200 miss, never `route_failed`. Where one status carries both a
|
||||
miss and a fault, `when:` adds a body predicate in the adapter expression language, evaluated
|
||||
only on a JSON-object body (`catalog_validate.py` rejects a `when` that is not a comparison or
|
||||
call, since a misspelt path would evaluate False forever and silently revert the endpoint to
|
||||
"every 4xx is an error"; `endpoint_view` shows agents `status` and `means` but not `when`):
|
||||
prospeo answers 400
|
||||
for `NO_MATCH` (a miss) and for `INVALID_DATAPOINTS` (a fault), so its three person endpoints
|
||||
declare `miss: {status: 400, when: "error_code == 'NO_MATCH'"}`. Provider knowledge lives in
|
||||
the YAML; `route.py` never names a provider. Live 2026-09-18: 64% of three days of
|
||||
|
||||
@@ -56,6 +56,7 @@ from treg.domain.catalog.store import COST_SOURCES as _SOURCES # noqa: E402
|
||||
from treg.domain.catalog.store import COST_UNITS as _UNITS # noqa: E402
|
||||
from treg.domain.catalog.store import CONFIDENCES as _CONFIDENCES # noqa: E402
|
||||
from treg.domain.catalog.store import effective_async_descriptor # noqa: E402
|
||||
from treg.domain.catalog.routing import paths as _paths # noqa: E402
|
||||
|
||||
SCOPES = {"any_account", "own_account"}
|
||||
METHODS = {"GET", "POST", "PUT", "PATCH", "DELETE"}
|
||||
@@ -896,6 +897,16 @@ def main(argv: list[str]) -> int:
|
||||
for f in REQUIRED[tier]:
|
||||
if not ep.get(f):
|
||||
fail(errors, where, f"missing required field '{f}'")
|
||||
miss = ep.get("miss")
|
||||
if isinstance(miss, dict) and miss.get("when") is not None:
|
||||
# The router evaluates `when` against the provider body; a misspelt path parses
|
||||
# fine, evaluates False on every body, and silently turns every declared miss back
|
||||
# into an error. Require a comparison or a call the expression language accepts.
|
||||
when = miss["when"]
|
||||
if not isinstance(when, str) or not (_paths._CMP.match(when.strip()) or _paths._CALL.match(when.strip())):
|
||||
fail(errors, where, f"miss.when must be a comparison or call in the adapter expression language, got {when!r}")
|
||||
elif miss.get("status") is None:
|
||||
fail(errors, where, "miss.when needs miss.status (the 4xx it narrows)")
|
||||
if eid in seen_ids:
|
||||
fail(errors, where, f"duplicate id (also in {seen_ids[eid]})")
|
||||
seen_ids[eid] = name
|
||||
|
||||
@@ -34,8 +34,7 @@ from ...domain.capacity.view import view as capacity_view
|
||||
from ...domain.capacity.signatures import classify as classify_capacity
|
||||
from ...domain.catalog import stats as endpoint_stats
|
||||
from ...domain.catalog import store as catalog_store
|
||||
from ...domain.catalog.routing import paths as P
|
||||
from ...domain.catalog.routing.contracts import canonical_identity
|
||||
from ...domain.catalog.routing.contracts import canonical_identity, declared_miss, miss_status
|
||||
from ...domain.catalog.routing.plan import (
|
||||
MAX_ERROR_FALLBACKS, Candidate, Plan, candidates_for, cost_at, ignored_filters, rank,
|
||||
)
|
||||
@@ -103,44 +102,18 @@ def _free_on_failure(cand: Candidate) -> bool:
|
||||
|
||||
|
||||
def _miss_status(endpoint: dict) -> int | None:
|
||||
"""The ERROR status this endpoint's YAML declares as "no result" (`miss: {status, means}`),
|
||||
or None when an error status means what it says. Only a 4xx counts: a `status: 200` block
|
||||
(tikhub's "an unknown id still answers 200 with a null body") documents a 2xx the adapter's
|
||||
own `miss` predicate decides, and honouring it here would call every success a miss."""
|
||||
m = endpoint.get("miss")
|
||||
if isinstance(m, dict) and m.get("status") is not None:
|
||||
try:
|
||||
status = int(m["status"])
|
||||
except (TypeError, ValueError):
|
||||
return None
|
||||
return status if 400 <= status < 500 else None
|
||||
return None
|
||||
"""See `routing.contracts.miss_status` — kept as the router's name for it (tests pin it)."""
|
||||
return miss_status(endpoint)
|
||||
|
||||
|
||||
def _declared_miss(endpoint: dict, status: int, raw: bytes) -> bool:
|
||||
"""True when a child's ERROR status is the endpoint's declared "no result" answer.
|
||||
|
||||
`miss: {status}` alone matches on status. `miss: {status, when}` adds a body predicate in the
|
||||
adapter expression language (`when: "error_code == 'NO_MATCH'"`) for providers that answer
|
||||
the SAME status for a miss and a real request error — prospeo 400s both `NO_MATCH` (a miss)
|
||||
and `INVALID_DATAPOINTS` (a fault). The provider knowledge stays in the YAML; nothing here
|
||||
names a provider. A body that is not a JSON object never satisfies a predicate."""
|
||||
if status != _miss_status(endpoint):
|
||||
return False
|
||||
when = (endpoint.get("miss") or {}).get("when")
|
||||
if not when:
|
||||
return True
|
||||
try:
|
||||
doc = json.loads(raw)
|
||||
except ValueError:
|
||||
return False
|
||||
if not isinstance(doc, dict):
|
||||
return False
|
||||
try:
|
||||
return bool(P.evaluate(str(when), doc))
|
||||
except Exception: # noqa: BLE001 — a broken predicate must read as "not a miss", never crash the parent
|
||||
log.warning("miss.when predicate failed for %s", endpoint.get("id"), exc_info=True)
|
||||
"""See `routing.contracts.declared_miss`: the router and the arena read the `miss:` block
|
||||
through the same function, so a prospeo 400 INVALID_DATAPOINTS is an error in both."""
|
||||
if not declared_miss(endpoint, status, raw):
|
||||
return False
|
||||
if (endpoint.get("miss") or {}).get("when"):
|
||||
log.debug("declared miss by predicate on %s", endpoint.get("id"))
|
||||
return True
|
||||
|
||||
|
||||
DEFAULT_MAX_COST_MICRO = 1_000_000 # $1.00 per routed call unless the caller says otherwise — a runaway guard, not a budget
|
||||
|
||||
@@ -6,6 +6,8 @@ import re
|
||||
from typing import Any
|
||||
from urllib.parse import urlsplit, urlunsplit
|
||||
|
||||
from .catalog.routing.contracts import declared_miss
|
||||
|
||||
VERSION = "2"
|
||||
MAX_RESULT_BYTES = 256_000
|
||||
MAX_BATCH_RAW_BYTES = 2_000_000
|
||||
@@ -209,8 +211,7 @@ def required_credit(attempts: list[dict], mode: str) -> int:
|
||||
|
||||
def classify(contract, adapter, endpoint: dict, status: int, doc: Any) -> tuple[str, dict]:
|
||||
"""Match the routed lookup's structural hit rule; retain verification qualifiers separately."""
|
||||
miss_status = (endpoint.get("miss") or {}).get("status")
|
||||
if status == miss_status and 400 <= status < 500:
|
||||
if declared_miss(endpoint, status, doc): # the router's reader of the `miss:` block, predicate included
|
||||
return "miss", {}
|
||||
if not 200 <= status < 300:
|
||||
return "error", {}
|
||||
|
||||
@@ -140,6 +140,48 @@ def parse_adapters(doc: dict) -> dict[str, Adapter]:
|
||||
|
||||
# ---- identity -------------------------------------------------------------------------------
|
||||
|
||||
def miss_status(endpoint: dict) -> int | None:
|
||||
"""The ERROR status this endpoint's YAML declares as "no result" (`miss: {status, means}`),
|
||||
or None. Only a 4xx counts: a `status: 200` block documents a 2xx the adapter's own `miss`
|
||||
predicate decides, and honouring it here would call every success a miss."""
|
||||
m = endpoint.get("miss")
|
||||
if isinstance(m, dict) and m.get("status") is not None:
|
||||
try:
|
||||
status = int(m["status"])
|
||||
except (TypeError, ValueError):
|
||||
return None
|
||||
return status if 400 <= status < 500 else None
|
||||
return None
|
||||
|
||||
|
||||
def declared_miss(endpoint: dict, status: int, body: Any) -> bool:
|
||||
"""True when a child's ERROR status is the endpoint's declared "no result" answer — the ONE
|
||||
reader of the `miss:` block for the router and the arena, so both agree.
|
||||
|
||||
`miss: {status}` matches on status alone. `miss: {status, when}` adds a body predicate in the
|
||||
adapter expression language (`when: "error_code == 'NO_MATCH'"`) for providers whose one
|
||||
status carries both a miss and a fault (prospeo 400: NO_MATCH vs INVALID_DATAPOINTS). `body`
|
||||
is the raw bytes or the parsed document; only a JSON object can satisfy a predicate, and a
|
||||
predicate that raises reads as "not a miss"."""
|
||||
if status != miss_status(endpoint):
|
||||
return False
|
||||
when = (endpoint.get("miss") or {}).get("when")
|
||||
if not when:
|
||||
return True
|
||||
doc = body
|
||||
if isinstance(body, (bytes, bytearray, str)):
|
||||
try:
|
||||
doc = json.loads(body)
|
||||
except ValueError:
|
||||
return False
|
||||
if not isinstance(doc, dict):
|
||||
return False
|
||||
try:
|
||||
return bool(P.evaluate(str(when), doc))
|
||||
except Exception: # noqa: BLE001 — a broken predicate must never crash a call
|
||||
return False
|
||||
|
||||
|
||||
def canonical_identity(contract: Contract, given: dict[str, Any]) -> tuple[dict[str, Any], tuple[str, ...] | None]:
|
||||
"""The caller's fields + everything derivable → (identity, the variant they supplied), or
|
||||
(identity, None) when no variant is complete. Derived keys count for matching adapters."""
|
||||
|
||||
@@ -135,8 +135,9 @@ def linkedin_url(v: Any) -> str | None:
|
||||
v = v.strip()
|
||||
if v.startswith("http"):
|
||||
return v
|
||||
if re.match(r"^(?:[a-z]{2,3}\.)?(?:www\.)?linkedin\.com/", v, re.I): # anchored: the HOST is linkedin, not a path that mentions it
|
||||
return f"https://{v}"
|
||||
m = re.match(r"^((?:[a-z]{2,3}\.)?(?:www\.)?linkedin\.com)(/.*)$", v, re.I) # anchored: the HOST is linkedin, not a path that mentions it
|
||||
if m:
|
||||
return f"https://{m.group(1).lower()}{m.group(2)}" # lower-cased host so linkedin_handle() can derive from it
|
||||
return f"https://www.linkedin.com/in/{v.strip('/')}"
|
||||
|
||||
|
||||
|
||||
@@ -751,7 +751,7 @@ def endpoint_view(ep: dict, provider_display: str, cat: Catalog | None = None) -
|
||||
"platform_blocked": ep.get("platform_blocked") or None,
|
||||
# "no match" semantics, when the endpoint has them — an agent that reads `miss` stops
|
||||
# treating an expected empty answer as a failed call (and stops retrying it).
|
||||
"miss": ep.get("miss"),
|
||||
"miss": ({k: v for k, v in ep["miss"].items() if k != "when"} if isinstance(ep.get("miss"), dict) else ep.get("miss")),
|
||||
# Only direct-id lookups can return a marked row; discovery surfaces never include one.
|
||||
"status": ep.get("status") or None,
|
||||
"status_note": ep.get("status_note") or None,
|
||||
|
||||
+21
-2
@@ -2349,9 +2349,14 @@ def test_declared_miss_honours_when_predicate_and_never_crashes():
|
||||
|
||||
def test_prospeo_and_limadata_person_finders_declare_their_miss():
|
||||
cat = catalog_store.load()
|
||||
from treg.domain.catalog.routing.contracts import declared_miss
|
||||
for eid in ("prospeo.people.email.find", "prospeo.people.phone.find", "prospeo.people.enrich"):
|
||||
m = cat.by_id[eid]["miss"]
|
||||
assert m["status"] == 400 and "NO_MATCH" in m["when"], eid
|
||||
ep = cat.by_id[eid]
|
||||
assert ep["miss"]["status"] == 400, eid
|
||||
# evaluate the predicate, not just its spelling: a misspelt path would silently never match
|
||||
assert declared_miss(ep, 400, {"error": True, "error_code": "NO_MATCH"}), eid
|
||||
assert not declared_miss(ep, 400, {"error": True, "error_code": "INVALID_DATAPOINTS"}), eid
|
||||
assert "when" not in catalog_store.endpoint_view(ep, "Prospeo", cat)["miss"], "internal predicate leaks to agents"
|
||||
for eid in ("limadata.people.email.find.name", "limadata.people.email.find.linkedin", "limadata.people.phone.find"):
|
||||
assert cat.by_id[eid]["miss"]["status"] == 404, eid
|
||||
|
||||
@@ -2392,3 +2397,17 @@ def test_linkedin_url_only_trusts_a_linkedin_host():
|
||||
# a path that merely mentions linkedin.com is a handle-shaped string, never promoted to that host
|
||||
assert P.linkedin_url("evil.example/?linkedin.com/in/x") == "https://www.linkedin.com/in/evil.example/?linkedin.com/in/x"
|
||||
assert P.linkedin_url("uk.linkedin.com/in/x") == "https://uk.linkedin.com/in/x"
|
||||
|
||||
|
||||
def test_arena_and_router_read_the_miss_block_the_same_way():
|
||||
from treg.domain import arena
|
||||
cat = catalog_store.load()
|
||||
ep = cat.by_id["prospeo.people.email.find"]; ad = cat.adapters[ep["id"]]; contract = cat.contracts["people.email.find"]
|
||||
assert arena.classify(contract, ad, ep, 400, {"error": True, "error_code": "NO_MATCH"})[0] == "miss"
|
||||
assert arena.classify(contract, ad, ep, 400, {"error": True, "error_code": "INVALID_DATAPOINTS"})[0] == "error"
|
||||
|
||||
|
||||
def test_linkedin_url_lowercases_the_host_so_the_handle_derives():
|
||||
from treg.domain.catalog.routing import paths as P
|
||||
assert P.linkedin_url("LinkedIn.com/in/Patrick") == "https://linkedin.com/in/Patrick"
|
||||
assert P.linkedin_handle(P.linkedin_url("WWW.LinkedIn.com/in/Patrick")) == "Patrick"
|
||||
|
||||
Reference in New Issue
Block a user