fix(feedback): serialize and validate legacy feedback

CHANGELOG.md

@@ -0,0 +1,13 @@
+# Changelog
+
+## Unreleased
+
+### Fixed
+
+- Correct legacy feedback serialization: existing `request_id`/`score` arguments
+  now send `trace_id`/integer `value`. Only integer scores from −10 through 10
+  are valid; integral floats are accepted without rescaling. Fractional,
+  out-of-range, boolean, non-numeric and nonfinite scores fail before dispatch.
+- Reject unsupported nonempty comments, including whitespace-only text, instead
+  of sending them. Omitted, `None` and empty comments are omitted. Both client
+  classes retain their synchronous feedback helper and `None` return shape.

README.md

@@ -213,6 +213,33 @@ parses the gateway's `x-routeplane-*` response headers: `provider`, `trace_id`,
 `request_id`, `cache`, `guardrails`, `hedged`, `shed`, `budget_remaining`,
 `budget_warning`, `compliance_warning`, `pii_masked`, `idempotent_replayed`.
 
+## Legacy feedback
+
+Use the gateway-generated request ID from response metadata, not the provider's
+completion body ID:
+
+```python
+completion, meta = client.create_with_meta(
+    model="gpt-4o-mini",
+    messages=[{"role": "user", "content": "Hello!"}],
+)
+if meta.request_id is not None:
+    client.feedback.create(request_id=meta.request_id, score=1)
+```
+
+The helper keeps its `request_id` and `score` arguments and sends
+`{"trace_id": "req_...", "value": 1}` to `POST /v1/feedback`. Scores must be
+integers from −10 through 10. Integral floats such as `1.0` become JSON integers
+without rescaling. Fractional, out-of-range and nonfinite values raise
+`ValueError`; booleans and non-numeric types raise `TypeError`, before dispatch.
+
+Comments are unsupported: omitted, `None` or empty-string comments are omitted
+from the wire. Every nonempty comment, including whitespace-only text, raises
+`ValueError`; other comment types raise `TypeError`. Successful calls return
+`None`, acknowledging acceptance rather than durable storage or target existence.
+The feedback helper is synchronous on both `Routeplane` and `AsyncRouteplane`;
+do not `await` it.
+
 ## Prompt management
 
 Managed prompt templates are fetched, rendered, and run through the ordinary

src/routeplane/resources/feedback.py

@@ -4,7 +4,7 @@
 
 from typing import Optional
 
-from ._base import BaseResource, prune_none
+from ._base import BaseResource
 
 __all__ = ["FeedbackResource"]
 
@@ -19,6 +19,23 @@ def create(
         score: float,
         comment: Optional[str] = None,
     ) -> None:
-        """``POST /v1/feedback`` — score a request (``request_id``) with an optional note."""
-        body = prune_none({"request_id": request_id, "score": score, "comment": comment})
+        """Score a gateway request using the legacy ``trace_id``/``value`` wire.
+
+        ``score`` must be an integer from -10 through 10; integral floats are
+        accepted without rescaling. Invalid types raise ``TypeError`` and
+        invalid values raise ``ValueError`` before any request is sent.
+        Comments are unsupported: omit them or pass ``None``/``""``.
+        Returns ``None`` on acknowledgement, not a durability guarantee.
+        """
+        if isinstance(score, bool) or not isinstance(score, (int, float)):
+            raise TypeError("score must be a number (int or float), not a boolean")
+        # Check the bounds before converting: this also rejects NaN/infinity
+        # and avoids coercing huge integers or truncating fractional scores.
+        if not -10 <= score <= 10 or int(score) != score:
+            raise ValueError("score must be an integer from -10 through 10")
+        if comment is not None and not isinstance(comment, str):
+            raise TypeError("comment must be a string or None")
+        if comment is not None and comment != "":
+            raise ValueError("comment is not supported by the legacy feedback endpoint")
+        body = {"trace_id": request_id, "value": int(score)}
         self._post("feedback", json=body)

tests/test_feedback.py

@@ -0,0 +1,85 @@
+"""Legacy feedback wire and pre-dispatch validation on every exposed path."""
+
+import json
+
+import httpx
+import pytest
+import pytest_asyncio
+import respx
+
+from routeplane import AsyncRouteplane, Routeplane
+from routeplane.resources import FeedbackResource
+
+BASE = "https://gateway.example.test/v1"
+
+
+@pytest_asyncio.fixture(params=["resource", "sync_client", "async_client"])
+async def feedback(request):
+    if request.param == "resource":
+        with httpx.Client() as transport:
+            yield FeedbackResource(api_key="rp_test", base_url=BASE, http_client=transport)
+    elif request.param == "sync_client":
+        with Routeplane(api_key="rp_test", base_url=BASE) as client:
+            yield client.feedback
+    else:
+        async with AsyncRouteplane(api_key="rp_test", base_url=BASE) as client:
+            # REST helpers on AsyncRouteplane are intentionally synchronous.
+            yield client.feedback
+
+
+@pytest.mark.parametrize("score", [-10, 0, 10, -10.0, -1.0, -0.0, 1.0, 10.0])
+@pytest.mark.parametrize("comment", [{}, {"comment": None}, {"comment": ""}])
+@pytest.mark.parametrize("status", [200, 204])
+@respx.mock
+async def test_feedback_legacy_wire(feedback, score, comment, status):
+    route = respx.post(f"{BASE}/feedback").mock(return_value=httpx.Response(status))
+    assert feedback.create(request_id="req_from_gateway", score=score, **comment) is None
+    assert route.call_count == 1
+    sent = route.calls.last.request
+    body = json.loads(sent.content)
+    assert body == {"trace_id": "req_from_gateway", "value": int(score)}
+    assert type(body["value"]) is int
+    assert sent.headers["x-routeplane-api-key"] == "rp_test"
+
+
+@pytest.mark.parametrize(
+    "score", [-11, 11, -10.1, 10.1, -0.5, 0.5, float("nan"), float("inf"), float("-inf"), 10**1000]
+)
+@respx.mock
+async def test_feedback_invalid_score_value_never_dispatches(feedback, score):
+    with pytest.raises(ValueError, match="score.*integer.*-10.*10"):
+        feedback.create(request_id="req_1", score=score)
+    assert len(respx.calls) == 0
+
+
+@pytest.mark.parametrize("score", [True, False, None, "1", [], {}, 1 + 0j])
+@respx.mock
+async def test_feedback_invalid_score_type_never_dispatches(feedback, score):
+    with pytest.raises(TypeError, match="score.*number"):
+        feedback.create(request_id="req_1", score=score)
+    assert len(respx.calls) == 0
+
+
+@pytest.mark.parametrize("comment", ["note", " ", "\t", "\n", "\u200b"])
+@respx.mock
+async def test_feedback_unsupported_comment_never_dispatches(feedback, comment):
+    with pytest.raises(ValueError, match="comment.*not supported"):
+        feedback.create(request_id="req_1", score=0, comment=comment)
+    assert len(respx.calls) == 0
+
+
+@pytest.mark.parametrize("comment", [False, 0, [], {}])
+@respx.mock
+async def test_feedback_invalid_comment_type_never_dispatches(feedback, comment):
+    with pytest.raises(TypeError, match="comment"):
+        feedback.create(request_id="req_1", score=0, comment=comment)
+    assert len(respx.calls) == 0
+
+
+@respx.mock
+async def test_feedback_preserves_http_error_contract(feedback):
+    route = respx.post(f"{BASE}/feedback").mock(return_value=httpx.Response(401))
+    with pytest.raises(httpx.HTTPStatusError) as error:
+        feedback.create(request_id="req_1", score=0)
+    assert error.value.response.status_code == 401
+    assert route.call_count == 1

tests/test_meta_convenience.py

@@ -1,4 +1,9 @@
-"""Tests for ``create_with_meta`` / ``stream_with_meta`` on both clients."""
+"""Tests for ``create_with_meta`` / ``stream_with_meta`` on both clients.
+
+Inject httpx clients so respx intercepts every request. Newer OpenAI versions
+default to httpx2, which these httpx fixtures do not mock. This deliberately
+tests the supported explicit-client path without changing production defaults.
+"""
 
 import json
 
@@ -54,7 +59,7 @@ def _sse():
 @respx.mock
 def test_create_with_meta_sync():
     respx.post(CHAT).mock(return_value=httpx.Response(200, json=_COMPLETION, headers=_META_HEADERS))
-    client = Routeplane(api_key="rp_test", base_url=BASE)
+    client = Routeplane(api_key="rp_test", base_url=BASE, http_client=httpx.Client())
     try:
         completion, meta = client.create_with_meta(
             model="gpt-4o", messages=[{"role": "user", "content": "hi"}]
@@ -69,7 +74,7 @@ def test_create_with_meta_sync():
 @respx.mock
 def test_stream_with_meta_sync():
     respx.post(CHAT).mock(return_value=_sse())
-    client = Routeplane(api_key="rp_test", base_url=BASE)
+    client = Routeplane(api_key="rp_test", base_url=BASE, http_client=httpx.Client())
     try:
         stream = client.stream_with_meta(
             model="gpt-4o", messages=[{"role": "user", "content": "hi"}]
@@ -85,7 +90,7 @@ def test_stream_with_meta_sync():
 @respx.mock
 async def test_create_with_meta_async():
     respx.post(CHAT).mock(return_value=httpx.Response(200, json=_COMPLETION, headers=_META_HEADERS))
-    client = AsyncRouteplane(api_key="rp_test", base_url=BASE)
+    client = AsyncRouteplane(api_key="rp_test", base_url=BASE, http_client=httpx.AsyncClient())
     try:
         completion, meta = await client.create_with_meta(
             model="gpt-4o", messages=[{"role": "user", "content": "hi"}]
@@ -99,7 +104,7 @@ async def test_create_with_meta_async():
 @respx.mock
 async def test_stream_with_meta_async():
     respx.post(CHAT).mock(return_value=_sse())
-    client = AsyncRouteplane(api_key="rp_test", base_url=BASE)
+    client = AsyncRouteplane(api_key="rp_test", base_url=BASE, http_client=httpx.AsyncClient())
     try:
         stream = await client.stream_with_meta(
             model="gpt-4o", messages=[{"role": "user", "content": "hi"}]

tests/test_resources.py

@@ -199,15 +199,16 @@ def test_feedback_create_prunes_comment():
     route = respx.post(f"{BASE}/feedback").mock(return_value=httpx.Response(204))
     assert FeedbackResource(**_kwargs()).create(request_id="req_1", score=1.0) is None
     body = _body(route)
-    assert body == {"request_id": "req_1", "score": 1.0}
+    assert body == {"trace_id": "req_1", "value": 1}
+    assert type(body["value"]) is int
     assert "comment" not in body
 
 
 @respx.mock
 def test_feedback_create_with_comment():
-    route = respx.post(f"{BASE}/feedback").mock(return_value=httpx.Response(204))
-    FeedbackResource(**_kwargs()).create(request_id="req_1", score=0.0, comment="meh")
-    assert _body(route) == {"request_id": "req_1", "score": 0.0, "comment": "meh"}
+    with pytest.raises(ValueError, match="comment.*not supported"):
+        FeedbackResource(**_kwargs()).create(request_id="req_1", score=0.0, comment="meh")
+    assert len(respx.calls) == 0
 
 
 # --- residency -------------------------------------------------------------