Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds KeyFactor.created_at serialization and new i18n keys; introduces deterministic sorting and many front-end changes: optimistic voting hooks/panels, PanelContainer/VotePanel/MorePanel, VerticalImpactBar, KeyFactorStrengthItem composition, API-backed optimistic voting flow, prop/type adjustments, and removal of several legacy components. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as KeyFactor UI
participant Panels as KeyFactorVotePanels
participant Optim as useOptimisticVote
participant API as Backend API
participant Feed as Comments Feed
User->>UI: click vote / open panel
UI->>Panels: open specific panel (impact/downvote/more)
User->>UI: select option & confirm
UI->>Optim: setOptimistic(vote)
Optim-->>UI: update local counts (optimistic)
UI->>API: POST voteKeyFactor
API-->>Feed: persist vote
alt success
Feed->>UI: aggregated update
Optim->>UI: clearOptimistic -> render server counts
else failure
API-->>Optim: error
Optim->>UI: clearOptimistic -> revert UI
end
sequenceDiagram
participant User
participant Anchor as Card Anchor
participant Panel as PanelContainer
participant Parent as Card Parent
User->>Anchor: click ellipsis / vote button
Anchor->>Panel: provide anchorRef for positioning
Panel->>Parent: onVotePanelToggle(true)
Parent->>Parent: ensure exclusivity (close others)
User->>Panel: click outside / close
Panel->>Parent: onVotePanelToggle(false)
Parent->>Panel: closePanel()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Contributor
Cleanup: Preview Environment RemovedThe preview environment for this PR has been destroyed.
Cleanup triggered by PR close at 2026-03-30T12:14:02Z |
ncarazon
force-pushed
the
feat/key-factors-new-iteration
branch
from
597275c to
14580b4
Compare
ncarazon
force-pushed
the
feat/key-factors-new-iteration
branch
from
14580b4 to
9858b7a
Compare
ncarazon
force-pushed
the
feat/key-factors-new-iteration
branch
from
d6d2a0c to
629c70a
Compare
ncarazon
force-pushed
the
feat/key-factors-new-iteration
branch
from
629c70a to
4aa477e
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
front_end/src/app/(main)/questions/[id]/components/key_factors/key_factors_question_consumer_section.tsx (1)
38-42:⚠️ Potential issue | 🟡 MinorAvoid logging
KeyFactorClickfor the “View all” action.Line 41 fires
KeyFactorClickwithevent_label: "fromTopList"insideopenKeyFactorsElement, and Line 56 calls that helper from the “View all” button. This misattributes “view all” interactions as top-list clicks.Also applies to: 56-59
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/questions/[id]/components/key_factors/key_factors_question_consumer_section.tsx around lines 38 - 42, The helper openKeyFactorsElement currently always calls sendAnalyticsEvent("KeyFactorClick", { event_label: "fromTopList" }), which misattributes the "View all" button; update openKeyFactorsElement (or create a small wrapper) so it accepts an optional analyticsLabel parameter (or a skipAnalytics flag) and only sends KeyFactorClick when the caller intends it; change the "View all" button call site to either pass a different label (e.g., "viewAll") or skip sending analytics, leaving other callers unchanged; ensure the function signature and all callers (including the View all invocation) are updated to avoid hardcoding event_label: "fromTopList".front_end/src/app/(main)/questions/[id]/components/key_factors/item_view/base_rate/key_factor_base_rate.tsx (1)
94-98:⚠️ Potential issue | 🟡 Minor
router.pushmay not work correctly for external URLs.If
baseRate.sourceis an external URL (e.g.,https://example.com),router.push()is not the correct approach—it's designed for internal Next.js routing. External URLs should usewindow.open()or an anchor element.🐛 Proposed fix for external URL handling
onClick={() => { if (!showSourceError && hasSource && baseRate.source) { - router.push(baseRate.source); + window.open(baseRate.source, "_blank", "noopener,noreferrer"); } }}Alternatively, consider replacing the div with a proper anchor element for better accessibility (right-click support, keyboard navigation, screen reader compatibility):
{!showSourceError && hasSource ? ( <a href={baseRate.source} target="_blank" rel="noopener noreferrer" className={cn( "text-left text-xs", baseRate.type === "trend" && "-mt-2", "text-blue-600 hover:underline dark:text-blue-600-dark" )} > (source) </a> ) : showSourceError ? ( <span className="text-left text-xs text-salmon-700 dark:text-salmon-700-dark"> {t("sourceMissing")} </span> ) : null}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/questions/[id]/components/key_factors/item_view/base_rate/key_factor_base_rate.tsx around lines 94 - 98, The click handler that uses router.push(baseRate.source) will fail for external URLs—update the onClick logic inside the component that references router.push, baseRate.source, showSourceError and hasSource to open external links correctly: if baseRate.source is an external URL use window.open(baseRate.source, "_blank", "noreferrer noopener") (or better yet replace the clickable div with an anchor element <a href=... target="_blank" rel="noopener noreferrer"> to get native external-link behavior and accessibility), and keep the existing showSourceError/hasSource conditional rendering unchanged so the source-missing text still displays when appropriate.
🧹 Nitpick comments (7)
front_end/src/app/(main)/questions/[id]/components/key_factors/add_in_comment/key_factors_add_in_comment_llm_suggestions.tsx (1)
680-680: Use a stable fallback timestamp for synthetic key factorsLine 680 recreates
created_aton every render. That can cause timestamp jitter in relative-time UI and unnecessary rerender churn downstream.Proposed fix
const KeyFactorsAddInCommentLLMSuggestions: React.FC<Props> = ({ onBack, postData, setSelectedType, onAllSuggestionsHandled, }) => { const t = useTranslations(); const { user } = useAuth(); + const fallbackCreatedAt = useMemo(() => new Date().toISOString(), []); @@ const fake: KeyFactor = { @@ - created_at: new Date().toISOString(), + created_at: fallbackCreatedAt,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/questions/[id]/components/key_factors/add_in_comment/key_factors_add_in_comment_llm_suggestions.tsx at line 680, The inline creation of created_at with new Date().toISOString() causes a new timestamp every render; compute and store a stable fallback timestamp once (e.g. in a useRef/useState or useMemo when the suggestion is first created/initialized) and replace the direct new Date().toISOString() usage in the synthetic key factor creation with that stable value so relative-time UI and downstream rendering don't jitter; locate the created_at assignment in key_factors_add_in_comment_llm_suggestions.tsx and use the persistedRef or memoizedTimestamp instead of calling new Date() on each render.front_end/src/app/(main)/questions/[id]/components/key_factors/item_view/key_factor_strength_voter.tsx (1)
28-32: TrimStrengthScaleprops to what the component actually uses.
StrengthScalenow only consumescount, butscore/modeare still part of its API surface, which is misleading for callers.♻️ Suggested API cleanup
-export const StrengthScale: FC<{ - score: number; - count: number; - mode?: "forecaster" | "consumer"; -}> = ({ count }) => { +export const StrengthScale: FC<{ count: number }> = ({ count }) => {- <StrengthScale - score={aggregate?.score ?? 0} - count={aggregate?.count ?? 0} - mode={mode} - /> + <StrengthScale count={aggregate?.count ?? 0} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/questions/[id]/components/key_factors/item_view/key_factor_strength_voter.tsx around lines 28 - 32, StrengthScale declares unused props (score and mode) which is misleading; update the StrengthScale component signature and prop type to only accept { count: number } (remove score and mode from the FC generic and the destructured params) and then update any callers to stop passing score/mode or adjust them to only pass count so the API matches implementation; modify the StrengthScale declaration and its usages to refer only to count to keep the component surface accurate.front_end/src/app/(main)/components/comments_feed_provider.tsx (1)
128-133: Consider adding the same tie-breaker tosetAndSortCombinedKeyFactors.The initial sort (lines 122-124) uses
b.id - a.idas a tie-breaker, butsetAndSortCombinedKeyFactorsdoes not. This could lead to inconsistent ordering between initial render and subsequent updates.Suggested fix for consistent sorting
const setAndSortCombinedKeyFactors = (keyFactors: KeyFactor[]) => { const sortedKeyFactors = [...keyFactors].sort( - (a, b) => (b.vote?.score || 0) - (a.vote?.score || 0) + (a, b) => (b.vote?.score || 0) - (a.vote?.score || 0) || b.id - a.id ); setCombinedKeyFactors(sortedKeyFactors); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/components/comments_feed_provider.tsx around lines 128 - 133, The sorting in setAndSortCombinedKeyFactors currently only orders by vote.score and lacks the id tie-breaker used in the initial sort, causing inconsistent ordering; update the sort comparator in setAndSortCombinedKeyFactors to include the same secondary tie-breaker (e.g., (b.vote?.score || 0) - (a.vote?.score || 0) || b.id - a.id) so ties on score are resolved by id like the initial sort, then call setCombinedKeyFactors with the resulting sorted array.front_end/src/app/(main)/questions/[id]/components/key_factors/item_view/news/key_factor_news_item.tsx (1)
56-117: Consolidate duplicate destination links to reduce tab stops.Lines 56–117 create three separate anchors to the same URL (favicon, title, metadata). This adds redundant focus stops for keyboard users; combining title+metadata into one link would improve accessibility and navigation flow.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/questions/[id]/components/key_factors/item_view/news/key_factor_news_item.tsx around lines 56 - 117, There are three anchors using the same {...linkProps} (favicon link, title link, metadata link) which creates redundant tab stops; consolidate them by keeping only one interactive anchor for the whole card (wrap the favicon, title, and metadata inside a single <a {...linkProps}>), remove the extra <a> elements (keep ImageWithFallback and the title/metadata spans inside the same anchor), ensure non-link elements (e.g., the decorative fallback span) remain semantic, and keep existing helpers/props (linkProps, getProxiedFaviconUrl, ImageWithFallback, title, source, date, formatDate, isCompact, isConsumer) so styles and accessibility attributes are preserved.front_end/src/app/(main)/questions/[id]/components/key_factors/item_view/key_factor_strength_item.tsx (3)
61-63: Consider using constants for vote scores.
downScoreconditionally usesStrengthValues.NO_IMPACTbutupScoreis hardcoded to5. For consistency and maintainability, consider using a named constant for the up score as well (e.g.,StrengthValues.STRONGif available).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/questions/[id]/components/key_factors/item_view/key_factor_strength_item.tsx around lines 61 - 63, The upScore is hardcoded to 5 while downScore uses StrengthValues.NO_IMPACT, so replace the literal with a named constant for consistency: use an existing StrengthValues member (e.g., StrengthValues.STRONG) or add a new StrengthValues entry for the strong/up score and assign upScore from it; update the code around isDirection, upScore and downScore (references: isDirection, upScore, downScore, KeyFactorVoteTypes.DIRECTION, StrengthValues.NO_IMPACT) so both scores come from StrengthValues rather than a magic number.
111-114: Unsafe double type assertion bypasses type safety.The
as unknown as KeyFactorVoteAggregatepattern circumvents TypeScript's type checking entirely. IfvoteKeyFactorreturns a different shape, runtime errors could occur silently.Consider either:
- Typing
voteKeyFactorto returnKeyFactorVoteAggregatedirectly- Adding runtime validation before casting
♻️ Suggested approach with type guard
- if (resp) { - const updated = resp as unknown as KeyFactorVoteAggregate; - setKeyFactorVote(keyFactor.id, updated); - } + if (resp && typeof resp === "object" && "aggregated_data" in resp) { + setKeyFactorVote(keyFactor.id, resp as KeyFactorVoteAggregate); + }Or better, update
voteKeyFactorreturn type to be properly typed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/questions/[id]/components/key_factors/item_view/key_factor_strength_item.tsx around lines 111 - 114, The code unsafely uses a double type assertion ("resp as unknown as KeyFactorVoteAggregate") which bypasses TypeScript checks; update the voteKeyFactor function to return a properly typed Promise<KeyFactorVoteAggregate> OR add a runtime type guard like isValidKeyFactorVoteAggregate(resp) and only call setKeyFactorVote(keyFactor.id, resp) after the guard passes (otherwise handle/log the error); reference the voteKeyFactor call, the resp variable, the KeyFactorVoteAggregate type, setKeyFactorVote, and keyFactor.id when making the change.
115-120: Consider adding user feedback on vote failure.The error is logged and the optimistic state reverts, but the user receives no explicit notification that their vote failed. A toast or brief error message would improve UX.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/questions/[id]/components/key_factors/item_view/key_factor_strength_item.tsx around lines 115 - 120, The catch block in key_factor_strength_item.tsx currently only logs errors and reverts optimistic state; add a user-facing error notification there so failures are visible. Inside the existing catch (e) { ... } for the vote handler, call your app's toast/notification helper (e.g., toast.error or showErrorToast) with a short message like "Failed to submit vote" and include e.message/details, then proceed to clearOptimistic() and setSubmitting(false); also add the necessary import for the toast/notification utility at the top of the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@front_end/messages/cs.json`:
- Around line 2071-2079: The new keys in front_end/messages/cs.json
("voteOnImpact", "why", "wrongDirection", "redundant", "thanksForVoting",
"viewComment", "createdTimeAgoBy", "createdTimeAgo") are still in English or
mixed; update their values to full Czech translations to ensure consistent
locale behavior—replace "voteOnImpact" with a Czech phrase for "Vote on impact",
"why" with a Czech equivalent for "WHY?", "wrongDirection" and "redundant" with
proper Czech words, "thanksForVoting" with a Czech thank-you message, and
translate "viewComment", "createdTimeAgoBy" and "createdTimeAgo" appropriately
(keeping placeholders like {timeAgo} and {author} intact) so the file contains
only Czech strings.
In `@front_end/messages/es.json`:
- Around line 2071-2079: Replace the mixed English strings with Spanish
equivalents for the listed message keys: set "viewComment" to "Ver comentario",
"createdTimeAgoBy" to "Creado {timeAgo} por @{author}", "createdTimeAgo" to
"Creado {timeAgo}", "why" to "¿POR QUÉ?", "wrongDirection" to "Dirección
incorrecta", "redundant" to "Redundante", and "thanksForVoting" to "¡Gracias por
votar!"; keep "direction" ("Dirección") and "voteOnImpact" ("VOTAR SOBRE EL
IMPACTO") as-is if already correct. Ensure you update the JSON values for the
keys viewComment, createdTimeAgoBy, createdTimeAgo, why, wrongDirection,
redundant, and thanksForVoting so the UI shows consistent Spanish copy.
In `@front_end/messages/pt.json`:
- Around line 2069-2077: Several keys in pt.json are still in English and must
be translated; replace the English values for "viewComment", "createdTimeAgoBy",
"createdTimeAgo", "direction", "voteOnImpact", "why", "wrongDirection",
"redundant" and "thanksForVoting" with appropriate Portuguese strings (e.g.,
"Ver comentário", "Criado há {timeAgo} por @{author}", "Criado há {timeAgo}",
"Direção", "VOTAR NO IMPACTO" may remain if intentional but confirm case, "Por
quê?", "Direção incorreta", "Redundante", "Obrigado por votar!"), keeping
placeholders like {timeAgo} and @{author} intact and preserving
punctuation/casing as used elsewhere in the file.
In `@front_end/messages/zh-TW.json`:
- Around line 2068-2076: The zh-TW locale file contains English strings that
must be localized: replace the values for keys "viewComment", "why",
"wrongDirection", "redundant", and "thanksForVoting" with proper Traditional
Chinese translations (keeping placeholders like {timeAgo} and @{author}
untouched); update the corresponding entries in messages/zh-TW.json so the UI is
fully localized for zh-TW (use appropriate Traditional Chinese phrases
consistent with your app's tone).
In `@front_end/messages/zh.json`:
- Around line 2073-2081: Several keys in the Chinese locale file are still in
English; update the values for "viewComment", "createdTimeAgoBy",
"createdTimeAgo", "direction", "voteOnImpact", "why", "wrongDirection",
"redundant", and "thanksForVoting" to their proper Chinese translations so the
zh.json locale is fully localized; locate these keys in
front_end/messages/zh.json and replace the English strings with concise Chinese
equivalents while preserving placeholders like {timeAgo} and @{author}.
In
`@front_end/src/app/`(main)/questions/[id]/components/key_factors/item_view/base_rate/key_factor_base_rate_trend.tsx:
- Line 50: In the KeyFactorBaseRateTrend component remove the forced CSS casing
on the translated text: locate the span rendering t("by") (the element with
className="font-normal lowercase") and delete the "lowercase" class so the
translation is displayed exactly as provided by the i18n resource; if visual
styling requires different presentation for some locales, handle casing in the
translation strings or use a locale-aware utility instead of forcing lowercase
in JSX.
In
`@front_end/src/app/`(main)/questions/[id]/components/key_factors/item_view/key_factor_vote_panels.tsx:
- Around line 32-34: The forEach callback in key_factor_vote_panels.tsx
implicitly returns a value which trips the lint rule; change the arrow callback
on others to use a block body so it does not return anything — e.g. replace the
implicit-return form that calls p.setShowPanel(false) with a block-style
callback that calls p.setShowPanel(false) inside braces (or alternatively use a
for...of loop over others) so the lint rule
lint/suspicious/useIterableCallbackReturn is satisfied; reference the variables
others and the method setShowPanel in the same conditional that checks open.
In
`@front_end/src/app/`(main)/questions/[id]/components/key_factors/item_view/question_link/question_link_key_factor_item.tsx:
- Around line 61-67: The component initializes userVote from
link.votes?.user_vote but never updates it when props change; add a useEffect
that watches link.votes?.user_vote (or the entire link prop) and calls
setUserVote to keep state in sync, and ensure any derived values like
strengthScore (referenced where strengthScore is computed) depend on link and
userVote so they recalculated when link updates; update the effect dependencies
accordingly and ensure initial logic (mapping 1 -> "agree", -1 -> "disagree",
else null) is reused inside the effect.
In
`@front_end/src/app/`(main)/questions/[id]/components/key_factors/key_factors_consumer_carousel.tsx:
- Around line 49-53: The divs in key_factors_consumer_carousel.tsx that render
as interactive wrappers with role="button" and tabIndex={0} must also handle
keyboard activation: add an onKeyDown handler (e.g., alongside the existing
onClick) that listens for Enter (key === "Enter") and Space (key === " " or key
=== "Spacebar") and calls the same activation function used by onClick; for
Space, call event.preventDefault() to avoid page scrolling. Ensure the handler
signature matches the component's event types and reuse the same callback logic
(the existing inline onClick) so both mouse and keyboard activate the
button-equivalent elements.
In
`@front_end/src/app/`(main)/questions/[id]/components/key_factors/key_factors_grid_placeholder.tsx:
- Around line 19-33: The placeholder currently renders an interactive <div> with
onClick which prevents keyboard activation; update the JSX in
key_factors_grid_placeholder.tsx so when onClick is provided you render a
<button type="button"> (instead of the outer <div>) with the same cn(...)
classes, the onClick handler, and keep inner content (FontAwesomeIcon/translated
label) and group-hover/focus-visible styles; when onClick is absent keep the
non-interactive <div>. Also ensure the button includes an accessible label (use
aria-label={t("addKeyFactor")}) and preserve any className prop and visual/focus
styles so keyboard users get the same experience.
In
`@front_end/src/app/`(main)/questions/[id]/components/key_factors/questions_feed_view/key_factors_tile_view.tsx:
- Around line 217-218: The effect currently always runs and calls
ClientPostsApi.getPost()/getQuestion() even for resolved or hidden posts; inside
the useEffect in the KeyFactorsTileView component, add an early return guard
that checks if post?.status === PostStatus.RESOLVED or shouldHideKeyFactors is
true before making any API calls so the effect exits immediately, preventing
unnecessary network requests; update the useEffect closure that calls
ClientPostsApi.getPost and ClientPostsApi.getQuestion to perform this check at
the top and only proceed with the API calls when the guard passes.
---
Outside diff comments:
In
`@front_end/src/app/`(main)/questions/[id]/components/key_factors/item_view/base_rate/key_factor_base_rate.tsx:
- Around line 94-98: The click handler that uses router.push(baseRate.source)
will fail for external URLs—update the onClick logic inside the component that
references router.push, baseRate.source, showSourceError and hasSource to open
external links correctly: if baseRate.source is an external URL use
window.open(baseRate.source, "_blank", "noreferrer noopener") (or better yet
replace the clickable div with an anchor element <a href=... target="_blank"
rel="noopener noreferrer"> to get native external-link behavior and
accessibility), and keep the existing showSourceError/hasSource conditional
rendering unchanged so the source-missing text still displays when appropriate.
In
`@front_end/src/app/`(main)/questions/[id]/components/key_factors/key_factors_question_consumer_section.tsx:
- Around line 38-42: The helper openKeyFactorsElement currently always calls
sendAnalyticsEvent("KeyFactorClick", { event_label: "fromTopList" }), which
misattributes the "View all" button; update openKeyFactorsElement (or create a
small wrapper) so it accepts an optional analyticsLabel parameter (or a
skipAnalytics flag) and only sends KeyFactorClick when the caller intends it;
change the "View all" button call site to either pass a different label (e.g.,
"viewAll") or skip sending analytics, leaving other callers unchanged; ensure
the function signature and all callers (including the View all invocation) are
updated to avoid hardcoding event_label: "fromTopList".
---
Nitpick comments:
In `@front_end/src/app/`(main)/components/comments_feed_provider.tsx:
- Around line 128-133: The sorting in setAndSortCombinedKeyFactors currently
only orders by vote.score and lacks the id tie-breaker used in the initial sort,
causing inconsistent ordering; update the sort comparator in
setAndSortCombinedKeyFactors to include the same secondary tie-breaker (e.g.,
(b.vote?.score || 0) - (a.vote?.score || 0) || b.id - a.id) so ties on score are
resolved by id like the initial sort, then call setCombinedKeyFactors with the
resulting sorted array.
In
`@front_end/src/app/`(main)/questions/[id]/components/key_factors/add_in_comment/key_factors_add_in_comment_llm_suggestions.tsx:
- Line 680: The inline creation of created_at with new Date().toISOString()
causes a new timestamp every render; compute and store a stable fallback
timestamp once (e.g. in a useRef/useState or useMemo when the suggestion is
first created/initialized) and replace the direct new Date().toISOString() usage
in the synthetic key factor creation with that stable value so relative-time UI
and downstream rendering don't jitter; locate the created_at assignment in
key_factors_add_in_comment_llm_suggestions.tsx and use the persistedRef or
memoizedTimestamp instead of calling new Date() on each render.
In
`@front_end/src/app/`(main)/questions/[id]/components/key_factors/item_view/key_factor_strength_item.tsx:
- Around line 61-63: The upScore is hardcoded to 5 while downScore uses
StrengthValues.NO_IMPACT, so replace the literal with a named constant for
consistency: use an existing StrengthValues member (e.g., StrengthValues.STRONG)
or add a new StrengthValues entry for the strong/up score and assign upScore
from it; update the code around isDirection, upScore and downScore (references:
isDirection, upScore, downScore, KeyFactorVoteTypes.DIRECTION,
StrengthValues.NO_IMPACT) so both scores come from StrengthValues rather than a
magic number.
- Around line 111-114: The code unsafely uses a double type assertion ("resp as
unknown as KeyFactorVoteAggregate") which bypasses TypeScript checks; update the
voteKeyFactor function to return a properly typed
Promise<KeyFactorVoteAggregate> OR add a runtime type guard like
isValidKeyFactorVoteAggregate(resp) and only call setKeyFactorVote(keyFactor.id,
resp) after the guard passes (otherwise handle/log the error); reference the
voteKeyFactor call, the resp variable, the KeyFactorVoteAggregate type,
setKeyFactorVote, and keyFactor.id when making the change.
- Around line 115-120: The catch block in key_factor_strength_item.tsx currently
only logs errors and reverts optimistic state; add a user-facing error
notification there so failures are visible. Inside the existing catch (e) { ...
} for the vote handler, call your app's toast/notification helper (e.g.,
toast.error or showErrorToast) with a short message like "Failed to submit vote"
and include e.message/details, then proceed to clearOptimistic() and
setSubmitting(false); also add the necessary import for the toast/notification
utility at the top of the file.
In
`@front_end/src/app/`(main)/questions/[id]/components/key_factors/item_view/key_factor_strength_voter.tsx:
- Around line 28-32: StrengthScale declares unused props (score and mode) which
is misleading; update the StrengthScale component signature and prop type to
only accept { count: number } (remove score and mode from the FC generic and the
destructured params) and then update any callers to stop passing score/mode or
adjust them to only pass count so the API matches implementation; modify the
StrengthScale declaration and its usages to refer only to count to keep the
component surface accurate.
In
`@front_end/src/app/`(main)/questions/[id]/components/key_factors/item_view/news/key_factor_news_item.tsx:
- Around line 56-117: There are three anchors using the same {...linkProps}
(favicon link, title link, metadata link) which creates redundant tab stops;
consolidate them by keeping only one interactive anchor for the whole card (wrap
the favicon, title, and metadata inside a single <a {...linkProps}>), remove the
extra <a> elements (keep ImageWithFallback and the title/metadata spans inside
the same anchor), ensure non-link elements (e.g., the decorative fallback span)
remain semantic, and keep existing helpers/props (linkProps,
getProxiedFaviconUrl, ImageWithFallback, title, source, date, formatDate,
isCompact, isConsumer) so styles and accessibility attributes are preserved.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9e98c3ff-6550-43ac-a2eb-bd215caf5f35
📒 Files selected for processing (45)
comments/serializers/key_factors.pyfront_end/messages/cs.jsonfront_end/messages/en.jsonfront_end/messages/es.jsonfront_end/messages/pt.jsonfront_end/messages/zh-TW.jsonfront_end/messages/zh.jsonfront_end/src/app/(main)/components/comments_feed_provider.tsxfront_end/src/app/(main)/questions/[id]/components/key_factors/add_in_comment/key_factors_add_in_comment_llm_suggestions.tsxfront_end/src/app/(main)/questions/[id]/components/key_factors/item_creation/driver/impact_direction_label.tsxfront_end/src/app/(main)/questions/[id]/components/key_factors/item_view/base_rate/key_factor_base_rate.tsxfront_end/src/app/(main)/questions/[id]/components/key_factors/item_view/base_rate/key_factor_base_rate_frequency.tsxfront_end/src/app/(main)/questions/[id]/components/key_factors/item_view/base_rate/key_factor_base_rate_trend.tsxfront_end/src/app/(main)/questions/[id]/components/key_factors/item_view/base_rate/key_factor_direction_voter.tsxfront_end/src/app/(main)/questions/[id]/components/key_factors/item_view/driver/key_factor_driver.tsxfront_end/src/app/(main)/questions/[id]/components/key_factors/item_view/dropdown_menu_items.tsxfront_end/src/app/(main)/questions/[id]/components/key_factors/item_view/index.tsxfront_end/src/app/(main)/questions/[id]/components/key_factors/item_view/key_factor_card_container.tsxfront_end/src/app/(main)/questions/[id]/components/key_factors/item_view/key_factor_strength_item.tsxfront_end/src/app/(main)/questions/[id]/components/key_factors/item_view/key_factor_strength_voter.tsxfront_end/src/app/(main)/questions/[id]/components/key_factors/item_view/key_factor_vote_panels.tsxfront_end/src/app/(main)/questions/[id]/components/key_factors/item_view/more_panel.tsxfront_end/src/app/(main)/questions/[id]/components/key_factors/item_view/news/key_factor_news.tsxfront_end/src/app/(main)/questions/[id]/components/key_factors/item_view/news/key_factor_news_item.tsxfront_end/src/app/(main)/questions/[id]/components/key_factors/item_view/panel_container.tsxfront_end/src/app/(main)/questions/[id]/components/key_factors/item_view/question_link/question_link_agree_voter.tsxfront_end/src/app/(main)/questions/[id]/components/key_factors/item_view/question_link/question_link_key_factor_item.tsxfront_end/src/app/(main)/questions/[id]/components/key_factors/item_view/segmented_progress_bar.tsxfront_end/src/app/(main)/questions/[id]/components/key_factors/item_view/thumb_vote_buttons.tsxfront_end/src/app/(main)/questions/[id]/components/key_factors/item_view/use_optimistic_vote.tsfront_end/src/app/(main)/questions/[id]/components/key_factors/item_view/use_vote_panel.tsfront_end/src/app/(main)/questions/[id]/components/key_factors/item_view/vertical_impact_bar.tsxfront_end/src/app/(main)/questions/[id]/components/key_factors/item_view/vote_panel.tsxfront_end/src/app/(main)/questions/[id]/components/key_factors/key_factors_comment_section.tsxfront_end/src/app/(main)/questions/[id]/components/key_factors/key_factors_consumer_carousel.tsxfront_end/src/app/(main)/questions/[id]/components/key_factors/key_factors_feed.tsxfront_end/src/app/(main)/questions/[id]/components/key_factors/key_factors_grid_placeholder.tsxfront_end/src/app/(main)/questions/[id]/components/key_factors/key_factors_question_consumer_section.tsxfront_end/src/app/(main)/questions/[id]/components/key_factors/key_factors_question_section.tsxfront_end/src/app/(main)/questions/[id]/components/key_factors/questions_feed_view/key_factors_tile_view.tsxfront_end/src/app/(main)/questions/[id]/components/question_layout/consumer_question_layout/index.tsxfront_end/src/app/(main)/questions/[id]/components/question_view/consumer_question_view/index.tsxfront_end/src/components/comment_feed/comment_card.tsxfront_end/src/types/comment.tsfront_end/src/utils/key_factors.ts
💤 Files with no reviewable changes (3)
- front_end/src/app/(main)/questions/[id]/components/key_factors/item_view/base_rate/key_factor_direction_voter.tsx
- front_end/src/app/(main)/questions/[id]/components/key_factors/item_view/segmented_progress_bar.tsx
- front_end/src/app/(main)/questions/[id]/components/key_factors/item_view/dropdown_menu_items.tsx
ncarazon
force-pushed
the
feat/key-factors-new-iteration
branch
from
7cf6b52 to
767a25a
Compare
Contributor
|
I think title of linked question shouldn't look like that (one word in every line). Can you please check with design? 
|
cemreinanc
approved these changes
Mar 14, 2026
Contributor
cemreinanc
left a comment
cemreinanc
left a comment
There was a problem hiding this comment.
Left couple of comments. LGTM otherwise.
ncarazon
force-pushed
the
feat/key-factors-new-iteration
branch
from
890b7c6 to
c20a690
Compare
ncarazon
force-pushed
the
feat/key-factors-new-iteration
branch
from
c20a690 to
b46d152
Compare
ncarazon
force-pushed
the
feat/key-factors-new-iteration
branch
from
b46d152 to
0b0e30b
Compare
ncarazon
force-pushed
the
feat/key-factors-new-iteration
branch
from
0b0e30b to
82b58fe
Compare
This was referenced Mar 30, 2026
Merged
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
2 participants
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.
This PR implements the 1st iteration of the Key Factors Redesign
Iteration 1: Unified Cards + Voting + Grid Layout
Summary by CodeRabbit
New Features
Internationalization
UI/UX