feat(reviews): surface detector rationale + threat categories on hold detail#390
Open
jiashuoz wants to merge 2 commits into
Open
feat(reviews): surface detector rationale + threat categories on hold detail#390jiashuoz wants to merge 2 commits into
jiashuoz wants to merge 2 commits into
Conversation
… detail Stacked on #389. Where the coarse review_reason answers "which producer held this" ("Content flagged by screening scan"), this adds the specific "why" to the expanded review row — e.g. "Prompt injection — instructs the agent to wire funds (0.92)". The data already existed: every scan hold writes a protection_events row with the aggregate threat `categories` and, in the per-detector `raw` breakdown, the detector's one-line rationale (provider.native_verdict). It was just unexposed — ListProtectionEventsByMessage existed but was wired to no HTTP handler. No new detection, no migration. Backend: - New httpapi dep ListProtectionEventsByMessage, wired from the store. - review_protection.go: ProtectionFindingView + ThreatCategoryView, and a builder that projects the audit rows — categories straight off the row, and a CURATED summary pulled from the per-detector raw (rationaleFromRaw prefers the flagged detector). The raw provider payload is never exposed to the client. - handleGetReview attaches `protection` best-effort (a failed events fetch just omits it → the coded review_reason remains the fallback). Populated ONLY on the review-detail path — MessageView carries the field but the agent /messages constructor never sets it (same review-only pattern as review_reason). - Regenerated api/openapi.yaml + TS/Python SDK bases; spec-drift gate passes. Frontend: - Carry `protection` through the wire type + projectPending into the detail. - reviewReason.ts: categoryLabel (open-set, prototype-safe like reviewReasonLabel) + protectionHeadline (highest-confidence scan category + rationale + score). - PendingRow: a "why held" callout at the top of the expanded panel; gate-only holds (no scan detail) fall back to the coarse collapsed-row label. Tests: - Builder units: category projection, flagged-detector rationale preference, gate-has-no-scan-detail, malformed JSON is safe (no panic/error). - Frontend: categoryLabel mapping/humanize/prototype-key safety, and protectionHeadline top-category selection + gate/empty → null. - JSONB persistence round-trip is already covered by identity's TestProtectionEvents_CreateAndList; the httpapi builder can't take a testutil DB dep (import cycle via apiserver), so coverage is split by design. 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 #390 (all three agents cleared the safety claims — no-raw-leak, review-only, panic-safety — these are the polish findings): - ux: the expanded callout showed the finding-level scan score, which can contradict the category label it sits next to (and differ from the collapsed row's number). protectionHeadline now shows the top CATEGORY's own score, falling back to the finding score when the category has none. - docs: ProtectionFindingView's comment claimed a gate finding "names the address" — it has no address field (deliberately; the reviewer already sees sender/recipient). Corrected the comment to match. - observability: the best-effort ListProtectionEventsByMessage error was swallowed silently; log it so a persistently-empty `protection` is greppable. - tests: handler-level TestReviews_DetailIncludesProtection (proves the dep wiring + attach end-to-end); extend the agent-surface non-leak test to assert `protection` is nil off the shared constructor; frontend cases for category-score-over-finding-score, no-category-score fallback, SWR-array non-mutation, and a prototype-key category name. 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
Where #389's coarse
review_reasonsays which producer held the message ("Content flagged by screening scan"), this adds the specific why to the expanded review row:Why it's cheap
The data already exists. Every scan hold writes a
protection_eventsrow carrying the aggregate threatcategoriesand — in the per-detectorrawbreakdown — the detector's one-line rationale (provider.native_verdict).ListProtectionEventsByMessagealready existed in the store; it was just wired to no HTTP handler. No new detection, no migration.Changes
Backend
ListProtectionEventsByMessage, wired from the store.review_protection.go:ProtectionFindingView+ThreatCategoryViewand a builder — categories straight off the row, and a curatedsummarypulled from the per-detectorraw(rationaleFromRawprefers the flagged detector). The raw provider payload is never exposed to the client.handleGetReviewattachesprotectionbest-effort (a failed fetch just omits it → codedreview_reasonstays the fallback). Populated only on the review-detail path —MessageViewcarries the field but the agent/messagesconstructor never sets it (same review-only pattern asreview_reason).api/openapi.yaml+ TS/Python SDK bases; spec-drift gate passes.Frontend
protectionthrough the wire type →projectPending→ detail.reviewReason.ts:categoryLabel(open-set, prototype-key-safe) +protectionHeadline(highest-confidence scan category + rationale + score).PendingRow: a "why held" callout at the top of the expanded panel; gate-only holds (no scan detail) fall back to the coarse collapsed-row label.Tests
categoryLabelmapping/humanize/prototype-key safety;protectionHeadlinetop-category selection + gate/empty → null. (14 assertions total inreviewReason.test.ts.)TestProtectionEvents_CreateAndList. The httpapi builder can't take atestutilDB dep (import cycle viaapiserver), so coverage is split by design.Security / privacy
protectionfields marked beta so the shape can evolve.Verification
go build ./...+go vetclean; builder units +TestSpecGoldenNoDriftpass.tsc --noEmitexit 0;reviewReason.test.ts14/14.🤖 Generated with Claude Code
https://claude.ai/code/session_01HUhZns9F31y8J4EZpmr9NQ