From c7dda9bf9cdee692dcc19c6952ce3b4d7759ab5d Mon Sep 17 00:00:00 2001 From: Phil Merrell Date: Wed, 16 Sep 2026 18:16:57 -0600 Subject: [PATCH] =?UTF-8?q?feat(feedback):=20retry-with-correction=20?= =?UTF-8?q?=E2=80=94=20the=20consequence=20behind=20a=20thumbs=20down?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit response-feedback spec §11 PR-1's other half: a down-thumb's reason row offers "Retry with that in mind", which prefills the composer with a correction template for the reason code (new ComposerDraftService; the composer consumes drafts for its session and never submits). When the user sends it, ChatRequestService hands the added user message to MessageFeedbackService.consumePendingRetry, which PUTs retryMessageId — the message's index, never its text — onto the F# row. The write is now an update_item upsert so a later re-thumb keeps the link. Open question 1 settled as a new turn rather than /steer (which targets a running turn). The admin profile reports feedback.retried and reworkUsd (spec §7 "rework cost": the thumbed call rows plus the retry turn's consecutive assistant rows). Spec §13 updated. Co-Authored-By: Claude Fable 5.1 --- .../src/apis/app_api/admin/costs/models.py | 6 ++ .../src/apis/app_api/admin/costs/service.py | 37 +++++++++ backend/src/apis/app_api/messages/models.py | 8 ++ backend/src/apis/app_api/sessions/routes.py | 5 +- .../shared/observability/content_policy.py | 1 + backend/src/apis/shared/sessions/feedback.py | 64 +++++++++++++--- backend/src/apis/shared/sessions/models.py | 8 ++ .../admin/costs/test_drilldown_routes.py | 2 +- .../costs/test_session_profile_service.py | 23 ++++++ .../app_api/test_message_feedback_routes.py | 11 ++- backend/tests/shared/test_message_feedback.py | 24 +++++- docs/specs/response-feedback.md | 24 ++++-- .../admin/costs/models/admin-cost.models.ts | 3 + .../costs/pages/session-cost-anatomy.page.ts | 9 +++ .../costs/pages/session-profile.util.spec.ts | 7 ++ .../admin/costs/pages/session-profile.util.ts | 9 +++ .../chat-input/chat-input.component.spec.ts | 68 +++++++++++++++++ .../chat-input/chat-input.component.ts | 21 ++++++ .../message-actions.component.spec.ts | 18 ++++- .../components/message-actions.component.ts | 19 +++++ .../chat/chat-request.service.spec.ts | 13 ++++ .../services/chat/chat-request.service.ts | 10 ++- .../session/services/models/message.model.ts | 2 + .../session/composer-draft.service.spec.ts | 28 +++++++ .../session/composer-draft.service.ts | 38 ++++++++++ .../session/message-feedback.service.spec.ts | 46 ++++++++++++ .../session/message-feedback.service.ts | 75 ++++++++++++++++++- 27 files changed, 553 insertions(+), 26 deletions(-) create mode 100644 frontend/ai.client/src/app/session/services/session/composer-draft.service.spec.ts create mode 100644 frontend/ai.client/src/app/session/services/session/composer-draft.service.ts diff --git a/backend/src/apis/app_api/admin/costs/models.py b/backend/src/apis/app_api/admin/costs/models.py index a0bfe98a..35cb6871 100644 --- a/backend/src/apis/app_api/admin/costs/models.py +++ b/backend/src/apis/app_api/admin/costs/models.py @@ -457,6 +457,12 @@ class FeedbackProfile(BaseModel): down: int = 0 by_turn_class: Optional[FeedbackByTurnClass] = Field(None, alias="byTurnClass") unjoined: int = 0 + # Down-thumbs the user followed with a retry-with-correction, and what + # that rework cost: the thumbed call(s) plus the retry turn's calls + # (response-feedback spec §7 "rework cost"). ``None`` when no retry has + # a cost row to price. + retried: int = 0 + rework_usd: Optional[float] = Field(None, alias="reworkUsd") class DataCoverage(BaseModel): diff --git a/backend/src/apis/app_api/admin/costs/service.py b/backend/src/apis/app_api/admin/costs/service.py index 83a88c71..fb68dd66 100644 --- a/backend/src/apis/app_api/admin/costs/service.py +++ b/backend/src/apis/app_api/admin/costs/service.py @@ -125,6 +125,13 @@ def _join_feedback( any_turn_class = any(_turn_class(r) is not None for r in records) buckets = FeedbackByTurnClass() if any_turn_class else None profile = FeedbackProfile() + # Every cost row per assistant message index, for pricing rework. + cost_by_message: Dict[int, float] = {} + for record in records: + message_id = _as_int(record.get("messageId")) + if message_id is not None: + cost_by_message[message_id] = cost_by_message.get(message_id, 0.0) + (_record_cost(record) or 0.0) + rework_total: Optional[float] = None for row in feedback_rows: # Explicit thumbs only — implicit signals (spec §10) share the row # family but answer a different question and must never be summed in. @@ -138,6 +145,12 @@ def _join_feedback( else: profile.down += 1 message_id = _as_int(row.get("messageId")) + retry_id = _as_int(row.get("retryMessageId")) + if value == -1 and retry_id is not None: + profile.retried += 1 + rework = _rework_cost(cost_by_message, message_id, retry_id) + if rework is not None: + rework_total = (rework_total or 0.0) + rework record = by_message.get(message_id) if message_id is not None else None if record is None: profile.unjoined += 1 @@ -155,9 +168,33 @@ def _join_feedback( else: bucket.down += 1 profile.by_turn_class = buckets + profile.rework_usd = round(rework_total, 6) if rework_total is not None else None return profile +def _rework_cost( + cost_by_message: Dict[int, float], + thumbed_message_id: Optional[int], + retry_message_id: int, +) -> Optional[float]: + """Dollars spent on a down-thumbed answer plus its retry: the thumbed + message's call rows, plus the retry turn's assistant rows. The retry is + a *user* message (no cost row); its turn's assistant messages are the + consecutive indexes after it — a gap means the next user message. ``None`` + when neither side has a cost row to price.""" + found = False + total = 0.0 + if thumbed_message_id is not None and thumbed_message_id in cost_by_message: + total += cost_by_message[thumbed_message_id] + found = True + index = retry_message_id + 1 + while index in cost_by_message: + total += cost_by_message[index] + found = True + index += 1 + return total if found else None + + def _context_tokens(record: Dict[str, Any]) -> int: """True context occupancy of one call: uncached input + cached prefix + newly cached tokens (Bedrock reports the three disjointly).""" diff --git a/backend/src/apis/app_api/messages/models.py b/backend/src/apis/app_api/messages/models.py index 95d4fc00..7d70d919 100644 --- a/backend/src/apis/app_api/messages/models.py +++ b/backend/src/apis/app_api/messages/models.py @@ -137,6 +137,10 @@ class MessageFeedback(BaseModel): value: Literal[1, -1] = Field(..., description="+1 for thumbs up, -1 for thumbs down") reason: Optional[FeedbackReason] = Field(None, description="Optional reason code (never free text)") + retry_message_id: Optional[int] = Field( + None, alias="retryMessageId", ge=0, + description="Index of the user message sent as a retry-with-correction after this thumb (content-free link)", + ) updated_at: str = Field(..., alias="updatedAt", description="ISO timestamp of the latest thumb") @@ -147,6 +151,10 @@ class MessageFeedbackRequest(BaseModel): value: Literal[1, -1] = Field(..., description="+1 for thumbs up, -1 for thumbs down") reason: Optional[FeedbackReason] = Field(None, description="Optional reason code (never free text)") + retry_message_id: Optional[int] = Field( + None, alias="retryMessageId", ge=0, + description="Set when the user sent a retry-with-correction: that user message's index", + ) class MessageMetadata(BaseModel): diff --git a/backend/src/apis/app_api/sessions/routes.py b/backend/src/apis/app_api/sessions/routes.py index 5d069c9b..a2d0a57a 100644 --- a/backend/src/apis/app_api/sessions/routes.py +++ b/backend/src/apis/app_api/sessions/routes.py @@ -694,7 +694,9 @@ async def put_message_feedback_endpoint( ``message_id`` is the message's 0-based index in the conversation — the trailing number of the SPA's ``msg-{sessionId}-{index}`` id, and the - ``messageId`` the message's cost row carries. + ``messageId`` the message's cost row carries. ``retryMessageId`` in the + body links the user message sent as a retry-with-correction; it is kept + across later thumbs on the same message. """ _require_message_feedback() try: @@ -704,6 +706,7 @@ async def put_message_feedback_endpoint( message_id=message_id, value=body.value, reason=body.reason, + retry_message_id=body.retry_message_id, ) except SessionNotOwned: raise HTTPException(status_code=404, detail=f"Session not found: {session_id}") diff --git a/backend/src/apis/shared/observability/content_policy.py b/backend/src/apis/shared/observability/content_policy.py index e2586ccf..6720bf79 100644 --- a/backend/src/apis/shared/observability/content_policy.py +++ b/backend/src/apis/shared/observability/content_policy.py @@ -192,6 +192,7 @@ def is_content_bearing(path: str) -> bool: "value", "reason", "signal", + "retryMessageId", # a message index, the retry-with-correction link "updatedAt", ) diff --git a/backend/src/apis/shared/sessions/feedback.py b/backend/src/apis/shared/sessions/feedback.py index 1400b163..15ed464e 100644 --- a/backend/src/apis/shared/sessions/feedback.py +++ b/backend/src/apis/shared/sessions/feedback.py @@ -15,13 +15,19 @@ SK: F#{session_id}#{message_id} GSI_PK: SESSION#{session_id} (SessionLookupIndex) GSI_SK: F#{message_id} - sessionId, messageId, userId, value, reason?, signal, updatedAt, ttl + sessionId, messageId, userId, value, reason?, signal, retryMessageId?, updatedAt, ttl ``signal`` is ``"explicit"`` for a thumb. ``docs/specs/response-feedback.md`` §10 adds implicit signals (copy, continue, edit-and-resend, abandonment) to this same row family under ``signal: "implicit"``; every reader here filters to explicit rows so that phase needs no backfill and the two are never summed. +``retryMessageId`` is the index of the user message the SPA sent as a +*retry with correction* after a down-thumb (response-feedback spec §7 "the +retry loop", §11 PR-1's consequence). It is a link, never the correction's +text: the profile counts retries and prices the rework from the cost rows +the link points at. A later re-thumb without the field keeps the link. + ``messageId`` is the same 0-based index the ``C#`` cost row carries for the assistant message, so an admin read joins feedback to the call's turn class (``hasDocuments`` / ``documentDigests`` / ``documentReads``, #1137) on @@ -82,9 +88,11 @@ def _keys(user_id: str, session_id: str, message_id: int) -> Dict[str, str]: def _to_model(item: Dict[str, Any]) -> MessageFeedback: + retry = item.get("retryMessageId") return MessageFeedback( value=int(item.get("value", 0)), reason=item.get("reason") if item.get("reason") in FEEDBACK_REASONS else None, + retry_message_id=int(retry) if retry is not None else None, updated_at=str(item.get("updatedAt", "")), ) @@ -124,9 +132,16 @@ async def put_message_feedback( message_id: int, value: int, reason: Optional[str] = None, + retry_message_id: Optional[int] = None, ) -> MessageFeedback: """Write (or replace) this user's thumb on one message. + An upsert (``update_item``): ``value`` / ``reason`` / ``updatedAt`` are + replaced on every call, ``retryMessageId`` is set when given and kept + otherwise, so a user who thumbs again after retrying does not lose the + link. ``ReturnValues=ALL_OLD`` tells us what a replace replaced, so the + session rollups move by the delta rather than double-counting. + Raises :class:`SessionNotOwned` when the session is not this user's, and ``ValueError`` on a value outside ``{1, -1}`` or a reason outside :data:`FEEDBACK_REASONS` — the route's request model already rejects @@ -136,35 +151,60 @@ async def put_message_feedback( raise ValueError("feedback value must be 1 or -1") if reason is not None and reason not in FEEDBACK_REASONS: raise ValueError("feedback reason must be one of the fixed codes") + if retry_message_id is not None and (not isinstance(retry_message_id, int) or retry_message_id < 0): + raise ValueError("retryMessageId must be a non-negative message index") if is_preview_session(session_id): # Preview sessions persist nothing; echo the thumb so the UI is consistent. - return MessageFeedback(value=value, reason=reason, updated_at=_now()) + return MessageFeedback(value=value, reason=reason, retry_message_id=retry_message_id, updated_at=_now()) table = _table() session_sk = await _owned_session_sk(session_id, user_id, table) now = _now() ttl = int((datetime.now(timezone.utc) + timedelta(days=FEEDBACK_TTL_DAYS)).timestamp()) - item: Dict[str, Any] = { - **_keys(user_id, session_id, message_id), + sets = { "GSI_PK": f"SESSION#{session_id}", "GSI_SK": f"F#{message_id}", "sessionId": session_id, "messageId": int(message_id), "userId": user_id, - "value": int(value), - "signal": "explicit", + "#value": int(value), + "#signal": "explicit", "updatedAt": now, - "ttl": ttl, + "#ttl": ttl, } if reason: - item["reason"] = reason - - # ReturnValues=ALL_OLD tells us what a replace is replacing, so the - # session rollups move by the delta rather than double-counting. - response = table.put_item(Item=item, ReturnValues="ALL_OLD") + sets["reason"] = reason + if retry_message_id is not None: + sets["retryMessageId"] = int(retry_message_id) + names = {"#value": "value", "#signal": "signal", "#ttl": "ttl"} + values: Dict[str, Any] = {} + set_parts = [] + for i, (attr, val) in enumerate(sets.items()): + placeholder = f":v{i}" + values[placeholder] = val + set_parts.append(f"{attr} = {placeholder}") + expression = "SET " + ", ".join(set_parts) + if not reason: + expression += " REMOVE reason" + + response = table.update_item( + Key=_keys(user_id, session_id, message_id), + UpdateExpression=expression, + ExpressionAttributeNames=names, + ExpressionAttributeValues=values, + ReturnValues="ALL_OLD", + ) previous = response.get("Attributes") or {} previous_value = int(previous.get("value", 0)) if previous else 0 + item: Dict[str, Any] = { + "value": int(value), + "reason": reason, + "retryMessageId": ( + int(retry_message_id) if retry_message_id is not None else previous.get("retryMessageId") + ), + "updatedAt": now, + } up = (1 if value == 1 else 0) - (1 if previous_value == 1 else 0) down = (1 if value == -1 else 0) - (1 if previous_value == -1 else 0) diff --git a/backend/src/apis/shared/sessions/models.py b/backend/src/apis/shared/sessions/models.py index ae3c3d9d..20e35922 100644 --- a/backend/src/apis/shared/sessions/models.py +++ b/backend/src/apis/shared/sessions/models.py @@ -662,6 +662,10 @@ class MessageFeedback(BaseModel): value: Literal[1, -1] = Field(..., description="+1 for thumbs up, -1 for thumbs down") reason: Optional[FeedbackReason] = Field(None, description="Optional reason code (never free text)") + retry_message_id: Optional[int] = Field( + None, alias="retryMessageId", ge=0, + description="Index of the user message sent as a retry-with-correction after this thumb (content-free link)", + ) updated_at: str = Field(..., alias="updatedAt", description="ISO timestamp of the latest thumb") @@ -672,6 +676,10 @@ class MessageFeedbackRequest(BaseModel): value: Literal[1, -1] = Field(..., description="+1 for thumbs up, -1 for thumbs down") reason: Optional[FeedbackReason] = Field(None, description="Optional reason code (never free text)") + retry_message_id: Optional[int] = Field( + None, alias="retryMessageId", ge=0, + description="Set when the user sent a retry-with-correction: that user message's index", + ) class MessageMetadata(BaseModel): diff --git a/backend/tests/apis/app_api/admin/costs/test_drilldown_routes.py b/backend/tests/apis/app_api/admin/costs/test_drilldown_routes.py index 6311b1f1..d74ff872 100644 --- a/backend/tests/apis/app_api/admin/costs/test_drilldown_routes.py +++ b/backend/tests/apis/app_api/admin/costs/test_drilldown_routes.py @@ -107,7 +107,7 @@ def test_session_profile_returns_200(): "prefixTokens": False, "windowTrim": False, "compactionEvents": False, "feedback": False, } - assert body["feedback"] == {"up": 0, "down": 0, "byTurnClass": None, "unjoined": 0} + assert body["feedback"] == {"up": 0, "down": 0, "byTurnClass": None, "unjoined": 0, "retried": 0, "reworkUsd": None} service.get_session_profile.assert_awaited_once_with("s1") diff --git a/backend/tests/apis/app_api/admin/costs/test_session_profile_service.py b/backend/tests/apis/app_api/admin/costs/test_session_profile_service.py index 530b8131..5a5b75be 100644 --- a/backend/tests/apis/app_api/admin/costs/test_session_profile_service.py +++ b/backend/tests/apis/app_api/admin/costs/test_session_profile_service.py @@ -333,6 +333,29 @@ async def test_no_feedback_rows_falls_back_to_rollups_and_coverage_is_honest(): assert p.feedback.by_turn_class is None +@pytest.mark.asyncio +async def test_retries_are_counted_and_rework_is_priced_from_the_cost_rows(): + # Turn A: assistant 1 (thumbed down, $0.10). Retry sent as user message 2; + # its turn is assistant 3 + 4 ($0.20 + $0.05, a tool round trip). User 5, + # assistant 6 ($0.99) is the NEXT turn and must not be counted. + records = [_call(1, cost=0.10), _call(3, cost=0.20), _call(4, cost=0.05), _call(6, cost=0.99)] + feedback = [{**_feedback(1, -1, "wrong"), "retryMessageId": 2}, _feedback(6, 1)] + p = await _service_with_feedback(_row(), records, feedback).get_session_profile("s1") + assert p.feedback.retried == 1 + assert p.feedback.rework_usd == 0.35 + assert (p.feedback.up, p.feedback.down) == (1, 1) + + # A retry whose turn has no cost rows yet (still streaming) counts, but prices only the thumbed side. + feedback = [{**_feedback(1, -1, "wrong"), "retryMessageId": 7}] + p = await _service_with_feedback(_row(), records, feedback).get_session_profile("s1") + assert p.feedback.retried == 1 and p.feedback.rework_usd == 0.10 + + # Nothing priced at all → None, not 0. + feedback = [{**_feedback(9, -1), "retryMessageId": 10}] + p = await _service_with_feedback(_row(), records, feedback).get_session_profile("s1") + assert p.feedback.retried == 1 and p.feedback.rework_usd is None + + @pytest.mark.asyncio async def test_implicit_signal_rows_are_never_summed_into_the_thumb_counts(): records = [_call(0)] diff --git a/backend/tests/apis/app_api/test_message_feedback_routes.py b/backend/tests/apis/app_api/test_message_feedback_routes.py index f2b7ed39..605af26c 100644 --- a/backend/tests/apis/app_api/test_message_feedback_routes.py +++ b/backend/tests/apis/app_api/test_message_feedback_routes.py @@ -48,7 +48,16 @@ def test_put_stores_and_echoes_the_thumb(monkeypatch): assert resp.status_code == 200 assert resp.json() == {"value": -1, "reason": "instructions", "updatedAt": "2026-09-16T00:00:00Z"} - put.assert_awaited_once_with(session_id="s1", user_id="user-1", message_id=3, value=-1, reason="instructions") + put.assert_awaited_once_with(session_id="s1", user_id="user-1", message_id=3, value=-1, reason="instructions", retry_message_id=None) + + +def test_put_forwards_the_retry_link(monkeypatch): + put = AsyncMock(return_value=MessageFeedback(value=-1, retry_message_id=4, updated_at="t")) + client = _client(monkeypatch, put=put) + resp = client.put("/sessions/s1/messages/3/feedback", json={"value": -1, "retryMessageId": 4}) + assert resp.status_code == 200 and resp.json()["retryMessageId"] == 4 + assert put.await_args.kwargs["retry_message_id"] == 4 + assert client.put("/sessions/s1/messages/3/feedback", json={"value": -1, "retryMessageId": -1}).status_code == 422 def test_put_rejects_free_text_and_out_of_range_values(monkeypatch): diff --git a/backend/tests/shared/test_message_feedback.py b/backend/tests/shared/test_message_feedback.py index c4ea26c5..aae508dd 100644 --- a/backend/tests/shared/test_message_feedback.py +++ b/backend/tests/shared/test_message_feedback.py @@ -73,7 +73,7 @@ async def test_put_writes_one_row_keyed_beside_the_cost_row(table): assert "ttl" in row # Content-free: nothing on the row but ids, a number, a timestamp and keys. assert row["signal"] == "explicit" - assert set(row) <= {"PK", "SK", "GSI_PK", "GSI_SK", "sessionId", "messageId", "userId", "value", "signal", "updatedAt", "ttl"} + assert set(row) <= {"PK", "SK", "GSI_PK", "GSI_SK", "sessionId", "messageId", "userId", "value", "signal", "retryMessageId", "updatedAt", "ttl"} @pytest.mark.asyncio @@ -97,6 +97,28 @@ async def test_second_thumb_replaces_the_first_and_rollups_follow(table): assert _feedback_items()[0]["reason"] == "wrong" +@pytest.mark.asyncio +async def test_retry_link_is_set_once_and_kept_across_later_thumbs(table): + """Retry-with-correction (response-feedback §7): the row links the user + message the correction was sent as, and a later re-thumb without the + field must not drop it.""" + first = await fb.put_message_feedback(SESSION, OWNER, 3, -1, reason="wrong") + assert first.retry_message_id is None + linked = await fb.put_message_feedback(SESSION, OWNER, 3, -1, reason="wrong", retry_message_id=4) + assert linked.retry_message_id == 4 + assert int(_feedback_items()[0]["retryMessageId"]) == 4 + + # Re-thumb (say, the retry was better and they flip to up): link stays. + again = await fb.put_message_feedback(SESSION, OWNER, 3, 1) + assert again.value == 1 and again.retry_message_id == 4 + row = _feedback_items()[0] + assert int(row["retryMessageId"]) == 4 and "reason" not in row + assert _session_row()["thumbsUp"] == 1 and _session_row()["thumbsDown"] == 0 + + with pytest.raises(ValueError): + await fb.put_message_feedback(SESSION, OWNER, 3, -1, retry_message_id=-1) + + @pytest.mark.asyncio async def test_delete_removes_the_row_and_decrements(table): await fb.put_message_feedback(SESSION, OWNER, 1, -1, reason="tool_failed") diff --git a/docs/specs/response-feedback.md b/docs/specs/response-feedback.md index b8aa0269..8c1ab116 100644 --- a/docs/specs/response-feedback.md +++ b/docs/specs/response-feedback.md @@ -1,9 +1,10 @@ # Response feedback **Status:** PARTIALLY BUILT — capture and the read model shipped in PR #1142 -(2026-09-16, as document-context-offload PR-7); the consequence (§11 PR-1's -retry-with-correction), implicit signals and eval sampling are not built. -See §13. Written 2026-09-04 from the "how would we benefit?" conversation. +(2026-09-16, as document-context-offload PR-7) and the consequence +(retry-with-correction) in the PR stacked on it; §11 PR-1 is therefore +complete. Implicit signals and eval sampling are not built. See §13. Written +2026-09-04 from the "how would we benefit?" conversation. **Refs:** `docs/specs/agentcore-evaluations-spike-findings.md` (the eval harness this feeds), `docs/specs/mid-turn-steering.md` (the injection path Phase 1 reuses), `docs/specs/agent-marketplace.md` D15 (the *other* feedback @@ -353,8 +354,21 @@ points: `agentSwitched`, skills) are further buckets in the same join loop. - **Open question 4 settled**: preview sessions echo the thumb and persist nothing, matching the `D#` write. Preview data never reaches aggregates. -- **Not built**: retry-with-correction (§11 PR-1's consequence — open - question 1 still stands), implicit signals (PR-2), eval sampling (PR-4), +- **Retry-with-correction (the consequence), second PR.** Open question 1 + settled: **a new turn, not `/steer`** — the steer path targets a *running* + turn through the lease row, and a finished turn is corrected by an + ordinary next message. The down-thumb reason row offers *Retry with that + in mind*, which prefills the composer (`ComposerDraftService`) with a + correction template for the reason code; the user edits and sends it as a + normal message, so it goes where messages go (AgentCore Memory) and never + touches the metadata table. When that message is added, the send path + links it to the thumb as `retryMessageId` on the `F#` row — an index, + never the text — and a later re-thumb keeps the link. This is open + question 2's answer for now: the pair is *recorded* without its content, + so Phase 6 can find it once the consent decision is made. The profile + reports `feedback.retried` and `reworkUsd` (§7 "rework cost": the thumbed + call rows plus the retry turn's consecutive assistant rows). +- **Not built**: implicit signals (PR-2), eval sampling (PR-4), author/marketplace surfaces (PR-5), the report-dialog escape hatch in the reason row. diff --git a/frontend/ai.client/src/app/admin/costs/models/admin-cost.models.ts b/frontend/ai.client/src/app/admin/costs/models/admin-cost.models.ts index 364f0f5b..c5735474 100644 --- a/frontend/ai.client/src/app/admin/costs/models/admin-cost.models.ts +++ b/frontend/ai.client/src/app/admin/costs/models/admin-cost.models.ts @@ -400,6 +400,9 @@ export interface FeedbackProfile { down: number; byTurnClass?: Record | null; unjoined?: number; + /** Down-thumbs followed by a retry-with-correction, and what the rework cost. */ + retried?: number; + reworkUsd?: number | null; } /** The content-free diagnostic profile of one conversation. */ diff --git a/frontend/ai.client/src/app/admin/costs/pages/session-cost-anatomy.page.ts b/frontend/ai.client/src/app/admin/costs/pages/session-cost-anatomy.page.ts index 450a9d2f..8a05b9e5 100644 --- a/frontend/ai.client/src/app/admin/costs/pages/session-cost-anatomy.page.ts +++ b/frontend/ai.client/src/app/admin/costs/pages/session-cost-anatomy.page.ts @@ -37,6 +37,7 @@ import { buildDiagnosticJson, downRate, feedbackByTurnClassLine, + feedbackRetryLine, formatBytes, formatEvidenceValue, humanizeKey, @@ -245,6 +246,10 @@ import { · turn class not tracked }

+ @if (feedbackRetryLine(); as retries) { + +

{{ retries }}

+ } } @else {

—

not tracked

@@ -813,6 +818,10 @@ export class SessionCostAnatomyPage { return feedback ? downRate(feedback) : null; }); + readonly feedbackRetryLine = computed(() => + this.profileResource.hasValue() ? feedbackRetryLine(this.profileResource.value().feedback) : null, + ); + readonly feedbackTurnClassLine = computed(() => this.profileResource.hasValue() ? feedbackByTurnClassLine(this.profileResource.value().feedback) : null, ); diff --git a/frontend/ai.client/src/app/admin/costs/pages/session-profile.util.spec.ts b/frontend/ai.client/src/app/admin/costs/pages/session-profile.util.spec.ts index 9763d7ef..ec9ad88a 100644 --- a/frontend/ai.client/src/app/admin/costs/pages/session-profile.util.spec.ts +++ b/frontend/ai.client/src/app/admin/costs/pages/session-profile.util.spec.ts @@ -4,6 +4,7 @@ import { cleanCeiling, downRate, feedbackByTurnClassLine, + feedbackRetryLine, formatBytes, formatEvidenceValue, formatTokensShort, @@ -43,6 +44,12 @@ describe('session-profile.util', () => { expect(line).toBe('full 50% of 2 · digest 0% of 2 · no docs 100% of 1'); }); + it('feedbackRetryLine names retries and prices rework only when known', () => { + expect(feedbackRetryLine({ up: 0, down: 1 })).toBeNull(); + expect(feedbackRetryLine({ up: 0, down: 1, retried: 1 })).toBe('1 retried'); + expect(feedbackRetryLine({ up: 0, down: 2, retried: 2, reworkUsd: 0.351 })).toBe('2 retried · $0.35 rework'); + }); + it('feedbackByTurnClassLine is null when the turn class is not tracked', () => { expect(feedbackByTurnClassLine({ up: 1, down: 1, byTurnClass: null })).toBeNull(); expect(feedbackByTurnClassLine({ up: 1, down: 1 })).toBeNull(); diff --git a/frontend/ai.client/src/app/admin/costs/pages/session-profile.util.ts b/frontend/ai.client/src/app/admin/costs/pages/session-profile.util.ts index 7463f493..eb7a63ff 100644 --- a/frontend/ai.client/src/app/admin/costs/pages/session-profile.util.ts +++ b/frontend/ai.client/src/app/admin/costs/pages/session-profile.util.ts @@ -232,6 +232,15 @@ export function downRate(counts: { up: number; down: number }): number | null { * (`full 50% of 4 · digest 0% of 2`). Classes with no thumbs are skipped; * null when the turn class is not tracked or nothing was thumbed. */ +/** `2 retried · $0.35 rework`, or null when nothing was retried. */ +export function feedbackRetryLine(feedback: FeedbackProfile | null | undefined): string | null { + const retried = feedback?.retried ?? 0; + if (retried === 0) return null; + const parts = [`${retried} retried`]; + if (feedback?.reworkUsd != null) parts.push(`$${feedback.reworkUsd.toFixed(2)} rework`); + return parts.join(' · '); +} + export function feedbackByTurnClassLine(feedback: FeedbackProfile | null | undefined): string | null { const by = feedback?.byTurnClass; if (!by) return null; diff --git a/frontend/ai.client/src/app/session/components/chat-input/chat-input.component.spec.ts b/frontend/ai.client/src/app/session/components/chat-input/chat-input.component.spec.ts index 0e9b5732..12bce0f3 100644 --- a/frontend/ai.client/src/app/session/components/chat-input/chat-input.component.spec.ts +++ b/frontend/ai.client/src/app/session/components/chat-input/chat-input.component.spec.ts @@ -10,6 +10,7 @@ import { ToastService } from '../../../services/toast/toast.service'; import { ToolService } from '../../../services/tool/tool.service'; import { VoiceChatService } from '../../services/voice'; import { SteeringService } from '../../services/chat/steering.service'; +import { ComposerDraftService } from '../../services/session/composer-draft.service'; import { ChatInputComponent } from './chat-input.component'; const AGENTS: MentionableAgent[] = [ @@ -1238,3 +1239,70 @@ describe('ChatInputComponent — rotating discovery hints', () => { expect(textarea.getAttribute('placeholder')).toContain('when this response finishes'); }); }); + +describe('ChatInputComponent — composer drafts (feedback retry-with-correction)', () => { + let fixture: ComponentFixture; + let component: ChatInputComponent; + + beforeEach(async () => { + await TestBed.configureTestingModule({ + imports: [ChatInputComponent], + providers: [ + { provide: AgentMentionService, useClass: MentionServiceStub }, + { provide: SkillCommandService, useClass: SkillCommandServiceStub }, + { + provide: FileUploadService, + useValue: { + pendingUploadsList: signal([]), + hasActivePendingUploads: signal(false), + readyUploadIds: signal([]), + clearReadyUploads: () => undefined, + clearPendingUpload: () => undefined, + }, + }, + { provide: ToastService, useValue: { error: () => undefined, warning: () => undefined, info: () => undefined } }, + { provide: ToolService, useValue: {} }, + { + provide: VoiceChatService, + useValue: { status: signal('idle'), isVoiceActive: signal(false), agentTranscript: signal('') }, + }, + { provide: SystemPromptsService, useValue: { activePrompt: signal(null) } }, + { provide: Router, useValue: { navigate: () => Promise.resolve(true) } }, + { provide: SteeringService, useClass: SteeringServiceStub }, + ], + }) + .overrideComponent(ChatInputComponent, { set: { imports: [], schemas: [NO_ERRORS_SCHEMA] } }) + .compileComponents(); + + fixture = TestBed.createComponent(ChatInputComponent); + component = fixture.componentInstance; + fixture.componentRef.setInput('showFileControls', false); + fixture.componentRef.setInput('showVoiceControl', false); + fixture.componentRef.setInput('autoFocus', false); + fixture.componentRef.setInput('sessionId', 's1'); + fixture.detectChanges(); + }); + + it('takes a draft for its own session into the textarea without submitting', () => { + const drafts = TestBed.inject(ComposerDraftService); + let submitted = 0; + component.messageSubmitted.subscribe(() => submitted++); + + drafts.request('s1', 'That answer ignored my instructions. '); + fixture.detectChanges(); + + expect(component.userInput()).toBe('That answer ignored my instructions. '); + const textarea = fixture.nativeElement.querySelector('textarea') as HTMLTextAreaElement; + expect(textarea.value).toBe('That answer ignored my instructions. '); + expect(drafts.pending()).toBeNull(); + expect(submitted).toBe(0); + }); + + it('leaves another session\'s draft alone', () => { + const drafts = TestBed.inject(ComposerDraftService); + drafts.request('s2', 'not mine'); + fixture.detectChanges(); + expect(component.userInput()).toBe(''); + expect(drafts.pending()?.text).toBe('not mine'); + }); +}); diff --git a/frontend/ai.client/src/app/session/components/chat-input/chat-input.component.ts b/frontend/ai.client/src/app/session/components/chat-input/chat-input.component.ts index fde8962a..f9e4f745 100644 --- a/frontend/ai.client/src/app/session/components/chat-input/chat-input.component.ts +++ b/frontend/ai.client/src/app/session/components/chat-input/chat-input.component.ts @@ -55,6 +55,7 @@ import { } from '../../../services/skill/skill-command.service'; import { SkillCommandMenuComponent } from './skill-command-menu.component'; import { SteeringService } from '../../services/chat/steering.service'; +import { ComposerDraftService } from '../../services/session/composer-draft.service'; // Must stay in sync with the inline min-height/max-height on the textarea in // chat-input.component.html. @@ -158,6 +159,7 @@ export class ChatInputComponent { private readonly fileUploadService = inject(FileUploadService); private readonly toastService = inject(ToastService); private readonly steering = inject(SteeringService); + private readonly composerDraft = inject(ComposerDraftService); private readonly toolService = inject(ToolService); private readonly voiceChatService = inject(VoiceChatService); protected readonly systemPromptsService = inject(SystemPromptsService); @@ -614,6 +616,25 @@ export class ChatInputComponent { } }); + // A feature (today: the feedback retry-with-correction) can hand this + // composer a draft for its session. Set it, size the textarea to it and + // focus so the user edits and sends; never submit on their behalf. + effect(() => { + const draft = this.composerDraft.pending(); + const sessionId = untracked(this.sessionId); + if (!draft || draft.sessionId !== sessionId) return; + const taken = untracked(() => this.composerDraft.consume(sessionId)); + if (!taken) return; + this.userInput.set(taken.text); + const textarea = this.messageInput()?.nativeElement; + if (textarea) { + textarea.value = taken.text; + this.autoResize(textarea); + textarea.focus(); + textarea.setSelectionRange(taken.text.length, taken.text.length); + } + }); + // Mirror the queue into SteeringService so the resume path can carry it // into the turn it restarts without reaching into this component. effect(() => { diff --git a/frontend/ai.client/src/app/session/components/message-list/components/message-actions.component.spec.ts b/frontend/ai.client/src/app/session/components/message-list/components/message-actions.component.spec.ts index 7c26e893..7c5bc927 100644 --- a/frontend/ai.client/src/app/session/components/message-list/components/message-actions.component.spec.ts +++ b/frontend/ai.client/src/app/session/components/message-list/components/message-actions.component.spec.ts @@ -130,6 +130,10 @@ class FakeFeedbackService { current: MessageFeedback | null = null; set: Array<{ id: string; value: 1 | -1; reason?: string }> = []; cleared: string[] = []; + retried: string[] = []; + requestRetry(message: Message): void { + this.retried.push(message.id); + } feedbackFor(): MessageFeedback | null { return this.current; } @@ -207,7 +211,8 @@ describe('MessageActionsComponent — thumbs feedback', () => { fixture.detectChanges(); const group = fixture.nativeElement.querySelector('[role="group"]'); expect(group).not.toBeNull(); - const chips = Array.from(group.querySelectorAll('button')) as HTMLButtonElement[]; + // Reason chips are the pressable ones; the Retry button shares the group but is not a reason. + const chips = Array.from(group.querySelectorAll('button[aria-pressed]')) as HTMLButtonElement[]; expect(chips.map((c) => c.textContent!.trim())).toEqual([ 'Wrong or made up', 'Ignored instructions', @@ -220,9 +225,20 @@ describe('MessageActionsComponent — thumbs feedback', () => { expect(feedback.set).toEqual([{ id: 'msg-sess-1-3', value: -1, reason: 'instructions' }]); }); + it('a thumbs down offers "Retry with that in mind", which asks the service to draft a correction', () => { + feedback.current = { value: -1, reason: 'wrong', updatedAt: 't' }; + fixture.detectChanges(); + const retry = fixture.nativeElement.querySelector('button[aria-label="Retry with that in mind"]') as HTMLButtonElement; + expect(retry).not.toBeNull(); + retry.click(); + expect(feedback.retried).toEqual(['msg-sess-1-3']); + expect(feedback.set).toEqual([]); + }); + it('reason codes stay hidden on a thumbs up', () => { feedback.current = { value: 1, updatedAt: 't' }; fixture.detectChanges(); expect(fixture.nativeElement.querySelector('[role="group"]')).toBeNull(); + expect(fixture.nativeElement.querySelector('button[aria-label="Retry with that in mind"]')).toBeNull(); }); }); diff --git a/frontend/ai.client/src/app/session/components/message-list/components/message-actions.component.ts b/frontend/ai.client/src/app/session/components/message-list/components/message-actions.component.ts index 5199430a..a329651e 100644 --- a/frontend/ai.client/src/app/session/components/message-list/components/message-actions.component.ts +++ b/frontend/ai.client/src/app/session/components/message-list/components/message-actions.component.ts @@ -95,6 +95,18 @@ import { TooltipDirective } from '../../../../components/tooltip'; {{ reasonLabels[reason] }} } + } } @@ -237,6 +249,13 @@ export class MessageActionsComponent { } } + /** The consequence: draft a correction for this thumb into the composer. */ + retry(): void { + const last = this.lastMessage(); + if (!last || this.feedbackValue() !== -1) return; + this.feedbackService.requestRetry(last); + } + pickReason(reason: FeedbackReason): void { const last = this.lastMessage(); if (!last || this.feedbackValue() !== -1) return; diff --git a/frontend/ai.client/src/app/session/services/chat/chat-request.service.spec.ts b/frontend/ai.client/src/app/session/services/chat/chat-request.service.spec.ts index baa6945b..4e199a44 100644 --- a/frontend/ai.client/src/app/session/services/chat/chat-request.service.spec.ts +++ b/frontend/ai.client/src/app/session/services/chat/chat-request.service.spec.ts @@ -5,6 +5,7 @@ import { ChatRequestService } from './chat-request.service'; import { ChatHttpService } from './chat-http.service'; import { ChatStateService } from './chat-state.service'; import { MessageMapService } from '../session/message-map.service'; +import { MessageFeedbackService } from '../session/message-feedback.service'; import { SessionService } from '../session/session.service'; import { UserService } from '../../../auth/user.service'; import { ModelService } from '../model/model.service'; @@ -60,6 +61,7 @@ describe('ChatRequestService', () => { { provide: ChatStateService, useValue: { setChatLoading: vi.fn(), setLastTurnContinuable: vi.fn(), setLastTurnInterrupted: vi.fn(), setViewedSession: vi.fn() } }, { provide: MessageMapService, useValue: { addUserMessage: vi.fn(), startStreaming: vi.fn(), beginContinuationStreaming: vi.fn(), endStreaming: vi.fn(), reloadMessagesForSession: vi.fn().mockResolvedValue(undefined) } }, { provide: SessionService, useValue: { addSessionToCache: vi.fn() } }, + { provide: MessageFeedbackService, useValue: { consumePendingRetry: vi.fn() } }, { provide: UserService, useValue: { getUser: vi.fn().mockReturnValue({ user_id: 'user1' }) } }, { provide: ModelService, useValue: mockModelService }, { provide: ToolService, useValue: mockToolService }, @@ -402,6 +404,17 @@ describe('ChatRequestService', () => { expect(messageMap.startStreaming).toHaveBeenCalledWith('preview-abc'); }); + it('offers the added user message to the feedback service as a possible retry', async () => { + const messageMap = TestBed.inject(MessageMapService) as any; + const feedback = TestBed.inject(MessageFeedbackService) as any; + const added = { id: 'msg-preview-abc-2', role: 'user', content: [] }; + messageMap.addUserMessage.mockReturnValue(added); + + await service.submitPreviewRequest(preview); + + expect(feedback.consumePendingRetry).toHaveBeenCalledWith('preview-abc', added); + }); + it('forwards file uploads', async () => { await service.submitPreviewRequest({ ...preview, fileUploadIds: ['up-1'] }); diff --git a/frontend/ai.client/src/app/session/services/chat/chat-request.service.ts b/frontend/ai.client/src/app/session/services/chat/chat-request.service.ts index f3e96769..93b0abe6 100644 --- a/frontend/ai.client/src/app/session/services/chat/chat-request.service.ts +++ b/frontend/ai.client/src/app/session/services/chat/chat-request.service.ts @@ -4,6 +4,7 @@ import { v4 as uuidv4 } from 'uuid'; import { ChatStateService } from './chat-state.service'; import { ChatHttpService } from './chat-http.service'; import { MessageMapService } from '../session/message-map.service'; +import { MessageFeedbackService } from '../session/message-feedback.service'; import { SessionService } from '../session/session.service'; import { UserService } from '../../../auth/user.service'; import { ModelService } from '../model/model.service'; @@ -41,6 +42,7 @@ export class ChatRequestService implements OnDestroy { private chatHttpService = inject(ChatHttpService); private chatStateService = inject(ChatStateService); private messageMapService = inject(MessageMapService); + private messageFeedbackService = inject(MessageFeedbackService); private sessionService = inject(SessionService); private userService = inject(UserService); private modelService = inject(ModelService); @@ -124,7 +126,10 @@ export class ChatRequestService implements OnDestroy { const fileAttachments = this.getFileAttachments(fileUploadIds); // Create and add user message with file attachments - this.messageMapService.addUserMessage(sessionId, userInput, fileAttachments); + const userMessage = this.messageMapService.addUserMessage(sessionId, userInput, fileAttachments); + // If this send is the retry a down-thumb asked for, link it to the thumb + // (an index on the feedback row — never the text). + this.messageFeedbackService.consumePendingRetry(sessionId, userMessage); // Start streaming for this conversation this.messageMapService.startStreaming(sessionId); @@ -215,7 +220,8 @@ export class ChatRequestService implements OnDestroy { this.chatStateService.setChatLoading(sessionId, true); const fileAttachments = this.getFileAttachments(fileUploadIds); - this.messageMapService.addUserMessage(sessionId, message, fileAttachments); + const userMessage = this.messageMapService.addUserMessage(sessionId, message, fileAttachments); + this.messageFeedbackService.consumePendingRetry(sessionId, userMessage); this.messageMapService.startStreaming(sessionId); // NOTE: Field name is 'rag_assistant_id' to avoid collision with AWS Bedrock diff --git a/frontend/ai.client/src/app/session/services/models/message.model.ts b/frontend/ai.client/src/app/session/services/models/message.model.ts index 71fa29dd..791ad5c3 100644 --- a/frontend/ai.client/src/app/session/services/models/message.model.ts +++ b/frontend/ai.client/src/app/session/services/models/message.model.ts @@ -116,6 +116,8 @@ export interface MessageFeedback { /** +1 thumbs up, -1 thumbs down */ value: 1 | -1; reason?: FeedbackReason; + /** Index of the user message sent as a retry-with-correction after this thumb. */ + retryMessageId?: number; updatedAt: string; } diff --git a/frontend/ai.client/src/app/session/services/session/composer-draft.service.spec.ts b/frontend/ai.client/src/app/session/services/session/composer-draft.service.spec.ts new file mode 100644 index 00000000..6c474bd8 --- /dev/null +++ b/frontend/ai.client/src/app/session/services/session/composer-draft.service.spec.ts @@ -0,0 +1,28 @@ +import { TestBed } from '@angular/core/testing'; +import { describe, it, expect, beforeEach } from 'vitest'; +import { ComposerDraftService } from './composer-draft.service'; + +describe('ComposerDraftService', () => { + let service: ComposerDraftService; + + beforeEach(() => { + TestBed.configureTestingModule({}); + service = TestBed.inject(ComposerDraftService); + }); + + it('hands a draft only to the composer of the session it was requested for', () => { + service.request('s1', 'redo it'); + expect(service.consume('s2')).toBeNull(); + expect(service.pending()?.text).toBe('redo it'); + expect(service.consume('s1')?.text).toBe('redo it'); + expect(service.pending()).toBeNull(); + expect(service.consume('s1')).toBeNull(); + }); + + it('two identical requests are distinct', () => { + service.request('s1', 'x'); + const first = service.pending()!.nonce; + service.request('s1', 'x'); + expect(service.pending()!.nonce).not.toBe(first); + }); +}); diff --git a/frontend/ai.client/src/app/session/services/session/composer-draft.service.ts b/frontend/ai.client/src/app/session/services/session/composer-draft.service.ts new file mode 100644 index 00000000..644e06a0 --- /dev/null +++ b/frontend/ai.client/src/app/session/services/session/composer-draft.service.ts @@ -0,0 +1,38 @@ +import { Injectable, signal } from '@angular/core'; + +/** A request to put text into the composer for the user to edit and send. */ +export interface ComposerDraft { + /** The session the draft belongs to; `null` for the new-conversation composer. */ + sessionId: string | null; + text: string; + /** Distinguishes two identical requests so the second is not ignored. */ + nonce: number; +} + +/** + * The one way a feature hands text to the composer without reaching into + * the component (there are several `app-chat-input` placements, so a + * view-child chain would have to thread through all of them). + * + * The composer consumes a draft matching its session: it sets the textarea, + * focuses it, and clears the request. Nothing is sent — the user edits and + * submits as usual. First consumer: the feedback retry-with-correction + * (docs/specs/response-feedback.md §7). + */ +@Injectable({ providedIn: 'root' }) +export class ComposerDraftService { + private nonce = 0; + readonly pending = signal(null); + + request(sessionId: string | null, text: string): void { + this.pending.set({ sessionId, text, nonce: ++this.nonce }); + } + + /** Take the pending draft for `sessionId`, if any, clearing it. */ + consume(sessionId: string | null): ComposerDraft | null { + const draft = this.pending(); + if (!draft || draft.sessionId !== sessionId) return null; + this.pending.set(null); + return draft; + } +} diff --git a/frontend/ai.client/src/app/session/services/session/message-feedback.service.spec.ts b/frontend/ai.client/src/app/session/services/session/message-feedback.service.spec.ts index ad5c0aee..d4e8496a 100644 --- a/frontend/ai.client/src/app/session/services/session/message-feedback.service.spec.ts +++ b/frontend/ai.client/src/app/session/services/session/message-feedback.service.spec.ts @@ -4,11 +4,13 @@ import { provideHttpClient } from '@angular/common/http'; import { signal } from '@angular/core'; import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import { ConfigService } from '../../../services/config.service'; +import { ComposerDraftService } from './composer-draft.service'; import { Message } from '../models/message.model'; import { MessageFeedbackService, parseMessageRef, readPersistedFeedback, + retryTemplate, } from './message-feedback.service'; function message(id: string, metadata: Record | null = null): Message { @@ -111,4 +113,48 @@ describe('MessageFeedbackService', () => { await service.setFeedback(message('placeholder'), 1); http.expectNone(() => true); }); + + describe('retry with correction', () => { + it('requestRetry drafts the reason\'s template into that session\'s composer', () => { + const drafts = TestBed.inject(ComposerDraftService); + const m = message('msg-s-3', { feedback: { value: -1, reason: 'instructions', updatedAt: 't' } }); + service.requestRetry(m); + const draft = drafts.pending(); + expect(draft?.sessionId).toBe('s'); + expect(draft?.text).toBe(retryTemplate('instructions')); + expect(draft?.text).toContain('ignored my instructions'); + http.expectNone(() => true); + }); + + it('links the sent correction to the thumb as retryMessageId, an index only', async () => { + const m = message('msg-s-3', { feedback: { value: -1, reason: 'wrong', updatedAt: 't' } }); + service.requestRetry(m); + service.consumePendingRetry('s', message('msg-s-4')); + const req = http.expectOne('http://api.test/sessions/s/messages/3/feedback'); + expect(req.request.method).toBe('PUT'); + expect(req.request.body).toEqual({ value: -1, reason: 'wrong', retryMessageId: 4 }); + req.flush({ value: -1, reason: 'wrong', retryMessageId: 4, updatedAt: 'u' }); + await Promise.resolve(); + expect(service.feedbackFor(m)?.retryMessageId).toBe(4); + // Consumed: a second send does not link again. + service.consumePendingRetry('s', message('msg-s-6')); + http.expectNone(() => true); + }); + + it('a message on another session leaves the retry pending; a withdrawn thumb drops it', () => { + const m = message('msg-s-3', { feedback: { value: -1, updatedAt: 't' } }); + service.requestRetry(m); + service.consumePendingRetry('other', message('msg-other-1')); + http.expectNone(() => true); + service.consumePendingRetry('s', message('msg-s-4')); + http.expectOne('http://api.test/sessions/s/messages/3/feedback').flush({ value: -1, retryMessageId: 4, updatedAt: 'u' }); + }); + + it('every reason has a template and none is empty', () => { + for (const reason of ['wrong', 'instructions', 'length', 'tool_failed', 'outdated', 'other'] as const) { + expect(retryTemplate(reason).length).toBeGreaterThan(10); + } + expect(retryTemplate(undefined)).toBe(retryTemplate('other')); + }); + }); }); diff --git a/frontend/ai.client/src/app/session/services/session/message-feedback.service.ts b/frontend/ai.client/src/app/session/services/session/message-feedback.service.ts index 0fb4cae9..ea6d0708 100644 --- a/frontend/ai.client/src/app/session/services/session/message-feedback.service.ts +++ b/frontend/ai.client/src/app/session/services/session/message-feedback.service.ts @@ -2,6 +2,7 @@ import { computed, inject, Injectable, signal } from '@angular/core'; import { HttpClient } from '@angular/common/http'; import { firstValueFrom } from 'rxjs'; import { ConfigService } from '../../../services/config.service'; +import { ComposerDraftService } from './composer-draft.service'; import { Message, MessageFeedback, FeedbackReason } from '../models/message.model'; /** @@ -22,6 +23,14 @@ import { Message, MessageFeedback, FeedbackReason } from '../models/message.mode export class MessageFeedbackService { private readonly http = inject(HttpClient); private readonly config = inject(ConfigService); + private readonly composerDraft = inject(ComposerDraftService); + + /** + * A down-thumb the user chose to retry, waiting for the correction to be + * sent. Consumed by `consumePendingRetry` when the next user message of + * that session is added, which is when the row gets its `retryMessageId`. + */ + private readonly pendingRetry = signal<{ sessionId: string; message: Message } | null>(null); /** Local overrides keyed by message id; `null` = withdrawn. */ private readonly overrides = signal>(new Map()); @@ -46,20 +55,27 @@ export class MessageFeedbackService { } /** Thumb a message, replacing any earlier thumb (one per user+message). */ - async setFeedback(message: Message, value: 1 | -1, reason?: FeedbackReason): Promise { + async setFeedback( + message: Message, + value: 1 | -1, + reason?: FeedbackReason, + retryMessageId?: number, + ): Promise { const target = parseMessageRef(message.id); if (!target) return; const previous = this.feedbackFor(message); const optimistic: MessageFeedback = { value, reason: reason ?? (previous?.value === value ? previous?.reason : undefined), + retryMessageId: retryMessageId ?? previous?.retryMessageId, updatedAt: new Date().toISOString(), }; this.setOverride(message.id, optimistic); this.markPending(message.id, true); try { - const body: { value: 1 | -1; reason?: FeedbackReason } = { value }; + const body: { value: 1 | -1; reason?: FeedbackReason; retryMessageId?: number } = { value }; if (optimistic.reason) body.reason = optimistic.reason; + if (retryMessageId !== undefined) body.retryMessageId = retryMessageId; const stored = await firstValueFrom( this.http.put(this.url(target.sessionId, target.index), body), ); @@ -71,6 +87,37 @@ export class MessageFeedbackService { } } + /** + * The consequence (spec §7 "the retry loop"): put a correction template for + * the thumb's reason into the composer for the user to edit and send. The + * template is conversation content and goes where the message goes; + * nothing of it is stored on the feedback row. + */ + requestRetry(message: Message): void { + const target = parseMessageRef(message.id); + if (!target) return; + const reason = this.feedbackFor(message)?.reason; + this.pendingRetry.set({ sessionId: target.sessionId, message }); + this.composerDraft.request(target.sessionId, retryTemplate(reason)); + } + + /** + * Called by the send path once the next user message of a session exists. + * Links it to the pending down-thumb as `retryMessageId` — an index, never + * the correction's text — and clears the pending retry. A message on + * another session, or with no server-shaped id, leaves the retry pending. + */ + consumePendingRetry(sessionId: string, userMessage: Message | null | undefined): void { + const pending = this.pendingRetry(); + if (!pending || pending.sessionId !== sessionId || !userMessage) return; + const sent = parseMessageRef(userMessage.id); + if (!sent) return; + this.pendingRetry.set(null); + const current = this.feedbackFor(pending.message); + if (!current || current.value !== -1) return; + void this.setFeedback(pending.message, -1, current.reason, sent.index); + } + /** Withdraw the thumb on a message. */ async clearFeedback(message: Message): Promise { const target = parseMessageRef(message.id); @@ -141,13 +188,37 @@ export function readPersistedFeedback(message: Message): MessageFeedback | null const value = (raw as { value?: unknown }).value; if (value !== 1 && value !== -1) return null; const reason = (raw as { reason?: unknown }).reason; + const retry = (raw as { retryMessageId?: unknown }).retryMessageId; return { value, reason: isFeedbackReason(reason) ? reason : undefined, + retryMessageId: typeof retry === 'number' && Number.isInteger(retry) && retry >= 0 ? retry : undefined, updatedAt: String((raw as { updatedAt?: unknown }).updatedAt ?? ''), }; } +/** + * Correction templates per reason code (spec §6 buckets). Each ends where the + * user's own words belong; they are prefilled, never auto-sent. + */ +export function retryTemplate(reason: FeedbackReason | undefined): string { + switch (reason) { + case 'wrong': + return 'That answer was wrong or made up. Redo it, checking each claim against the sources you actually have, and say what you are unsure about. Specifically: '; + case 'instructions': + return 'That answer ignored my instructions. Redo it following exactly what I asked for. In particular: '; + case 'length': + return 'That answer was the wrong length. Redo it '; + case 'tool_failed': + return 'A tool or search failed in that answer. Try again, and if it fails again tell me instead of guessing. '; + case 'outdated': + return 'That answer was out of date. Redo it using the most current information you have, and say how current it is. '; + case 'other': + default: + return 'That answer did not work for me. Redo it, and this time '; + } +} + export const FEEDBACK_REASONS: readonly FeedbackReason[] = ['wrong', 'instructions', 'length', 'tool_failed', 'outdated', 'other']; export function isFeedbackReason(value: unknown): value is FeedbackReason {