Conversation
📝 WalkthroughWalkthroughAdds a memoized hook and consumer carousel for combining top key factors and question-link aggregates, integrates them into the prediction flow UI (flow variant), and provides a helper to open the flow comments panel and scroll to a comment. Changes
Sequence DiagramsequenceDiagram
participant User
participant Carousel as KeyFactorsConsumerCarousel
participant Analytics
participant Hook as useTopKeyFactorsCarouselItems
participant Helper as openFlowCommentsAndScrollToComment
participant Browser
participant DOM
User->>Carousel: Click carousel item
Carousel->>Analytics: Emit KeyFactorClick event
Carousel->>Hook: (uses memoized items) identify item.kind
alt item.kind == "keyFactor" and variant == "flow"
Carousel->>Helper: openFlowCommentsAndScrollToComment(commentId)
Helper->>Browser: update URL hash (no reload)
Helper->>DOM: ensure toggle `PREDICTION_FLOW_COMMENTS_TOGGLE_ID` is open
Helper->>DOM: scroll target comment into view (with offset & retry)
DOM-->>User: comment visible
else onKeyFactorClick callback provided
Carousel->>User: call onKeyFactorClick callback
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
🧹 Preview Environment Cleaned UpThe preview environment for this PR has been destroyed.
Cleanup triggered by PR close at 2026-01-29T15:22:57Z |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@front_end/src/app/`(main)/questions/[id]/components/key_factors/key_factors_consumer_carousel.tsx:
- Around line 49-61: The onClick handler in key_factors_consumer_carousel.tsx
double-counts analytics because sendAnalyticsEvent("KeyFactorClick", ...) is
called unconditionally while openKeyFactorsElement already emits the same event;
update the handler so that sendAnalyticsEvent is only called when using the
onKeyFactorClick path (i.e., when onKeyFactorClick is defined) or remove the
redundant sendAnalyticsEvent call and rely on openKeyFactorsElement to emit the
event, leaving onKeyFactorClick callers responsible for their own analytics;
adjust the onClick flow around onKeyFactorClick, openKeyFactorsElement, and
sendAnalyticsEvent to ensure a single emission.
- Around line 49-79: The two <button> elements in
key_factors_consumer_carousel.tsx should explicitly set type="button" to avoid
accidental form submission; update both button usages (the branch that calls
onKeyFactorClick/openKeyFactorsElement and the alternate branch that calls
openKeyFactorsElement for question links) to include type="button" while leaving
existing onClick logic (onKeyFactorClick, openKeyFactorsElement,
sendAnalyticsEvent) and className/KeyFactorItem props unchanged.
🧹 Nitpick comments (2)
front_end/src/app/(main)/questions/[id]/components/key_factors/hooks/use_top_key_factors_carousel_items.tsx (1)
11-14: Avoid duplicatingTopItemdefinitions.Line 11‑14 redefines
TopItem, which is already exported from.../types.ts; keeping a single source prevents drift.♻️ Proposed refactor
-import { KeyFactor } from "@/types/comment"; +import { KeyFactor } from "@/types/comment"; +import type { TopItem } from "../types"; @@ -export type TopItem = - | { kind: "keyFactor"; keyFactor: KeyFactor } - | { kind: "questionLink"; link: FetchedAggregateCoherenceLink }; +export type { TopItem };front_end/src/app/(main)/questions/[id]/components/key_factors/key_factors_question_section.tsx (1)
45-70: Consider reusing hook outputs to avoid duplicate question‑link filtering.Line 45‑60 duplicates filtering and
totalCountthat the hook already computes; pullingquestionLinkAggregates/totalCountfromuseTopKeyFactorsCarouselItemskeeps the logic in sync.♻️ Proposed refactor
-import useCoherenceLinksContext from "@/app/(main)/components/coherence_links_provider"; @@ -import { FC, useEffect, useMemo } from "react"; +import { FC, useEffect } from "react"; @@ - const { aggregateCoherenceLinks } = useCoherenceLinksContext(); - - const questionLinkAggregates = useMemo( - () => - aggregateCoherenceLinks?.data.filter( - (it) => it.links_nr > 1 && it.strength !== null && it.direction !== null - ) ?? [], - [aggregateCoherenceLinks?.data] - ); - - const hasQuestionLinks = questionLinkAggregates.length > 0; - - const totalCount = useMemo( - () => combinedKeyFactors.length + questionLinkAggregates.length, - [combinedKeyFactors.length, questionLinkAggregates.length] - ); - - const { items: topItems } = useTopKeyFactorsCarouselItems({ + const { + items: topItems, + questionLinkAggregates, + totalCount, + } = useTopKeyFactorsCarouselItems({ keyFactors: combinedKeyFactors, sortMode: isFlow ? "strength" : "freshness", }); + const hasQuestionLinks = questionLinkAggregates.length > 0;
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
front_end/src/app/(main)/questions/[id]/components/key_factors/hooks/use_top_key_factors_carousel_items.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/key_factors_consumer_carousel.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/types.tsfront_end/src/app/(prediction-flow)/components/prediction_flow_comments.tsxfront_end/src/app/(prediction-flow)/components/prediction_flow_post.tsxfront_end/src/app/(prediction-flow)/helpers.ts
🧰 Additional context used
🧬 Code graph analysis (4)
front_end/src/app/(main)/questions/[id]/components/key_factors/key_factors_consumer_carousel.tsx (6)
front_end/src/app/(main)/questions/[id]/components/key_factors/hooks/use_top_key_factors_carousel_items.tsx (1)
TopItem(11-13)front_end/src/app/(main)/questions/[id]/components/key_factors/types.ts (1)
TopItem(12-14)front_end/src/app/(main)/questions/[id]/components/question_layout/question_layout_context.tsx (1)
useQuestionLayout(62-70)front_end/src/app/(main)/questions/[id]/components/key_factors/utils.ts (1)
openKeyFactorsSectionAndScrollTo(29-92)front_end/src/utils/analytics.ts (1)
sendAnalyticsEvent(11-17)front_end/src/app/(main)/questions/[id]/components/key_factors/item_view/index.tsx (1)
KeyFactorItem(26-76)
front_end/src/app/(main)/questions/[id]/components/key_factors/key_factors_question_consumer_section.tsx (2)
front_end/src/app/(main)/questions/[id]/components/key_factors/hooks/use_top_key_factors_carousel_items.tsx (2)
useTopKeyFactorsCarouselItems(28-92)MAX_TOP_KEY_FACTORS(9-9)front_end/src/app/(main)/questions/[id]/components/key_factors/utils.ts (1)
openKeyFactorsSectionAndScrollTo(29-92)
front_end/src/app/(prediction-flow)/helpers.ts (1)
front_end/src/app/(prediction-flow)/components/prediction_flow_comments.tsx (1)
PREDICTION_FLOW_COMMENTS_TOGGLE_ID(10-11)
front_end/src/app/(main)/questions/[id]/components/key_factors/types.ts (2)
front_end/src/app/(main)/questions/[id]/components/key_factors/hooks/use_top_key_factors_carousel_items.tsx (1)
TopItem(11-13)front_end/src/types/coherence.ts (1)
FetchedAggregateCoherenceLink(59-66)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build Docker Image / Build Docker Image
- GitHub Check: Backend Checks
- GitHub Check: integration-tests
- GitHub Check: Frontend Checks
🔇 Additional comments (10)
front_end/src/app/(main)/questions/[id]/components/key_factors/types.ts (1)
1-14: LGTM: TopItem union is clear and reusable.Good addition for sharing carousel item shapes across components.
front_end/src/app/(main)/questions/[id]/components/key_factors/item_view/question_link/question_link_key_factor_item.tsx (1)
29-37: Nice: className passthrough enables external styling.This keeps the item component flexible without changing its defaults.
Also applies to: 164-169
front_end/src/app/(main)/questions/[id]/components/key_factors/hooks/use_top_key_factors_carousel_items.tsx (1)
21-91: Hook sorting/limiting logic looks solid.Clear separation of key factors vs. question links with sensible limiting.
front_end/src/app/(main)/questions/[id]/components/key_factors/key_factors_question_consumer_section.tsx (1)
27-60: LGTM: hook‑driven topItems + carousel integration.Nice consolidation around the new hook and carousel component.
front_end/src/app/(main)/questions/[id]/components/key_factors/key_factors_question_section.tsx (1)
91-128: Flow variant rendering and count‑based title look good.The conditional carousel rendering aligns well with the new flow UX.
front_end/src/app/(prediction-flow)/components/prediction_flow_post.tsx (2)
9-9: LGTM — KeyFactorsQuestionSection import is appropriate.
150-160: Nice integration of key factors into the flow details.
Placement and thevariant="flow"usage align with the compact UI requirement.front_end/src/app/(prediction-flow)/components/prediction_flow_comments.tsx (1)
10-32: LGTM — exported toggle ID and wiring are clear.front_end/src/app/(prediction-flow)/helpers.ts (2)
6-7: LGTM — helper import is fine.
65-117: The toggle lookup is correct and will work with SectionToggle's actual DOM structure. Theidis applied to theDisclosurewrapper element, andDisclosureButton(rendered as<button>) is a direct child witharia-expandedmanaged by Headlessui. The currentquerySelector("button[aria-expanded]")call will correctly find the button and is already resilient to the component structure.Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
...t_end/src/app/(main)/questions/[id]/components/key_factors/key_factors_consumer_carousel.tsx
Show resolved
Hide resolved
...t_end/src/app/(main)/questions/[id]/components/key_factors/key_factors_consumer_carousel.tsx
Show resolved
Hide resolved
cemreinanc
left a comment
cemreinanc
left a comment
There was a problem hiding this comment.
LGTM.
One small issue this edge gradient in light mode doesnt match with the background.
ncarazon
force-pushed
the
feat/show-key-factors-in-prediction-flow
branch
from
517cb48 to
bba3c02
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
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/key_factors_question_consumer_section.tsx (1)
49-57: Addtype="button"to the "View All" button.Same defensive practice as the carousel buttons: explicitly set
type="button"to prevent accidental form submissions.🐛 Proposed fix
<button + type="button" onClick={() => { openKeyFactorsElement("[id='key-factors']"); sendAnalyticsEvent("KeyFactorViewAllClick"); }} className="text-sm text-blue-700 hover:text-blue-800 dark:text-blue-700-dark dark:hover:text-blue-600-dark" >
🧹 Nitpick comments (1)
front_end/src/app/(main)/questions/[id]/components/key_factors/key_factors_question_section.tsx (1)
57-69: Consider usingtotalCountfrom the hook to avoid duplication.The
totalCountcomputed here (lines 57-60) duplicates the same calculation insideuseTopKeyFactorsCarouselItems. Destructuring it from the hook would reduce code duplication and ensure consistency.♻️ Proposed refactor
- const totalCount = useMemo( - () => combinedKeyFactors.length + questionLinkAggregates.length, - [combinedKeyFactors.length, questionLinkAggregates.length] - ); - const { factorsLimit } = user?.id ? getKeyFactorsLimits(combinedKeyFactors, user.id) : { factorsLimit: 0 }; - const { items: topItems } = useTopKeyFactorsCarouselItems({ + const { items: topItems, totalCount } = useTopKeyFactorsCarouselItems({ keyFactors: combinedKeyFactors, sortMode: isFlow ? "strength" : "freshness", });
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
front_end/src/app/(main)/questions/[id]/components/key_factors/hooks/use_top_key_factors_carousel_items.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/key_factors_consumer_carousel.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/types.tsfront_end/src/app/(prediction-flow)/components/prediction_flow_comments.tsxfront_end/src/app/(prediction-flow)/components/prediction_flow_post.tsxfront_end/src/app/(prediction-flow)/helpers.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- front_end/src/app/(prediction-flow)/helpers.ts
- front_end/src/app/(prediction-flow)/components/prediction_flow_post.tsx
- front_end/src/app/(main)/questions/[id]/components/key_factors/item_view/question_link/question_link_key_factor_item.tsx
- front_end/src/app/(prediction-flow)/components/prediction_flow_comments.tsx
- front_end/src/app/(main)/questions/[id]/components/key_factors/hooks/use_top_key_factors_carousel_items.tsx
- front_end/src/app/(main)/questions/[id]/components/key_factors/types.ts
🧰 Additional context used
🧬 Code graph analysis (3)
front_end/src/app/(main)/questions/[id]/components/key_factors/key_factors_question_consumer_section.tsx (3)
front_end/src/app/(main)/questions/[id]/components/key_factors/hooks/use_top_key_factors_carousel_items.tsx (2)
useTopKeyFactorsCarouselItems(28-92)MAX_TOP_KEY_FACTORS(9-9)front_end/src/app/(main)/questions/[id]/components/key_factors/utils.ts (1)
openKeyFactorsSectionAndScrollTo(29-92)front_end/src/utils/analytics.ts (1)
sendAnalyticsEvent(11-17)
front_end/src/app/(main)/questions/[id]/components/key_factors/key_factors_question_section.tsx (4)
front_end/src/types/post.ts (1)
PostWithForecasts(214-218)front_end/src/app/(main)/questions/[id]/components/key_factors/hooks.ts (1)
getKeyFactorsLimits(371-406)front_end/src/app/(main)/questions/[id]/components/key_factors/hooks/use_top_key_factors_carousel_items.tsx (1)
useTopKeyFactorsCarouselItems(28-92)front_end/src/app/(prediction-flow)/helpers.ts (1)
openFlowCommentsAndScrollToComment(65-118)
front_end/src/app/(main)/questions/[id]/components/key_factors/key_factors_consumer_carousel.tsx (7)
front_end/src/types/post.ts (1)
PostWithForecasts(214-218)front_end/src/app/(main)/questions/[id]/components/key_factors/hooks/use_top_key_factors_carousel_items.tsx (1)
TopItem(11-13)front_end/src/app/(main)/questions/[id]/components/key_factors/types.ts (1)
TopItem(12-14)front_end/src/app/(main)/questions/[id]/components/question_layout/question_layout_context.tsx (1)
useQuestionLayout(62-70)front_end/src/app/(main)/questions/[id]/components/key_factors/utils.ts (1)
openKeyFactorsSectionAndScrollTo(29-92)front_end/src/utils/analytics.ts (1)
sendAnalyticsEvent(11-17)front_end/src/app/(main)/questions/[id]/components/key_factors/item_view/index.tsx (1)
KeyFactorItem(26-76)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build Docker Image / Build Docker Image
- GitHub Check: integration-tests
- GitHub Check: Backend Checks
- GitHub Check: Frontend Checks
🔇 Additional comments (7)
front_end/src/app/(main)/questions/[id]/components/key_factors/key_factors_consumer_carousel.tsx (2)
1-21: LGTM!The imports, type definitions, and component signature are well-structured. The
Propstype correctly defines the optionalonKeyFactorClickcallback andlightVariantstyling prop.
32-41: LGTM!The
openKeyFactorsElementhelper correctly chains the layout expansion request, scroll-to behavior, and analytics in a single call path.front_end/src/app/(main)/questions/[id]/components/key_factors/key_factors_question_consumer_section.tsx (2)
27-36: LGTM!Good use of the new
useTopKeyFactorsCarouselItemshook to centralize data fetching and transformation logic. TheopenKeyFactorsElementhelper maintains a clear sequence of layout expansion, scroll, and analytics.
60-60: LGTM!Clean integration of
KeyFactorsConsumerCarouselwith the computedtopItems.front_end/src/app/(main)/questions/[id]/components/key_factors/key_factors_question_section.tsx (3)
118-128: Verify question link handling in flow mode.When
onKeyFactorClickis provided, it only handleskeyFactoritems. ForquestionLinkitems,KeyFactorsConsumerCarouselfalls back toopenKeyFactorsElement, which opens the key factors section and scrolls—but in flow mode, this may not be the intended behavior since flow mode usesopenFlowCommentsAndScrollToCommentfor navigation.If question links should behave differently in flow mode (e.g., navigate to a related comment or do nothing), consider either:
- Adding an
onQuestionLinkClickcallback to the carousel- Filtering out question links when in flow mode
91-95: LGTM!Clean conditional logic for section title and default open state based on the variant.
129-140: LGTM!The non-flow rendering path is preserved correctly with the existing
ExpandableContentandKeyFactorsFeedcomponents.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
2 participants
Closes #4028
This PR adds key factors to the prediction flow.
Scroll to the comment behavior:
scroll-to-comment.mov
Summary by CodeRabbit
New Features
Style
✏️ Tip: You can customize this high-level summary in your review settings.