Repository navigation
Refine reviewer rules based on automated tracker feedback #90591
Description
Refinements to two Claude reviewer rules based on accept/reject feedback gathered via the automated tracker. Each refinement narrows the rule's firing condition so reviewers stop dismissing technically-wrong suggestions, which was eroding trust in unrelated rules.
The originally scoped PERF-9 refinement was dropped during implementation - the cases the new carve-out would have permitted (intermediate state that is render-driving) are exactly the cases PERF-14 already wants migrated to useSyncExternalStore. Adding the carve-out would have contradicted PERF-14, and legitimate chains that fall outside PERF-14's contract (timer- or Promise-based) are already covered by the existing "external events that can't be combined" bullet on PERF-9.
CLEAN-REACT-PATTERNS-4
Background
Claude reviewer enforces CLEAN-REACT-PATTERNS-4, flagging any internal helper function in a component that returns JSX. Stated rationale: React Compiler cannot independently memoize JSX created inside a parent's closure. Internal render helpers are common in this codebase, especially as switch/conditional dispatchers to already-extracted child components. Across the sampled PR threads (88691, 85949, 85426) all three suggestions were rejected by reviewers citing RC behavior or code-move context.
Problem
When the rule fires on thin delegation wrappers that React Compiler already memoizes correctly, developers reject the suggestion as technically wrong, which erodes trust in every other rule the reviewer emits.
Solution
Scope CLEAN-REACT-PATTERNS-4 to fire only when the helper (a) contains hooks, side effects, or non-trivial business logic, (b) builds deeply nested JSX inline, or (c) closes over mutable parent state. Add explicit carve-outs: (1) switch/conditional helpers that delegate to already-extracted child components, (2) .map() over already-extracted components. Correct the rule's docs claim about RC memoization to reflect that RC handles thin wrappers without issue. Skip the "code merely moved" carve-out - too fuzzy and indirectly covered by (1) and (2).
CLEAN-REACT-PATTERNS-1
Background
Claude reviewer enforces CLEAN-REACT-PATTERNS-1, flagging components on prop count (monolithic interface), mixed responsibilities, and config-array rendering (Case 4). Sampled feedback: 3 violations, 1 thumbs-down, 2 substantive developer replies pushing back on legitimate React patterns (88344, 86663).
Problem
When the rule counts props that exist solely to coordinate shared state or callbacks across sibling children, components with legitimately-lifted state get flagged as monolithic, which forces developers into unnatural decompositions or rule-ignoring habits.
Solution
Exempt props from the monolithic-interface prop count when they exist to coordinate shared state or callbacks across sibling children whose state must live in the common parent - the coordination role must be structurally detectable (state setter passed to multiple children, or callback consumed by multiple children) to prevent argument-shaped escape hatches. Defer the Case 4 (config-array rendering) threshold change until evidence reaches ~10+ thumbs-down; n=1 is insufficient signal and "~6-8 items" is arbitrary and gameable.
Activity
kacper-mikolajczak commentedon May 18, 2026
CC @Julesssss @rlinoz
Julesssss commentedon May 18, 2026
Agree, not sure how exactly to tighten the prompt though.
BTW once we figure out a good workflow for analysing and actioning these improvement me and Rodrigo are thinking about making this an automated weekly chore assigned to the internal engineering team. So any promp you used to generate this issue analysis, or other tips would be very helpful.
kacper-mikolajczak commentedon May 19, 2026
Thanks @Julesssss! Quick note - the rule changes already shipped in #90590, so for these two rules "tightening" turned into concrete edits.
I do pick up those improvements proposals from the analyzer sometimes in order to check how it's performing, but I definitely agree automating steps further and thus moving human-in-the-loop checkpoint into later stages in the process would be a good way forwards that reduces maintenance burden.
For the prompt, those issues were gathered from the analyzer run and passed through quick verification + Exfy's P/S hourglass prompt. Later on, there was one refinement after Adam's review. How have you guys been working so far with analyzer improvements?
Julesssss commentedon May 19, 2026
We simply looked through the 3 top level issues listed in here and manually looked into the highly reported rules.
kacper-mikolajczak commentedon May 19, 2026
Sounds like a good starting point. Are you satisfied with this flow for now and test it more or you'd like start thinking about next iteration?
[-]Refine reviewer rules based on automated tracker feedback[/-][+][Due for payment 2026-05-26] Refine reviewer rules based on automated tracker feedback[/+]melvin-bot commentedon May 19, 2026
Reviewinglabel has been removed, please complete the "BugZero Checklist".melvin-bot commentedon May 19, 2026
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.3.77-3 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:
If no regressions arise, payment will be issued on 2026-05-26. 🎊
For reference, here are some details about the assignees on this issue:
[-][Due for payment 2026-05-26] Refine reviewer rules based on automated tracker feedback[/-][+]Refine reviewer rules based on automated tracker feedback[/+]Julesssss commentedon May 19, 2026
Open to ideas for further iterations!
melvin-bot commentedon May 25, 2026
@rlinoz, @Julesssss, @kacper-mikolajczak Huh... This is 4 days overdue. Who can take care of this?
kacper-mikolajczak commentedon May 26, 2026
I think we can close this and continue the exploration for further automation separately.