feat(reviews): surface hold reason on the review queue#389
Open
jiashuoz wants to merge 2 commits into
Open
Conversation
Every message in the pending-review tab now shows WHY it was held — a reader-friendly line derived from the coded screening verdict, e.g. "Content flagged by screening scan (0.87)", "Sender blocked by inbound policy", "Recipient blocked by outbound policy". The queue previously exposed only `flag_reason`, which is populated for inbound sender-gate holds ONLY — blank for content-scan holds and every outbound hold, exactly the rows a reviewer most needs context on. The authoritative `messages.review_reason` (+ `scan_score`) is populated for every hold path (both directions, gate and scan) and already lived on the row (migration 037/040) but was never selected by the review queries. Backend (additive, no migration): - ListReviews / GetReviewWithContent now select review_reason + scan_score onto ReviewListItem / identity.Message. - ReviewView (list) exposes review_reason + scan_score. - MessageView gains the fields but they are populated ONLY in the review detail handler, so they never ride along on the agent /messages surface. - Regenerated api/openapi.yaml + TS/Python SDK bases (review_reason + scan_score only); spec-drift gate passes. Frontend: - Carry both fields through ReviewWire/MessageViewWire → types → PendingRow. - New reviewReason.ts code→label map (open-set safe: unknown codes are humanized, not dropped); rendered as a subline in the collapsed header. Tests: - DB-backed TestListReviews_SurfacesHoldReason asserts reason + score come back on list AND detail, for a scan hold and a gate hold. - reviewReason.test.ts covers the label map, score formatting, gate/no-score, empty→null, and unknown-code humanization. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HUhZns9F31y8J4EZpmr9NQ
From the review + adversarial pass on #389: - a11y: the "why held" subline used var(--warn), which fails WCAG AA at 11px in light mode (~3.1:1). Switch to var(--warn-strong) to match the file's own convention (the "no longer pending" banner) and pass AA; drop the dead #b45309 literal fallback. - robustness: reviewReasonLabel() looked up the label map with `?? `, so a coded reason colliding with an Object.prototype key ("constructor", "toString", …) resolved to an inherited function/object and would crash the React render. Guard with `typeof mapped === "string"`. Not reachable today (review_reason is server-assigned from a fixed set) but the field is documented open-set — stay defensive. - cleanliness: hoist the label to a single const in PendingRow (was computed three times per row: guard, title, body). - tests: pin the central non-leak guarantee at the httpapi layer (TestMessageViewFromIdentity_NoReviewReasonLeak — the shared constructor must never populate review_reason/scan_score onto the agent /messages surface); add frontend cases for prototype-key codes, a legit 0 score, gate+score, and non-finite scores. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HUhZns9F31y8J4EZpmr9NQ
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Every message in the pending-review tab now shows why it was held — a reader-friendly line derived from the coded screening verdict:
⚑ Content flagged by screening scan (0.87)⚑ Sender blocked by inbound policy⚑ Recipient blocked by outbound policyWhy
The queue previously surfaced only
flag_reason, which is populated for inbound sender-gate holds only — blank for content-scan holds and every outbound hold, i.e. exactly the rows a reviewer most needs context on. The authoritativemessages.review_reason(+scan_score) is populated for every hold path (both directions, gate and scan) and already lived on the row (migration 037/040), but was never selected by the review queries or exposed by the API.Changes
Backend (additive — no migration):
ListReviews/GetReviewWithContentnow selectreview_reason+scan_score.ReviewView(list) exposes both fields.MessageViewgains the fields but they're populated only in the review-detail handler (handleGetReview), so they never ride along on the agent/messagessurface (held rows are never returned there anyway).api/openapi.yaml+ TS/Python SDK bases (review_reason+scan_scoreonly); spec-drift gate passes.Frontend:
ReviewWire/MessageViewWire→ types →PendingRow.reviewReason.tscode→label map — open-set safe: unknown codes are humanized, not dropped. Rendered as a subline in the always-visible collapsed header.Tests:
TestListReviews_SurfacesHoldReason: asserts reason + score come back on list and detail, for a scan hold and a gate hold.reviewReason.test.ts: label map, score formatting, gate/no-score, empty→null, unknown-code humanization.Not included (follow-up)
The LLM detector's actual one-line rationale + threat categories still live only in
protection_events(unexposed). Surfacing that ("prompt-injection: instructs the agent to exfiltrate…") is a larger change — a new review sub-resource — deliberately deferred.Verification
go build ./...+go vetclean;Reviewtest suite + new test pass (Postgres :5433).reviewReason.test.ts5/5;tsc --noEmitexit 0.TestSpecGoldenNoDriftpasses.🤖 Generated with Claude Code
https://claude.ai/code/session_01HUhZns9F31y8J4EZpmr9NQ