rayhpeng 551865abcf fix(feedback): address review on dialog state, errors, and test seams
Four findings from @willem-bd on #4401:

- FeedbackDialog stays mounted across messages, so selected tags and the
  comment survived an ESC/click-outside dismiss and pre-filled the next
  thumbs-down. Reset on every close path via a wrapped onOpenChange.
- Neither the dialog's handleSubmit nor handleDialogSubmit caught a failed
  enrichment PUT, so a rejection went unhandled and the user got no signal.
  Catch in the dialog (where the rejection lands), toast, and keep the input
  for a retry.
- rate_run awaited the RunLookup port before Feedback.create validated the
  rating, contradicting its own "before any I/O happens" docstring: an
  invalid rating on an unknown run surfaced as RunNotFoundError. Validate
  first, restoring the legacy router's 400-before-404 ordering. The service
  test now uses an unknown run id so it actually pins that order.
- InMemoryFeedbackRepository moved out of test_feedback.py into
  tests/feedback_fakes.py (with FakeRunLookup) so two test modules share it
  without one importing the other; conftest states the tests-dir sys.path
  dependency explicitly, which also makes it work under
  --import-mode=importlib.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-27 10:58:45 +08:00

75 lines
3.3 KiB
Python

from deerflow.domain.feedback.model import Feedback, RunNotFoundError
from deerflow.domain.feedback.ports import FeedbackRepository, RunLookup
class FeedbackService:
"""Input port of the feedback context (application service).
Orchestrates use cases only: fetch/verify -> apply domain rules ->
persist through output ports. Holds no business rules itself (those
live on the Feedback aggregate) and knows nothing about HTTP or
storage. user_id is always passed in explicitly -- resolving the
current user is the primary adapter's job.
"""
def __init__(self, repository: FeedbackRepository, runs: RunLookup):
self._repository = repository
self._runs = runs
async def _require_run(self, thread_id: str, run_id: str) -> None:
"""Reject runs that do not exist or belong to another thread."""
if await self._runs.thread_of(run_id) != thread_id:
raise RunNotFoundError("Run does not belong to the specified thread")
async def rate_run(
self,
thread_id: str,
run_id: str,
*,
rating: int,
comment: str | None,
user_id: str | None,
tags: tuple[str, ...] | list[str] = (),
) -> Feedback:
"""Set the user's current rating for a run (idempotent).
Backs the PUT endpoint: "my current verdict on this run is X".
Builds the aggregate first (which validates rating and tags), then
verifies run ownership, then stores it with upsert-by-identity
semantics -- repeated calls replace the previous rating.
Raises:
InvalidRatingError: rating is not +1 or -1. Raised by the
aggregate factory before any port call, so a malformed
rating is reported as such even for an unknown run.
InvalidTagError: a tag is not a known reason slug (same
pre-I/O guarantee).
RunNotFoundError: the run does not exist or does not belong
to the given thread (cross-thread ids are rejected).
"""
feedback = Feedback.create(
run_id=run_id,
thread_id=thread_id,
rating=rating,
user_id=user_id,
comment=comment,
tags=tags,
)
await self._require_run(thread_id, run_id)
return await self._repository.save(feedback)
async def retract_run_rating(self, thread_id: str, run_id: str, *, user_id: str | None) -> bool:
"""Withdraw the user's rating for a run (clicking the active button
again). Returns False when there was nothing to retract."""
return await self._repository.remove_for_run(thread_id, run_id, user_id=user_id)
async def latest_per_run_in_thread(self, thread_id: str, *, user_id: str | None) -> dict[str, Feedback]:
"""Current feedback per run across a thread -- powers the message-list
thumb badges (full-list path)."""
return await self._repository.latest_per_run_in_thread(thread_id, user_id=user_id)
async def latest_for_runs(self, thread_id: str, run_ids: set[str], *, user_id: str | None) -> dict[str, Feedback]:
"""Current feedback for the selected runs only -- powers the paged
message-list badges."""
return await self._repository.latest_for_runs(thread_id, run_ids, user_id=user_id)