[386] feat: show manual moderator actions in the Recent Decisions log - #1030
[386] feat: show manual moderator actions in the Recent Decisions log#1030calebmcquaid wants to merge 1 commit into
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Thanks @calebmcquaid! I have a few quick questions on the UI: 
^ is this search bar supposed to look like this? 
^ I'm a little confused on what these hyperlinks are, are they Jobs? |
Confirmed that search bar bug happens on Those are Items and they link to Investigations |
|
|
||
| Bulk actions appear in the **Recent Decisions** log alongside queue-based decisions, so there is a full audit trail of what was actioned, by whom, and under which policies. | ||
|
|
||
| Use the **Show** control there to switch between all activity, queue decisions only, or manual actions only. The **Origin** column marks where each row came from — `Review Job` for a queue decision, `Manual Action` for anything taken from Bulk Actioning or Investigation. |
There was a problem hiding this comment.
cc @cassidyjames in case you want to take a look at changes to docs
| // That 1000 limit is client-side only and `bulkExecuteActions` caps nothing, | ||
| // so a programmatic caller can still exceed it; the truncation notice below | ||
| // stays for that case. | ||
| const PAGE_SIZE = 1000; |
There was a problem hiding this comment.
do we need this client-side limit? seems arbitrary?
| // Bulk Actioning caps a run at 1000 ids, so this covers everything the UI can | ||
| // produce and truncation becomes the exception rather than routine. At 100 a | ||
| // 250-item run hid 22 of its 36 failures, and which items you saw was | ||
| // arbitrary (`ORDER BY item_id`) — failures are the reason this panel exists. |
There was a problem hiding this comment.
At 100 a 250-item run hid 22 of its 36 failures, and which items you saw was
this does not seem like it belongs in a comment we commit to the repo! not sure what it's referencing?
| * Renders in the page's detail side panel — the same slot a selected | ||
| * decision uses (`ManualReviewRecentDecisionSummary` + | ||
| * `ManualReviewJobReview`) — not inside the feed table, so a wide run's | ||
| * item list has room to breathe instead of wrapping inside a table cell. |
There was a problem hiding this comment.
i don't think this level of detail is needed in the comments, IMO.
| // arbitrary slice — failures can fall outside it. Say that | ||
| // rather than letting the reader assume they see every one. | ||
| <span className="text-coop-alert-red"> | ||
| {` — showing first ${data.manualActionItems.items.length}; some failures may not be listed`} |
There was a problem hiding this comment.
some failures may not be listed
I think that's a pretty bad UX. If there's a failure, let's error loudly so the user knows whether something went wrong or not.
| } | ||
| } | ||
|
|
||
| # Skips CSV export. This is untouched by the merged-feed cursor paging |
There was a problem hiding this comment.
i think this comment is redundant? a reader would never be wondering "is this touched by the merged-feed cursor stuff above?"
|
|
||
| /** | ||
| * The side panel shows exactly one thing at a time — a decision or a manual | ||
| * action, never both. Modeling the selection as a single discriminated union |
There was a problem hiding this comment.
really there's no need to mention the alternative here.
in general these comments are overly verbose and seem to be written for this PR, not for the long-term codebase.
|
|
||
| /** | ||
| * The decision-detail side panel and the `getDecidedJob` lookup both predate | ||
| * the merged feed and still expect a `ManualReviewDecision`-shaped object. |
There was a problem hiding this comment.
again, no need for this "predate the merged feed" stuff.
| // Runaway guard for both CSV exports: stop paging after this many requests | ||
| // even if the server keeps returning more (e.g. `nextCursor` never goes | ||
| // null, or the offset loop never runs dry). | ||
| const CSV_MAX_PAGES = 100; |
There was a problem hiding this comment.
these silent length limits are a bit of an antipattern. ideally we wouldn't have them at all.
the problem here is that we generate the CSV on the frontend instead of properly, on the backend. i guess that's a separate issue, though -- would you mind creating a follow-up ticket for that?
i'm a bit unsure how best to handle this until we do that, though!
|
|
||
| /** | ||
| * Mirrors `MERGED_VIEW_WINDOW_MS` in `ModerationActivityFeed` — the server | ||
| * bounds how far back a page scans ClickHouse for manual actions. Stated in |
There was a problem hiding this comment.
as per other comments, the server should not bound this.
3 participants
Context & Requests for Reviewers
Last of four for #386.
Recent Decisions now shows queue decisions and manual actions in one list, with an Origin column telling them apart.
A control switches between All, Decisions and Actions. A bulk run is one row with item and failure counts. The side panel lists every item and failures marked. Filtering by queue or decision type flips Show to Decisions and says why.
Tests
224 tests across 34 files.