Open
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)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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
🚀 Preview EnvironmentYour preview environment is ready!
Details
ℹ️ Preview Environment InfoIsolation:
Limitations:
Cleanup:
|
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: 3
🤖 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/src/app/`(main)/questions/[id]/components/key_factors/item_view/base_rate/key_factor_base_rate.tsx:
- Line 47: The base-rate card is incorrectly using direction voting: in the
KeyFactorBaseRate component (the JSX prop
voteType={KeyFactorVoteTypes.DIRECTION}) switch the voteType to
KeyFactorVoteTypes.STRENGTH so base-rate semantics remain strength-based; locate
the JSX where voteType is passed (voteType prop on the key factor base-rate
component) and replace DIRECTION with STRENGTH to prevent incorrect downvote
values in the new panel flow.
- Line 99: The hardcoded label "(source)" in the KeyFactorBaseRate component
should be replaced with a localized string: use your app's translation helper
(e.g., t(...) or useTranslations(...)) inside the KeyFactorBaseRate component to
render a translation key like "keyFactors.compactSource" instead of the literal
"(source)", and add that key to the locale files for all languages; locate the
JSX rendering the label in key_factor_base_rate (the string "(source)") and swap
it for the translation call so i18n picks up the label.
In
`@front_end/src/app/`(main)/questions/[id]/components/key_factors/item_view/key_factor_strength_voter.tsx:
- Around line 38-39: The component key_factor_strength_voter.tsx is forcing
lowercase via the Tailwind class "lowercase" on the localized string
t("votesWithCount", { count }), which can break translator intent and
locale-specific casing; remove the "lowercase" utility from the div's class list
and let the translation supply proper casing (or, if you need visual styling,
apply a presentation-only solution that doesn't alter the source text such as a
locale-aware CSS rule or adjust the translation strings), keeping the reference
to t("votesWithCount", { count }) unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 00f5e540-270f-4f9f-b736-138f20b18687
📒 Files selected for processing (11)
front_end/messages/cs.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_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_trend.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/question_link/question_link_key_factor_item.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- front_end/messages/pt.json
- front_end/src/app/(main)/components/comments_feed_provider.tsx
- front_end/messages/zh.json
- front_end/messages/zh-TW.json
...pp/(main)/questions/[id]/components/key_factors/item_view/base_rate/key_factor_base_rate.tsx
Show resolved
Hide resolved
...pp/(main)/questions/[id]/components/key_factors/item_view/base_rate/key_factor_base_rate.tsx
Outdated
Show resolved
Hide resolved
...src/app/(main)/questions/[id]/components/key_factors/item_view/key_factor_strength_voter.tsx
Outdated
Show resolved
Hide resolved
ncarazon
force-pushed
the
feat/key-factors-new-iteration
branch
from
767a25a to
717aa41
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
front_end/src/app/(main)/questions/[id]/components/key_factors/item_view/base_rate/key_factor_base_rate.tsx (1)
47-47:⚠️ Potential issue | 🟠 MajorUse strength voting semantics for base-rate cards.
Line 47 still passes
KeyFactorVoteTypes.DIRECTION; base-rate cards should stay onSTRENGTHto avoid incorrect vote meaning/payload in the panel flow.Proposed fix
- voteType={KeyFactorVoteTypes.DIRECTION} + voteType={KeyFactorVoteTypes.STRENGTH}🤖 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 at line 47, The base-rate card is incorrectly using KeyFactorVoteTypes.DIRECTION which produces the wrong vote semantics/payload; update the prop in the KeyFactorBaseRate component usage to KeyFactorVoteTypes.STRENGTH so base-rate cards emit strength votes. Locate the JSX where voteType={KeyFactorVoteTypes.DIRECTION} is passed (in key_factor_base_rate.tsx / KeyFactorBaseRate) and change that symbol to KeyFactorVoteTypes.STRENGTH, leaving surrounding props/handlers unchanged.
🧹 Nitpick comments (3)
front_end/src/app/(main)/questions/[id]/components/key_factors/item_view/key_factor_strength_voter.tsx (1)
24-24: Consider cleaning up the_modeprop contract.
_modeis still part of the public props and defaulted on Line 141, but it has no effect now. That can confuse callers about supported behavior. If compatibility is no longer needed, remove it; otherwise mark it explicitly deprecated in the prop type.Suggested cleanup
type Props = { keyFactorId: number; vote: KeyFactorVoteAggregate; className?: string; allowVotes?: boolean; - _mode?: "forecaster" | "consumer"; footerControls?: ReactElement; }; @@ const KeyFactorStrengthVoter: FC<Props> = ({ keyFactorId, vote, className, allowVotes, - _mode = "forecaster", footerControls, }) => {Also applies to: 141-141
🤖 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 at line 24, The _mode prop on the KeyFactorStrengthVoter component is exposed in the public props but is unused and confusing; remove _mode from the component props type and from any default value assignment (and delete any no-op references in key_factor_strength_voter.tsx), or if you must keep it for backward compatibility, annotate it as deprecated (add a /** `@deprecated` */ JSDoc on the _mode property in the props/interface and leave a clear comment near the default assignment) so callers know it has no effect; update or run a quick grep for usages to adjust callers if you remove it.front_end/src/app/(main)/questions/[id]/components/key_factors/item_view/question_link/question_link_key_factor_item.tsx (2)
197-200: Addrelfor safer new-tab links.Line 199 opens a new tab via
target="_blank"withoutrel. Addrel="noopener noreferrer"for safer opener behavior.🔐 Proposed tweak
<Link href={getPostLink({ id: otherQuestion.post_id })} target="_blank" + rel="noopener noreferrer" className={cn(🤖 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/question_link/question_link_key_factor_item.tsx around lines 197 - 200, The Link rendering in question_link_key_factor_item uses target="_blank" without a rel attribute; update the Link element (the one calling getPostLink({ id: otherQuestion.post_id })) to include rel="noopener noreferrer" so new-tab navigation is safe (preventing access to window.opener and avoiding potential referrer leaks) while preserving the existing href, target and className (cn) props.
61-80: Extract vote mapping to a helper to avoid drift.The
1/-1/null -> agree/disagree/nullmapping is duplicated in state init and the sync effect. A tiny helper keeps this logic single-sourced.♻️ Proposed refactor
+const mapUserVote = (vote: number | null | undefined): "agree" | "disagree" | null => + vote === 1 ? "agree" : vote === -1 ? "disagree" : null; + const [userVote, setUserVote] = useState<"agree" | "disagree" | null>( - link.votes?.user_vote === 1 - ? "agree" - : link.votes?.user_vote === -1 - ? "disagree" - : null + mapUserVote(link.votes?.user_vote) ); useEffect(() => { - setUserVote( - link.votes?.user_vote === 1 - ? "agree" - : link.votes?.user_vote === -1 - ? "disagree" - : null - ); + setUserVote(mapUserVote(link.votes?.user_vote)); }, [link.id, link.votes?.user_vote]);🤖 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/question_link/question_link_key_factor_item.tsx around lines 61 - 80, Extract the duplicated mapping logic that converts numeric vote values to the string union into a single helper (e.g., mapVoteValueToUserVote) and use it both when initializing the userVote state and inside the useEffect that syncs votes; specifically replace the inline ternary used in useState for userVote and the one inside the effect that calls setUserVote with a call to the new helper, leaving setUserVote, userVote, link.votes?.user_vote, and the existing useEffect intact so behavior is unchanged.
🤖 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/src/app/`(main)/components/comments_feed_provider.tsx:
- Around line 140-143: The setCombinedKeyFactors updater updates a key factor's
vote but doesn't reorder the list, leaving stale rank positions; modify the
setCombinedKeyFactors((prev) => ...) callback used in comments_feed_provider.tsx
so after mapping the updated item (matching keyFactorId and replacing vote with
aggregate) you return a new array sorted by the ranking metric (e.g., vote or
score) in the correct order (descending or as your UI expects); ensure you do
this immutably (map => sort on the new array) so the updated key factor moves to
its proper position in the list.
In
`@front_end/src/app/`(main)/questions/[id]/components/key_factors/item_view/base_rate/key_factor_base_rate.tsx:
- Around line 93-100: Validate and sanitize the URL before rendering: ensure the
value in baseRate.source (used in key_factor_base_rate.tsx) and the analogous
source used in key_factor_base_rate_trend.tsx is checked against an allowlist of
safe schemes (only http:// and https://) and only render the <a> tag when the
URL passes that check; if it fails, render a non-clickable fallback (plain text
or omitted link) and avoid inserting the raw value into href to prevent
javascript:, data:, or other unsafe schemes from causing XSS.
---
Duplicate comments:
In
`@front_end/src/app/`(main)/questions/[id]/components/key_factors/item_view/base_rate/key_factor_base_rate.tsx:
- Line 47: The base-rate card is incorrectly using KeyFactorVoteTypes.DIRECTION
which produces the wrong vote semantics/payload; update the prop in the
KeyFactorBaseRate component usage to KeyFactorVoteTypes.STRENGTH so base-rate
cards emit strength votes. Locate the JSX where
voteType={KeyFactorVoteTypes.DIRECTION} is passed (in key_factor_base_rate.tsx /
KeyFactorBaseRate) and change that symbol to KeyFactorVoteTypes.STRENGTH,
leaving surrounding props/handlers unchanged.
---
Nitpick comments:
In
`@front_end/src/app/`(main)/questions/[id]/components/key_factors/item_view/key_factor_strength_voter.tsx:
- Line 24: The _mode prop on the KeyFactorStrengthVoter component is exposed in
the public props but is unused and confusing; remove _mode from the component
props type and from any default value assignment (and delete any no-op
references in key_factor_strength_voter.tsx), or if you must keep it for
backward compatibility, annotate it as deprecated (add a /** `@deprecated` */
JSDoc on the _mode property in the props/interface and leave a clear comment
near the default assignment) so callers know it has no effect; update or run a
quick grep for usages to adjust callers if you remove it.
In
`@front_end/src/app/`(main)/questions/[id]/components/key_factors/item_view/question_link/question_link_key_factor_item.tsx:
- Around line 197-200: The Link rendering in question_link_key_factor_item uses
target="_blank" without a rel attribute; update the Link element (the one
calling getPostLink({ id: otherQuestion.post_id })) to include rel="noopener
noreferrer" so new-tab navigation is safe (preventing access to window.opener
and avoiding potential referrer leaks) while preserving the existing href,
target and className (cn) props.
- Around line 61-80: Extract the duplicated mapping logic that converts numeric
vote values to the string union into a single helper (e.g.,
mapVoteValueToUserVote) and use it both when initializing the userVote state and
inside the useEffect that syncs votes; specifically replace the inline ternary
used in useState for userVote and the one inside the effect that calls
setUserVote with a call to the new helper, leaving setUserVote, userVote,
link.votes?.user_vote, and the existing useEffect intact so behavior is
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1116ff6f-184c-494b-9090-9649ea7193d7
📒 Files selected for processing (11)
front_end/messages/cs.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_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_trend.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/question_link/question_link_key_factor_item.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
- front_end/messages/zh.json
- front_end/messages/zh-TW.json
- front_end/messages/es.json
- front_end/messages/pt.json
- front_end/src/app/(main)/questions/[id]/components/key_factors/item_view/base_rate/key_factor_base_rate_trend.tsx
...pp/(main)/questions/[id]/components/key_factors/item_view/base_rate/key_factor_base_rate.tsx
Show resolved
Hide resolved
ncarazon
force-pushed
the
feat/key-factors-new-iteration
branch
from
717aa41 to
25adbd1
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
front_end/src/app/(main)/questions/[id]/components/key_factors/item_view/question_link/question_link_agree_voter.tsx (1)
114-130:⚠️ Potential issue | 🟠 MajorOnly update parent-visible state after the vote actually applies.
handleVote()callsonChange(next)and flips the copy hint beforepushVote()gets past the auth/aggregationIdguards or the server response. That means sign-in prompts and failed mutations can leave the parent UI out of sync withcurrentVote.Possible fix
- const pushVote = async (next: "agree" | "disagree" | null) => { - if (!aggregationId) return; + const pushVote = async ( + next: "agree" | "disagree" | null + ): Promise<boolean> => { + if (!aggregationId) return false; if (!user) { setCurrentModal({ type: "signin" }); - return; + return false; } - if (user.is_bot) return; + if (user.is_bot) return false; const vote: AggregateLinkVoteValue = next === "agree" ? 1 : next === "disagree" ? -1 : null; setSubmitting(true); setOptimistic(vote); try { const res = await voteAggregateCoherenceLink(aggregationId, vote); - if ("errors" in res) return; + if ("errors" in res) return false; const data = res.data; if ("strength" in data) { onStrengthChange?.(data.strength ?? null); } await updateCoherenceLinks(); + return true; } catch (e) { console.error("Failed to vote aggregate coherence link", e); + return false; } finally { clearOptimistic(); setSubmitting(false); } }; ... - setShowCopyHint(!hasPersonalCopy && next === "agree"); - onChange?.(next); - void pushVote(next); + void pushVote(next).then((applied) => { + if (!applied) return; + setShowCopyHint(!hasPersonalCopy && next === "agree"); + onChange?.(next); + });Also applies to: 141-157
🤖 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/question_link/question_link_agree_voter.tsx around lines 114 - 130, pushVote currently updates parent-visible state (via onChange and flipping the copy hint in handleVote) before auth/aggregationId checks and the server mutation succeed, causing parent UI to get out-of-sync on sign-in prompts or failed mutations; change the flow so pushVote (and handleVote) only perform local optimistic UI changes (e.g., setOptimistic, setSubmitting) but do NOT call onChange or mutate parent-visible state until after voteAggregateCoherenceLink returns successfully, and on any early return (no aggregationId, unauthenticated, is_bot) or on server error revert the optimistic state and avoid calling onChange; use the existing symbols pushVote, handleVote, setOptimistic, setSubmitting, voteAggregateCoherenceLink, onChange and currentVote to implement this conditional commit/rollback behavior.
♻️ Duplicate comments (4)
front_end/src/app/(main)/components/comments_feed_provider.tsx (1)
140-144:⚠️ Potential issue | 🟡 MinorRe-sort after vote updates to keep rank order correct.
The functional update pattern is a good improvement. However, when the vote score changes, the list is not re-sorted, so key factors can remain in stale positions. This is inconsistent with
setAndSortCombinedKeyFactors(Line 128-133) which does sort.💡 Proposed fix
setCombinedKeyFactors((prev) => - prev.map((kf) => + [...prev.map((kf) => kf.id === keyFactorId ? { ...kf, vote: aggregate } : kf - ) + )].sort( + (a, b) => (b.vote?.score || 0) - (a.vote?.score || 0) || b.id - a.id + ) );🤖 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 140 - 144, The update to setCombinedKeyFactors uses a functional map to replace the voted item but doesn't re-sort the array, leaving ranks stale; update the logic that handles the vote (the setCombinedKeyFactors functional update for keyFactorId/aggregate) to re-sort the resulting list by vote (same comparator used by setAndSortCombinedKeyFactors) before setting, or simply call setAndSortCombinedKeyFactors with the updated item so the list is re-ordered after the vote change.front_end/src/app/(main)/questions/[id]/components/key_factors/key_factors_consumer_carousel.tsx (1)
49-53:⚠️ Potential issue | 🟠 MajorAdd keyboard activation for
role="button"elements.Both interactive
divwrappers haverole="button"andtabIndex={0}but lackonKeyDownhandlers. Keyboard users can focus these elements but cannot activate them with Enter or Space, violating WAI-ARIA button semantics.Proposed fix
+const handleKeyDown = ( + e: React.KeyboardEvent, + callback: () => void +) => { + if (e.key === "Enter" || e.key === " ") { + e.preventDefault(); + callback(); + } +}; <div className="cursor-pointer text-left no-underline" role="button" tabIndex={0} + onKeyDown={(e) => + handleKeyDown(e, () => { + if (onKeyFactorClick) { + onKeyFactorClick(item.keyFactor); + sendAnalyticsEvent("KeyFactorClick", { + event_label: "fromTopList", + }); + } else { + openKeyFactorsElement(`[id="key-factor-${item.keyFactor.id}"]`); + } + }) + } onClick={(e) => {Apply the same pattern to the second
divwrapper at lines 77-81.Also applies to: 77-81
🤖 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_consumer_carousel.tsx around lines 49 - 53, The two interactive divs in key_factors_consumer_carousel.tsx (the wrappers with role="button", tabIndex={0} and onClick handlers) lack keyboard activation; add an onKeyDown handler alongside each onClick that listens for Enter (key === 'Enter') and Space (key === ' ' or key === 'Spacebar') and calls the same click handler (preventDefault for Space to avoid scrolling) so keyboard users can activate the control; implement this pattern for both the first wrapper (the onClick at lines ~49-53) and the second wrapper (the onClick at lines ~77-81).front_end/src/app/(main)/questions/[id]/components/key_factors/item_view/base_rate/key_factor_base_rate.tsx (1)
88-100:⚠️ Potential issue | 🟠 MajorAllowlist the compact source URL before rendering the anchor.
This branch still writes
baseRate.sourcedirectly intohref. Any unexpected scheme here turns a tiny metadata link into unsafe external navigation. Restrict it tohttp:/https:and skip the anchor otherwise.One way to harden it
const safeSource = typeof baseRate.source === "string" && /^https?:\/\//i.test(baseRate.source.trim()) ? baseRate.source.trim() : null;- ) : hasSource ? ( + ) : safeSource ? ( <a - href={baseRate.source} + href={safeSource} target="_blank" rel="noopener noreferrer" className="text-blue-600 hover:underline dark:text-blue-600-dark" > ({t("source")}) </a> ) : 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 88 - 100, The anchor currently uses baseRate.source directly which can allow unsafe schemes; validate and compact the URL before rendering by computing a safeSource (e.g., ensure typeof baseRate.source === "string", trim it, and only accept it if it matches /^https?:\/\//i) and then use safeSource in the href and to gate rendering of the <a> (keep existing showSourceError / hasSource logic but replace the hasSource branch to render the anchor only when safeSource is truthy, otherwise render the plain text or fallback that was intended).front_end/src/app/(main)/questions/[id]/components/key_factors/item_view/key_factor_vote_panels.tsx (1)
26-35:⚠️ Potential issue | 🟠 MajorBiome will still fail on this
forEachcallback.Line 33 implicitly returns
p.setShowPanel(false), which still tripslint/suspicious/useIterableCallbackReturn.Minimal fix
target.setShowPanel(open); if (open) { - others.forEach((p) => p.setShowPanel(false)); + others.forEach((p) => { + p.setShowPanel(false); + }); }🤖 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_vote_panels.tsx around lines 26 - 35, The forEach callback in toggleExclusive implicitly returns p.setShowPanel(false) which trips the lint rule; change the callback to have an explicit statement body (e.g., use a block: others.forEach((p) => { p.setShowPanel(false); });) or replace the forEach with a for...of loop to call p.setShowPanel(false) with no returned value, referencing toggleExclusive and the others / setShowPanel identifiers to locate the code.
🧹 Nitpick comments (6)
front_end/src/app/(main)/components/comments_feed_provider.tsx (2)
161-163: Avoid unnecessary object spread for unchanged comments.Line 162 creates a new object reference for comments that weren't modified, which can trigger unnecessary re-renders in consuming components.
♻️ Proposed fix
} - return { ...comment }; + return comment; });🤖 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 161 - 163, In the comments mapping logic (inside the map callback that currently does "return { ...comment }"), avoid creating a new object for unchanged items; instead return the original comment reference when no changes are needed and only create a new object when the comment is being updated (e.g., when id matches). Update the map callback in CommentsFeedProvider (the function that maps over comments) to conditionally return comment (unchanged) or a new object with updated fields (changed) to prevent unnecessary re-renders.
128-133: Consistent sorting within provider, but note cross-component divergence.The tie-break
b.id - a.idmatches the initial sort (Line 123), which is good for internal consistency. However,use_top_key_factors_carousel_items.tsxuses a different tie-break order (score → freshness → id), which could produce different orderings when consumers re-sort.If the intent is for all views to show the same order when sorted by "strength", consider aligning the comparators.
🤖 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 current setAndSortCombinedKeyFactors comparator in setAndSortCombinedKeyFactors sorts by vote score then id (b.vote?.score || 0) - (a.vote?.score || 0) || b.id - a.id, which diverges from use_top_key_factors_carousel_items.tsx’s comparator (score → freshness → id); update setAndSortCombinedKeyFactors to match that ordering by inserting the same freshness tie-break between score and id (e.g., compare freshness timestamp/field used in the carousel comparator, with the same sort direction), ensuring the comparator references the same freshness property and keeps id as the final tie-break to guarantee consistent ordering across components.front_end/src/app/(main)/questions/[id]/components/key_factors/item_view/question_link/question_link_key_factor_item.tsx (1)
211-214: Consider fixing the type mismatch instead of double-casting.The
as unknown as QuestionWithNumericForecastscast suggests a type incompatibility betweenbinaryForecastQuestionand whatBinaryCPBarexpects. This could mask type errors.Consider either:
- Updating the
BinaryCPBarprop type to acceptQuestionWithForecasts- Creating a proper type guard or conversion function
🤖 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/question_link/question_link_key_factor_item.tsx around lines 211 - 214, The prop passed to BinaryCPBar is being force-cast (binaryForecastQuestion as unknown as QuestionWithNumericForecasts) which hides a real type mismatch; either update the BinaryCPBar prop type to accept the broader QuestionWithForecasts shape, or implement a type guard/conversion function (e.g., isQuestionWithNumericForecasts or convertToQuestionWithNumericForecasts) that validates/transforms binaryForecastQuestion into a true QuestionWithNumericForecasts before passing it to BinaryCPBar, and replace the double-cast with that safe check/conversion.front_end/src/app/(main)/questions/[id]/components/key_factors/key_factors_feed.tsx (1)
43-65: Randomization insetOrdermay cause inconsistent ordering across renders.The sorting function uses
Math.random()which can produce different results on each call. While the effect guards against re-randomizing existing items, ifcombinedKeyFactorschanges (e.g., after voting), the randomization could behave unexpectedly sinceMath.random()is called fresh each time the effect runs.Consider seeding the randomization or using a deterministic shuffle based on item IDs if consistent ordering is important.
🤖 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_feed.tsx around lines 43 - 65, The current randomization in the setOrder updater uses Math.random() inside a sort which yields non-deterministic order across renders when combinedKeyFactors changes; replace this with a deterministic shuffle used when initializing order: implement a pure function (e.g., deterministicShuffle or computeSeededScore) that derives a stable score per item from unique properties like kf.id (and optionally kf.freshness) — for example hash the id (and freshness) to a number — then sort by that score instead of Math.random(); keep the existing logic in setOrder to append only new ids when prev exists so existing ordering is preserved.front_end/src/app/(main)/questions/[id]/components/key_factors/key_factors_question_section.tsx (1)
121-141: Flow variant rendersnullwhen no top items exist.In the flow variant (lines 121-131), when
topItems.length === 0, the component rendersnullinside theSectionToggle. This means the section header will still be visible but with empty content. Consider whether this is the intended behavior, or if the entire section should be hidden when there are no items in flow mode.🤖 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_section.tsx around lines 121 - 141, The flow branch currently renders null for content when isFlow is true and topItems.length === 0, leaving the SectionToggle header visible; update the render logic around isFlow/topItems so the entire section (the SectionToggle that wraps this JSX) is not rendered when isFlow && topItems.length === 0. Concretely, locate the conditional that decides rendering of KeyFactorsConsumerCarousel/ExpandableContent and change it to conditionally skip rendering the SectionToggle (or return null from the parent component) for the empty-flow case, keeping KeyFactorsConsumerCarousel, topItems, isFlow and keyFactorsExpanded as the points of reference. Ensure behavior for non-flow mode (ExpandableContent -> KeyFactorsFeed) is unchanged.front_end/src/app/(main)/questions/[id]/components/key_factors/item_view/panel_container.tsx (1)
17-31: Panel position is static and won't update on scroll/resize.
getAnchorStylecomputes position once at render usinggetBoundingClientRect(). If the viewport scrolls or resizes while the panel is open, it will remain at the original coordinates. Consider whether this is acceptable for your use case, or if you need to recalculate on scroll/resize events.Also applies to: 43-43
🤖 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/panel_container.tsx around lines 17 - 31, getAnchorStyle currently computes a fixed position once from anchorRef.getBoundingClientRect and never updates on scroll/resize; change the component that uses getAnchorStyle to store the computed style in state (e.g., anchorStyle) and recalculate it on window 'scroll' and 'resize' (or use a ResizeObserver on the anchor) by calling getAnchorStyle(anchorRef) inside a useEffect, throttling/debouncing or using requestAnimationFrame to avoid jank, and make sure to remove listeners/observers in the cleanup; apply the same change to the other usage noted around the second getAnchorStyle call so the panel position updates correctly when viewport changes.
🤖 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/src/app/`(main)/questions/[id]/components/key_factors/add_in_comment/key_factors_add_in_comment_llm_suggestions.tsx:
- Line 109: The suggested key factors are showing live author/created_at
metadata because fallbackCreatedAt is set to now and isSuggested isn't
propagated into the metadata renderer; update the code so suggested items do not
show provenance: either (A) thread the isSuggested prop from KeyFactorItem into
MorePanel and in MorePanel conditionally skip rendering the author/relative-time
block when isSuggested is true, or (B) set fallbackCreatedAt to a neutral
sentinel (e.g., new Date(0).toISOString()) instead of new Date().toISOString()
and ensure any fake author display is suppressed for isSuggested items; change
the relevant symbols: fallbackCreatedAt, KeyFactorItem, MorePanel, and
isSuggested to implement one of these fixes.
In
`@front_end/src/app/`(main)/questions/[id]/components/key_factors/item_view/key_factor_strength_item.tsx:
- Around line 61-64: The component currently uses hard-coded scores (upScore =
5, downScore = isDirection ? -5 : StrengthValues.NO_IMPACT) when the thumb is
clicked, ignoring the second-step local selectedOption; change the flow so the
initial thumb click only opens the panel and does not call submit(), move the
mutation call into submit() so it reads the selectedOption from component state,
and map selectedOption -> score inside submit() (use KeyFactorVoteTypes,
StrengthValues and existing selectedOption variable to compute the final score)
so the chosen Low/High/downvote reason is sent in the mutation; update all
similar blocks (lines around 95-123 and 157-165) to follow the same pattern.
In
`@front_end/src/app/`(main)/questions/[id]/components/key_factors/item_view/more_panel.tsx:
- Around line 115-123: The code currently lowercases the translated token
t("by") at render time (t("by").toLowerCase()), which breaks locale-specific
casing and translator control; update the component in more_panel.tsx to stop
lowercasing the token and instead use a full translation that includes the
author (e.g., add/use a single translation key like "createdBy" or
"createdByWithAuthor" and call t("createdBy", { author:
`@${keyFactor.author.username}` }) or t("createdByWithAuthor", { author:
`@${keyFactor.author.username}`, timeAgo: createdDate }) so translators control
casing/punctuation and avoid runtime .toLowerCase() on t("by").
In
`@front_end/src/app/`(main)/questions/[id]/components/key_factors/item_view/news/key_factor_news_item.tsx:
- Around line 61-73: The null branch currently renders an empty gray box losing
the newspaper cue; update the component so when faviconUrl is falsy the same
newspaper fallback used inside ImageWithFallback is rendered instead. Locate the
JSX around faviconUrl and replace the empty <span> with the identical fallback
span that contains <FontAwesomeIcon icon={faNewspaper} ...>, ensuring the
fallback styling matches the ImageWithFallback child; this keeps the visual cue
for sources without favicons while leaving getProxiedFaviconUrl and
ImageWithFallback usage unchanged.
In
`@front_end/src/app/`(main)/questions/[id]/components/key_factors/item_view/panel_container.tsx:
- Around line 53-59: The close button in PanelContainer (the button with
onClick={onClose} and FontAwesomeIcon faXmark) lacks accessible text; add an
aria-label (e.g., aria-label="Close" or aria-label={t('Close')} if using i18n)
to the button element so screen readers can announce its purpose, keeping the
icon-only visual but providing accessible text for the button.
In
`@front_end/src/app/`(main)/questions/[id]/components/key_factors/key_factors_feed.tsx:
- Around line 174-205: The masonry branch (the return that renders <div
className="columns-2..."> with items.map and questionLinkAggregates.map) never
renders the addModal portal, so include the same addModal rendering used in the
0-items and 1-3 items branches inside this 4+ items block; locate the JSX that
maps items and QuestionLinkKeyFactorItem (key `post-key-factor-${kf.id}` and
`question-link-kf-${link.id}`) and add the addModal portal (the same element
referenced as addModal in the other branches) alongside or after those mapped
children so the modal is mounted when the "key-factors" masonry layout is shown.
---
Outside diff comments:
In
`@front_end/src/app/`(main)/questions/[id]/components/key_factors/item_view/question_link/question_link_agree_voter.tsx:
- Around line 114-130: pushVote currently updates parent-visible state (via
onChange and flipping the copy hint in handleVote) before auth/aggregationId
checks and the server mutation succeed, causing parent UI to get out-of-sync on
sign-in prompts or failed mutations; change the flow so pushVote (and
handleVote) only perform local optimistic UI changes (e.g., setOptimistic,
setSubmitting) but do NOT call onChange or mutate parent-visible state until
after voteAggregateCoherenceLink returns successfully, and on any early return
(no aggregationId, unauthenticated, is_bot) or on server error revert the
optimistic state and avoid calling onChange; use the existing symbols pushVote,
handleVote, setOptimistic, setSubmitting, voteAggregateCoherenceLink, onChange
and currentVote to implement this conditional commit/rollback behavior.
---
Duplicate comments:
In `@front_end/src/app/`(main)/components/comments_feed_provider.tsx:
- Around line 140-144: The update to setCombinedKeyFactors uses a functional map
to replace the voted item but doesn't re-sort the array, leaving ranks stale;
update the logic that handles the vote (the setCombinedKeyFactors functional
update for keyFactorId/aggregate) to re-sort the resulting list by vote (same
comparator used by setAndSortCombinedKeyFactors) before setting, or simply call
setAndSortCombinedKeyFactors with the updated item so the list is re-ordered
after the vote change.
In
`@front_end/src/app/`(main)/questions/[id]/components/key_factors/item_view/base_rate/key_factor_base_rate.tsx:
- Around line 88-100: The anchor currently uses baseRate.source directly which
can allow unsafe schemes; validate and compact the URL before rendering by
computing a safeSource (e.g., ensure typeof baseRate.source === "string", trim
it, and only accept it if it matches /^https?:\/\//i) and then use safeSource in
the href and to gate rendering of the <a> (keep existing showSourceError /
hasSource logic but replace the hasSource branch to render the anchor only when
safeSource is truthy, otherwise render the plain text or fallback that was
intended).
In
`@front_end/src/app/`(main)/questions/[id]/components/key_factors/item_view/key_factor_vote_panels.tsx:
- Around line 26-35: The forEach callback in toggleExclusive implicitly returns
p.setShowPanel(false) which trips the lint rule; change the callback to have an
explicit statement body (e.g., use a block: others.forEach((p) => {
p.setShowPanel(false); });) or replace the forEach with a for...of loop to call
p.setShowPanel(false) with no returned value, referencing toggleExclusive and
the others / setShowPanel identifiers to locate the code.
In
`@front_end/src/app/`(main)/questions/[id]/components/key_factors/key_factors_consumer_carousel.tsx:
- Around line 49-53: The two interactive divs in
key_factors_consumer_carousel.tsx (the wrappers with role="button", tabIndex={0}
and onClick handlers) lack keyboard activation; add an onKeyDown handler
alongside each onClick that listens for Enter (key === 'Enter') and Space (key
=== ' ' or key === 'Spacebar') and calls the same click handler (preventDefault
for Space to avoid scrolling) so keyboard users can activate the control;
implement this pattern for both the first wrapper (the onClick at lines ~49-53)
and the second wrapper (the onClick at lines ~77-81).
---
Nitpick comments:
In `@front_end/src/app/`(main)/components/comments_feed_provider.tsx:
- Around line 161-163: In the comments mapping logic (inside the map callback
that currently does "return { ...comment }"), avoid creating a new object for
unchanged items; instead return the original comment reference when no changes
are needed and only create a new object when the comment is being updated (e.g.,
when id matches). Update the map callback in CommentsFeedProvider (the function
that maps over comments) to conditionally return comment (unchanged) or a new
object with updated fields (changed) to prevent unnecessary re-renders.
- Around line 128-133: The current setAndSortCombinedKeyFactors comparator in
setAndSortCombinedKeyFactors sorts by vote score then id (b.vote?.score || 0) -
(a.vote?.score || 0) || b.id - a.id, which diverges from
use_top_key_factors_carousel_items.tsx’s comparator (score → freshness → id);
update setAndSortCombinedKeyFactors to match that ordering by inserting the same
freshness tie-break between score and id (e.g., compare freshness
timestamp/field used in the carousel comparator, with the same sort direction),
ensuring the comparator references the same freshness property and keeps id as
the final tie-break to guarantee consistent ordering across components.
In
`@front_end/src/app/`(main)/questions/[id]/components/key_factors/item_view/panel_container.tsx:
- Around line 17-31: getAnchorStyle currently computes a fixed position once
from anchorRef.getBoundingClientRect and never updates on scroll/resize; change
the component that uses getAnchorStyle to store the computed style in state
(e.g., anchorStyle) and recalculate it on window 'scroll' and 'resize' (or use a
ResizeObserver on the anchor) by calling getAnchorStyle(anchorRef) inside a
useEffect, throttling/debouncing or using requestAnimationFrame to avoid jank,
and make sure to remove listeners/observers in the cleanup; apply the same
change to the other usage noted around the second getAnchorStyle call so the
panel position updates correctly when viewport changes.
In
`@front_end/src/app/`(main)/questions/[id]/components/key_factors/item_view/question_link/question_link_key_factor_item.tsx:
- Around line 211-214: The prop passed to BinaryCPBar is being force-cast
(binaryForecastQuestion as unknown as QuestionWithNumericForecasts) which hides
a real type mismatch; either update the BinaryCPBar prop type to accept the
broader QuestionWithForecasts shape, or implement a type guard/conversion
function (e.g., isQuestionWithNumericForecasts or
convertToQuestionWithNumericForecasts) that validates/transforms
binaryForecastQuestion into a true QuestionWithNumericForecasts before passing
it to BinaryCPBar, and replace the double-cast with that safe check/conversion.
In
`@front_end/src/app/`(main)/questions/[id]/components/key_factors/key_factors_feed.tsx:
- Around line 43-65: The current randomization in the setOrder updater uses
Math.random() inside a sort which yields non-deterministic order across renders
when combinedKeyFactors changes; replace this with a deterministic shuffle used
when initializing order: implement a pure function (e.g., deterministicShuffle
or computeSeededScore) that derives a stable score per item from unique
properties like kf.id (and optionally kf.freshness) — for example hash the id
(and freshness) to a number — then sort by that score instead of Math.random();
keep the existing logic in setOrder to append only new ids when prev exists so
existing ordering is preserved.
In
`@front_end/src/app/`(main)/questions/[id]/components/key_factors/key_factors_question_section.tsx:
- Around line 121-141: The flow branch currently renders null for content when
isFlow is true and topItems.length === 0, leaving the SectionToggle header
visible; update the render logic around isFlow/topItems so the entire section
(the SectionToggle that wraps this JSX) is not rendered when isFlow &&
topItems.length === 0. Concretely, locate the conditional that decides rendering
of KeyFactorsConsumerCarousel/ExpandableContent and change it to conditionally
skip rendering the SectionToggle (or return null from the parent component) for
the empty-flow case, keeping KeyFactorsConsumerCarousel, topItems, isFlow and
keyFactorsExpanded as the points of reference. Ensure behavior for non-flow mode
(ExpandableContent -> KeyFactorsFeed) is unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: fa2ef132-842b-4a86-a994-35e76992602b
📒 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/dropdown_menu_items.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/base_rate/key_factor_direction_voter.tsx
🚧 Files skipped from review as they are similar to previous changes (22)
- front_end/src/app/(main)/questions/[id]/components/key_factors/item_view/base_rate/key_factor_base_rate_trend.tsx
- front_end/src/app/(main)/questions/[id]/components/key_factors/item_view/base_rate/key_factor_base_rate_frequency.tsx
- front_end/src/utils/key_factors.ts
- front_end/src/types/comment.ts
- front_end/src/app/(main)/questions/[id]/components/key_factors/item_view/news/key_factor_news.tsx
- front_end/src/app/(main)/questions/[id]/components/key_factors/item_view/thumb_vote_buttons.tsx
- front_end/src/app/(main)/questions/[id]/components/key_factors/item_view/vote_panel.tsx
- front_end/src/app/(main)/questions/[id]/components/key_factors/item_view/key_factor_strength_voter.tsx
- front_end/messages/zh.json
- front_end/src/app/(main)/questions/[id]/components/key_factors/item_view/use_vote_panel.ts
- front_end/src/app/(main)/questions/[id]/components/key_factors/item_view/driver/key_factor_driver.tsx
- front_end/src/app/(main)/questions/[id]/components/question_layout/consumer_question_layout/index.tsx
- front_end/src/app/(main)/questions/[id]/components/key_factors/item_view/key_factor_card_container.tsx
- front_end/src/app/(main)/questions/[id]/components/question_view/consumer_question_view/index.tsx
- front_end/messages/cs.json
- comments/serializers/key_factors.py
- front_end/src/components/comment_feed/comment_card.tsx
- front_end/src/app/(main)/questions/[id]/components/key_factors/item_view/vertical_impact_bar.tsx
- front_end/src/app/(main)/questions/[id]/components/key_factors/key_factors_grid_placeholder.tsx
- front_end/src/app/(main)/questions/[id]/components/key_factors/questions_feed_view/key_factors_tile_view.tsx
- front_end/messages/zh-TW.json
- front_end/src/app/(main)/questions/[id]/components/key_factors/key_factors_comment_section.tsx
| }) => { | ||
| const t = useTranslations(); | ||
| const { user } = useAuth(); | ||
| const fallbackCreatedAt = useMemo(() => new Date().toISOString(), []); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C3 'isSuggested|created_at|createdTimeAgoBy|createdTimeAgo|author' front_end/src/app front_end/src/componentsRepository: Metaculus/metaculus
Length of output: 50376
🏁 Script executed:
#!/bin/bash
# First, let's examine the specific file mentioned in the review
wc -l front_end/src/app/\(main\)/questions/\[id\]/components/key_factors/add_in_comment/key_factors_add_in_comment_llm_suggestions.tsxRepository: Metaculus/metaculus
Length of output: 191
🏁 Script executed:
#!/bin/bash
# Read the file around the mentioned lines
cat -n front_end/src/app/\(main\)/questions/\[id\]/components/key_factors/add_in_comment/key_factors_add_in_comment_llm_suggestions.tsx | sed -n '100,120p'Repository: Metaculus/metaculus
Length of output: 799
🏁 Script executed:
#!/bin/bash
# Also check around line 681
cat -n front_end/src/app/\(main\)/questions/\[id\]/components/key_factors/add_in_comment/key_factors_add_in_comment_llm_suggestions.tsx | sed -n '670,690p'Repository: Metaculus/metaculus
Length of output: 936
🏁 Script executed:
#!/bin/bash
# Now search for KeyFactorItem component to see how it renders metadata
rg -n "KeyFactorItem" front_end/src --type tsx --type ts -A 5 | head -100Repository: Metaculus/metaculus
Length of output: 90
🏁 Script executed:
#!/bin/bash
# Search specifically for isSuggested usage in KeyFactorItem or related rendering
rg -n "isSuggested" front_end/src -B 3 -A 3 --type tsx --type tsRepository: Metaculus/metaculus
Length of output: 90
🏁 Script executed:
#!/bin/bash
# Search for KeyFactorItem component without specifying file type
rg -n "KeyFactorItem" front_end/src -A 5 | head -150Repository: Metaculus/metaculus
Length of output: 13906
🏁 Script executed:
#!/bin/bash
# Search for isSuggested usage
rg -n "isSuggested" front_end/src -B 3 -A 3Repository: Metaculus/metaculus
Length of output: 7652
🏁 Script executed:
#!/bin/bash
# Find KeyFactorItem definition
fd "KeyFactorItem" front_end/srcRepository: Metaculus/metaculus
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Read the KeyFactorItem component to see how it handles isSuggested and renders metadata
cat -n front_end/src/app/\(main\)/questions/\[id\]/components/key_factors/item_view/index.tsxRepository: Metaculus/metaculus
Length of output: 4550
🏁 Script executed:
#!/bin/bash
# Read KeyFactorCardContainer to see how it renders metadata
cat -n front_end/src/app/\(main\)/questions/\[id\]/components/key_factors/item_view/key_factor_card_container.tsxRepository: Metaculus/metaculus
Length of output: 2850
🏁 Script executed:
#!/bin/bash
# Also check KeyFactorNews since that's one of the metadata containers
cat -n front_end/src/app/\(main\)/questions/\[id\]/components/key_factors/item_view/news/key_factor_news.tsx | head -150Repository: Metaculus/metaculus
Length of output: 1118
🏁 Script executed:
#!/bin/bash
# Search for where author and created_at are rendered in KeyFactor components
rg -n "keyFactor\.author|keyFactor\.created_at|formatDate.*created_at" front_end/src/app/\(main\)/questions/\[id\]/components/key_factors/ -A 2Repository: Metaculus/metaculus
Length of output: 1102
🏁 Script executed:
#!/bin/bash
# Check KeyFactorDriver component as well
cat -n front_end/src/app/\(main\)/questions/\[id\]/components/key_factors/item_view/driver/key_factor_driver.tsx | head -100Repository: Metaculus/metaculus
Length of output: 1016
🏁 Script executed:
#!/bin/bash
# Read the more_panel.tsx file to see if it checks isSuggested
cat -n front_end/src/app/\(main\)/questions/\[id\]/components/key_factors/item_view/more_panel.tsxRepository: Metaculus/metaculus
Length of output: 4993
🏁 Script executed:
#!/bin/bash
# Check KeyFactorVotePanels to see if it passes isSuggested to MorePanel
cat -n front_end/src/app/\(main\)/questions/\[id\]/components/key_factors/item_view/key_factor_vote_panels.tsxRepository: Metaculus/metaculus
Length of output: 4727
Suppress created_at and author metadata from suggested key factors, or use a neutral sentinel value.
The isSuggested prop is passed to KeyFactorItem but is not threaded through to MorePanel, which unconditionally displays author and relative creation time (lines 116–123 of more_panel.tsx). This means suggested cards will display the live fallbackCreatedAt timestamp (current time) and the fake author, making previews appear to have actual provenance. Either suppress the metadata section in MorePanel for suggested items or replace fallbackCreatedAt with a neutral sentinel value like an epoch date.
Also applies to: 681-681
🤖 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 109, The suggested key factors are showing live author/created_at
metadata because fallbackCreatedAt is set to now and isSuggested isn't
propagated into the metadata renderer; update the code so suggested items do not
show provenance: either (A) thread the isSuggested prop from KeyFactorItem into
MorePanel and in MorePanel conditionally skip rendering the author/relative-time
block when isSuggested is true, or (B) set fallbackCreatedAt to a neutral
sentinel (e.g., new Date(0).toISOString()) instead of new Date().toISOString()
and ensure any fake author display is suppressed for isSuggested items; change
the relevant symbols: fallbackCreatedAt, KeyFactorItem, MorePanel, and
isSuggested to implement one of these fixes.
Comment on lines
+61
to
+64
| const isDirection = voteType === KeyFactorVoteTypes.DIRECTION; | ||
| const upScore = 5; | ||
| const downScore = isDirection ? -5 : StrengthValues.NO_IMPACT; | ||
|
|
Contributor
There was a problem hiding this comment.
The second-step panel is cosmetic right now.
The mutation path is still hard-coded to 5 / NO_IMPACT / -5, while the new panels only store a local selectedOption. So a user can pick “Low” or a downvote reason after the request has already been sent with the fixed score. The selected panel option needs to feed back into submit(), and the mutation should happen from that second step instead of the initial thumb click.
Also applies to: 95-123, 157-165
🤖 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 - 64, The component currently uses hard-coded scores (upScore =
5, downScore = isDirection ? -5 : StrengthValues.NO_IMPACT) when the thumb is
clicked, ignoring the second-step local selectedOption; change the flow so the
initial thumb click only opens the panel and does not call submit(), move the
mutation call into submit() so it reads the selectedOption from component state,
and map selectedOption -> score inside submit() (use KeyFactorVoteTypes,
StrengthValues and existing selectedOption variable to compute the final score)
so the chosen Low/High/downvote reason is sent in the mutation; update all
similar blocks (lines around 95-123 and 157-165) to follow the same pattern.
Comment on lines
+115
to
+123
| <span className="font-normal text-gray-500 dark:text-gray-500-dark"> | ||
| {t("createdTimeAgo", { timeAgo: createdDate })} | ||
| </span>{" "} | ||
| <span className="font-normal text-gray-600 dark:text-gray-600-dark"> | ||
| {t("by").toLowerCase()}{" "} | ||
| <span className="text-blue-700 dark:text-blue-700-dark"> | ||
| @{keyFactor.author.username} | ||
| </span> | ||
| </span> |
Contributor
There was a problem hiding this comment.
Don’t lowercase translated text at render time.
t("by").toLowerCase() hardcodes English casing rules into the UI. That breaks locales with special case mappings and takes wording control away from translators. Prefer a dedicated full-string translation here, or render the translated token as-is.
🤖 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/more_panel.tsx
around lines 115 - 123, The code currently lowercases the translated token
t("by") at render time (t("by").toLowerCase()), which breaks locale-specific
casing and translator control; update the component in more_panel.tsx to stop
lowercasing the token and instead use a full translation that includes the
author (e.g., add/use a single translation key like "createdBy" or
"createdByWithAuthor" and call t("createdBy", { author:
`@${keyFactor.author.username}` }) or t("createdByWithAuthor", { author:
`@${keyFactor.author.username}`, timeAgo: createdDate }) so translators control
casing/punctuation and avoid runtime .toLowerCase() on t("by").
Comment on lines
61
to
73
| {faviconUrl ? ( | ||
| <ImageWithFallback | ||
| className="size-[42px] cursor-pointer rounded" | ||
| className="size-10 cursor-pointer rounded" | ||
| src={getProxiedFaviconUrl(faviconUrl)} | ||
| alt={`${source} logo`} | ||
| > | ||
| <span className="flex size-[42px] items-center justify-center rounded bg-gray-200 dark:bg-gray-200-dark"> | ||
| <span className="flex size-10 items-center justify-center rounded bg-gray-200 dark:bg-gray-200-dark"> | ||
| <FontAwesomeIcon icon={faNewspaper} size="xl" /> | ||
| </span> | ||
| </ImageWithFallback> | ||
| ) : ( | ||
| <span className="flex size-[42px] cursor-pointer items-center justify-center rounded bg-gray-200 dark:bg-gray-200-dark" /> | ||
| <span className="flex size-10 cursor-pointer items-center justify-center rounded bg-gray-200 dark:bg-gray-200-dark" /> | ||
| )} |
Contributor
There was a problem hiding this comment.
Keep the generic news icon when there’s no favicon URL.
The null branch now renders an empty gray box, so sources without favicons lose the visual cue entirely. Reuse the same newspaper fallback you already render when ImageWithFallback fails.
Minimal fix
- ) : (
- <span className="flex size-10 cursor-pointer items-center justify-center rounded bg-gray-200 dark:bg-gray-200-dark" />
- )}
+ ) : (
+ <span className="flex size-10 items-center justify-center rounded bg-gray-200 dark:bg-gray-200-dark">
+ <FontAwesomeIcon icon={faNewspaper} size="xl" />
+ </span>
+ )}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| {faviconUrl ? ( | |
| <ImageWithFallback | |
| className="size-[42px] cursor-pointer rounded" | |
| className="size-10 cursor-pointer rounded" | |
| src={getProxiedFaviconUrl(faviconUrl)} | |
| alt={`${source} logo`} | |
| > | |
| <span className="flex size-[42px] items-center justify-center rounded bg-gray-200 dark:bg-gray-200-dark"> | |
| <span className="flex size-10 items-center justify-center rounded bg-gray-200 dark:bg-gray-200-dark"> | |
| <FontAwesomeIcon icon={faNewspaper} size="xl" /> | |
| </span> | |
| </ImageWithFallback> | |
| ) : ( | |
| <span className="flex size-[42px] cursor-pointer items-center justify-center rounded bg-gray-200 dark:bg-gray-200-dark" /> | |
| <span className="flex size-10 cursor-pointer items-center justify-center rounded bg-gray-200 dark:bg-gray-200-dark" /> | |
| )} | |
| {faviconUrl ? ( | |
| <ImageWithFallback | |
| className="size-10 cursor-pointer rounded" | |
| src={getProxiedFaviconUrl(faviconUrl)} | |
| alt={`${source} logo`} | |
| > | |
| <span className="flex size-10 items-center justify-center rounded bg-gray-200 dark:bg-gray-200-dark"> | |
| <FontAwesomeIcon icon={faNewspaper} size="xl" /> | |
| </span> | |
| </ImageWithFallback> | |
| ) : ( | |
| <span className="flex size-10 items-center justify-center rounded bg-gray-200 dark:bg-gray-200-dark"> | |
| <FontAwesomeIcon icon={faNewspaper} size="xl" /> | |
| </span> | |
| )} |
🤖 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 61 - 73, The null branch currently renders an empty gray box losing
the newspaper cue; update the component so when faviconUrl is falsy the same
newspaper fallback used inside ImageWithFallback is rendered instead. Locate the
JSX around faviconUrl and replace the empty <span> with the identical fallback
span that contains <FontAwesomeIcon icon={faNewspaper} ...>, ensuring the
fallback styling matches the ImageWithFallback child; this keeps the visual cue
for sources without favicons while leaving getProxiedFaviconUrl and
ImageWithFallback usage unchanged.
Comment on lines
+53
to
+59
| <button | ||
| type="button" | ||
| onClick={onClose} | ||
| className="absolute right-2.5 top-2.5 flex items-center justify-center text-blue-500 hover:text-blue-700 dark:text-blue-500-dark dark:hover:text-blue-700-dark" | ||
| > | ||
| <FontAwesomeIcon icon={faXmark} className="text-xs" /> | ||
| </button> |
Contributor
There was a problem hiding this comment.
Add aria-label to the close button for accessibility.
The close button uses only an icon without accessible text. Screen reader users won't understand its purpose.
Proposed fix
<button
type="button"
onClick={onClose}
+ aria-label="Close"
className="absolute right-2.5 top-2.5 flex items-center justify-center text-blue-500 hover:text-blue-700 dark:text-blue-500-dark dark:hover:text-blue-700-dark"
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <button | |
| type="button" | |
| onClick={onClose} | |
| className="absolute right-2.5 top-2.5 flex items-center justify-center text-blue-500 hover:text-blue-700 dark:text-blue-500-dark dark:hover:text-blue-700-dark" | |
| > | |
| <FontAwesomeIcon icon={faXmark} className="text-xs" /> | |
| </button> | |
| <button | |
| type="button" | |
| onClick={onClose} | |
| aria-label="Close" | |
| className="absolute right-2.5 top-2.5 flex items-center justify-center text-blue-500 hover:text-blue-700 dark:text-blue-500-dark dark:hover:text-blue-700-dark" | |
| > | |
| <FontAwesomeIcon icon={faXmark} className="text-xs" /> | |
| </button> |
🤖 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/panel_container.tsx
around lines 53 - 59, The close button in PanelContainer (the button with
onClick={onClose} and FontAwesomeIcon faXmark) lacks accessible text; add an
aria-label (e.g., aria-label="Close" or aria-label={t('Close')} if using i18n)
to the button element so screen readers can announce its purpose, keeping the
icon-only visual but providing accessible text for the button.
Comment on lines
+174
to
205
| // 4+ items: masonry layout | ||
| return ( | ||
| <div className="flex flex-col gap-2.5" id="key-factors"> | ||
| <div className="columns-2 gap-2.5 md:columns-3" id="key-factors"> | ||
| {items.map((kf) => ( | ||
| <KeyFactorItem | ||
| id={`key-factor-${kf.id}`} | ||
| <div | ||
| key={`post-key-factor-${kf.id}`} | ||
| keyFactor={kf} | ||
| projectPermission={post.user_permission} | ||
| className={keyFactorItemClassName} | ||
| /> | ||
| className="mb-2 break-inside-avoid" | ||
| > | ||
| <KeyFactorItem | ||
| id={`key-factor-${kf.id}`} | ||
| keyFactor={kf} | ||
| projectPermission={post.user_permission} | ||
| isCompact={isMobileCompact} | ||
| /> | ||
| </div> | ||
| ))} | ||
|
|
||
| {questionLinkAggregates.length > 0 && | ||
| questionLinkAggregates.map((link) => ( | ||
| {questionLinkAggregates.map((link) => ( | ||
| <div | ||
| key={`question-link-kf-${link.id}`} | ||
| className="mb-2 break-inside-avoid" | ||
| > | ||
| <QuestionLinkKeyFactorItem | ||
| id={`question-link-kf-${link.id}`} | ||
| key={`question-link-kf-${link.id}`} | ||
| link={link} | ||
| post={post} | ||
| compact={isMobileCompact} | ||
| /> | ||
| ))} | ||
| </div> | ||
| ))} | ||
| </div> | ||
| ); |
Contributor
There was a problem hiding this comment.
addModal is not rendered in the 4+ items layout.
The addModal portal is rendered in the 0-items and 1-3 items branches, but not in the 4+ items masonry layout (lines 174-205). If users can trigger the add flow from elsewhere while viewing 4+ key factors, the modal won't appear.
Proposed fix
// 4+ items: masonry layout
return (
+ <>
<div className="columns-2 gap-2.5 md:columns-3" id="key-factors">
{items.map((kf) => (
// ...
))}
</div>
+ {addModal}
+ </>
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // 4+ items: masonry layout | |
| return ( | |
| <div className="flex flex-col gap-2.5" id="key-factors"> | |
| <div className="columns-2 gap-2.5 md:columns-3" id="key-factors"> | |
| {items.map((kf) => ( | |
| <KeyFactorItem | |
| id={`key-factor-${kf.id}`} | |
| <div | |
| key={`post-key-factor-${kf.id}`} | |
| keyFactor={kf} | |
| projectPermission={post.user_permission} | |
| className={keyFactorItemClassName} | |
| /> | |
| className="mb-2 break-inside-avoid" | |
| > | |
| <KeyFactorItem | |
| id={`key-factor-${kf.id}`} | |
| keyFactor={kf} | |
| projectPermission={post.user_permission} | |
| isCompact={isMobileCompact} | |
| /> | |
| </div> | |
| ))} | |
| {questionLinkAggregates.length > 0 && | |
| questionLinkAggregates.map((link) => ( | |
| {questionLinkAggregates.map((link) => ( | |
| <div | |
| key={`question-link-kf-${link.id}`} | |
| className="mb-2 break-inside-avoid" | |
| > | |
| <QuestionLinkKeyFactorItem | |
| id={`question-link-kf-${link.id}`} | |
| key={`question-link-kf-${link.id}`} | |
| link={link} | |
| post={post} | |
| compact={isMobileCompact} | |
| /> | |
| ))} | |
| </div> | |
| ))} | |
| </div> | |
| ); | |
| // 4+ items: masonry layout | |
| return ( | |
| <> | |
| <div className="columns-2 gap-2.5 md:columns-3" id="key-factors"> | |
| {items.map((kf) => ( | |
| <div | |
| key={`post-key-factor-${kf.id}`} | |
| className="mb-2 break-inside-avoid" | |
| > | |
| <KeyFactorItem | |
| id={`key-factor-${kf.id}`} | |
| keyFactor={kf} | |
| projectPermission={post.user_permission} | |
| isCompact={isMobileCompact} | |
| /> | |
| </div> | |
| ))} | |
| {questionLinkAggregates.map((link) => ( | |
| <div | |
| key={`question-link-kf-${link.id}`} | |
| className="mb-2 break-inside-avoid" | |
| > | |
| <QuestionLinkKeyFactorItem | |
| id={`question-link-kf-${link.id}`} | |
| link={link} | |
| post={post} | |
| compact={isMobileCompact} | |
| /> | |
| </div> | |
| ))} | |
| </div> | |
| {addModal} | |
| </> | |
| ); |
🤖 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_feed.tsx
around lines 174 - 205, The masonry branch (the return that renders <div
className="columns-2..."> with items.map and questionLinkAggregates.map) never
renders the addModal portal, so include the same addModal rendering used in the
0-items and 1-3 items branches inside this 4+ items block; locate the JSX that
maps items and QuestionLinkKeyFactorItem (key `post-key-factor-${kf.id}` and
`question-link-kf-${link.id}`) and add the addModal portal (the same element
referenced as addModal in the other branches) alongside or after those mapped
children so the modal is mounted when the "key-factors" masonry layout is shown.
Contributor
|
I think title of linked question shouldn't look like that (one word in every line). Can you please check with design? 
|
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