Open
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR refactors multiple-choice option visibility and ordering logic by introducing a dynamic visibility function ( Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 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
🧪 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:
|
![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: 4
🧹 Nitpick comments (1)
front_end/src/components/post_card/multiple_choice_tile/choice_option.tsx (1)
101-109: Keep the full option text discoverable.Now that every label is single-line truncated, long options with the same prefix can collapse to the same visible text. Adding a
titlehere keeps the full wording available without changing layout.📝 Suggested tweak
<div + title={choice} className={cn( "resize-label min-w-0 flex-1 pr-2.5 text-left text-sm font-normal leading-4", "truncate", labelClassName )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/components/post_card/multiple_choice_tile/choice_option.tsx` around lines 101 - 109, The label div that renders the option text (in choice_option.tsx) is truncated and should expose the full text via a title attribute; update the div that uses cn(...) and renders {choice} to include title={choice} (or title={`${choice}`} to handle non-strings) so long options remain discoverable while keeping the single-line layout; reference the rendered element that currently uses className={cn(...)} and the choice and labelClassName props when making this change.
🤖 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/multiple_choices_chart_view/choices_legend/index.tsx:
- Around line 85-89: The legend popover/dropdown still renders full labels for
hidden choices; update the popover item rendering to use the same truncation as
the pinned chips by passing truncateLabel(label || choice, 30) instead of label
|| choice. Locate the popover/dropdown mapping where hidden legend items are
rendered (the code near legendChoices.map / ChoiceCheckbox in
choices_legend/index.tsx) and replace the raw label with truncateLabel(...),
ensuring any component props or keys that display text (e.g., ChoiceCheckbox or
the dropdown item renderer) receive the truncated string.
In
`@front_end/src/components/detailed_question_card/detailed_question_card/multiple_choice_chart_card.tsx`:
- Around line 253-257: The embed path currently uses
buildChoicesWithOthers(choiceItems) which inserts a hardcoded English "Others"
label causing the legend to remain untranslated; update the embed branch in
embedChoiceItems to pass the localized label (reuse the same i18n key/translator
used by the tooltip path) into buildChoicesWithOthers or map the returned
"Others" item label through the component's translator before passing to
MultipleChoiceTile so the legend uses the translated string; reference the
embedChoiceItems constant, buildChoicesWithOthers, and MultipleChoiceTile when
making this change.
In `@front_end/src/utils/questions/choices.ts`:
- Around line 46-66: The code incorrectly assumes all inactive entries share the
same aggregationTimestamps/userTimestamps by using inactive[0] (aggTs, userTs)
and summing aggregationValues[i]/userValues[i]; instead build a unified
timestamp set from all inactive[].aggregationTimestamps and all
inactive[].userTimestamps, create per-entry maps from timestamp->value, and then
compute aggregationValues and userValues by iterating that unified timestamp
list and summing values from each map (using null for missing entries) so
timestamps are aligned across different histories; update the logic that uses
aggTs/userTs, sumNullable, aggregationValues and userValues accordingly and keep
generateChoiceItemsFromGroupQuestions()’s timestamp semantics consistent.
- Around line 201-214: The current isCPHidden branch is mutating item.active to
make the first N definition-order items active, which swaps the selected set;
instead, when isCPHidden (cpRevealsOn or hideCP logic) do not change any
item.active flags — preserve the existing CP-based selected set — and only
adjust presentation order/visible slice if needed; so in the isCPHidden branch
remove the choiceItems.forEach mutation that sets item.active, keep computing
effectiveActiveCount via getEffectiveVisibleCount for presentation purposes, and
return choiceItems (or a reordered/trimmed view) without altering any
item.active properties.
---
Nitpick comments:
In `@front_end/src/components/post_card/multiple_choice_tile/choice_option.tsx`:
- Around line 101-109: The label div that renders the option text (in
choice_option.tsx) is truncated and should expose the full text via a title
attribute; update the div that uses cn(...) and renders {choice} to include
title={choice} (or title={`${choice}`} to handle non-strings) so long options
remain discoverable while keeping the single-line layout; reference the rendered
element that currently uses className={cn(...)} and the choice and
labelClassName props when making this change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e12e446a-3f7e-4fd3-98d2-235ee8440ff4
📒 Files selected for processing (15)
front_end/src/app/(main)/questions/[id]/components/group_timeline.tsxfront_end/src/app/(main)/questions/[id]/components/multiple_choices_chart_view/choices_legend/index.tsxfront_end/src/app/(main)/questions/[id]/components/multiple_choices_chart_view/index.tsxfront_end/src/components/consumer_post_card/group_forecast_card/numeric_forecast_card.tsxfront_end/src/components/consumer_post_card/group_forecast_card/percentage_forecast_card.tsxfront_end/src/components/detailed_question_card/detailed_group_card/index.tsxfront_end/src/components/detailed_question_card/detailed_question_card/multiple_choice_chart_card.tsxfront_end/src/components/detailed_question_card/embeds.tsfront_end/src/components/forecast_maker/forecast_maker_question/forecast_maker_multiple_choice.tsxfront_end/src/components/post_card/group_of_questions_tile/index.tsxfront_end/src/components/post_card/multiple_choice_tile/choice_option.tsxfront_end/src/components/post_card/multiple_choice_tile/index.tsxfront_end/src/components/post_card/question_tile/index.tsxfront_end/src/constants/questions.tsfront_end/src/utils/questions/choices.ts
💤 Files with no reviewable changes (2)
- front_end/src/components/detailed_question_card/embeds.ts
- front_end/src/components/forecast_maker/forecast_maker_question/forecast_maker_multiple_choice.tsx
...rc/app/(main)/questions/[id]/components/multiple_choices_chart_view/choices_legend/index.tsx
Show resolved
Hide resolved
Comment on lines
253
to
+257
| const embedChoiceItems = useMemo(() => { | ||
| if (!embedMode) return choiceItems; | ||
| const othersLabel = "Others"; | ||
|
|
||
| return buildEmbedChoicesWithOthers( | ||
| choiceItems, | ||
| maxVisibleCheckboxes, | ||
| othersLabel | ||
| ); | ||
| }, [choiceItems, embedMode, maxVisibleCheckboxes]); | ||
| return buildChoicesWithOthers(choiceItems); | ||
| }, [choiceItems, embedMode]); |
Contributor
There was a problem hiding this comment.
Localize the embed "Others" label.
buildChoicesWithOthers() falls back to a hardcoded English label, and this embed path passes that item straight into MultipleChoiceTile, so the legend will show English text in every locale. Reuse the translated label here, like the tooltip path already does.
🌐 Suggested fix
const embedChoiceItems = useMemo(() => {
if (!embedMode) return choiceItems;
-
- return buildChoicesWithOthers(choiceItems);
- }, [choiceItems, embedMode]);
+ const inactiveCount = choiceItems.filter((c) => !c.active).length;
+ return buildChoicesWithOthers(
+ choiceItems,
+ t("othersCount", { count: inactiveCount })
+ );
+ }, [choiceItems, embedMode, t]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@front_end/src/components/detailed_question_card/detailed_question_card/multiple_choice_chart_card.tsx`
around lines 253 - 257, The embed path currently uses
buildChoicesWithOthers(choiceItems) which inserts a hardcoded English "Others"
label causing the legend to remain untranslated; update the embed branch in
embedChoiceItems to pass the localized label (reuse the same i18n key/translator
used by the tooltip path) into buildChoicesWithOthers or map the returned
"Others" item label through the component's translator before passing to
MultipleChoiceTile so the legend uses the translated string; reference the
embedChoiceItems constant, buildChoicesWithOthers, and MultipleChoiceTile when
making this change.
Contributor
Author
There was a problem hiding this comment.
Does embeds have localization implemented? Or are they just EN? check the embed related files outside ofthis pr
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find embed-related files
fd -e tsx -e ts embed --exclude node_modules | grep -v __tests__ | head -30
echo "---"
# Check if embed routes/pages use next-intl or any i18n
rg -n "useTranslations|getTranslations|NextIntlClientProvider|next-intl" --type=tsx --type=ts -g "*embed*" -l 2>/dev/null || \
rg -rn "useTranslations|getTranslations|NextIntlClientProvider|next-intl" --include="*.tsx" --include="*.ts" -l | xargs grep -l -i "embed" 2>/dev/null | head -20
echo "---"
# Check embed page/layout files for locale setup
fd -e tsx -e ts -e js . front_end/src/app --exec grep -l -i "embed" {} \; 2>/dev/null | head -20
Length of output: 2786
🏁 Script executed:
#!/bin/bash
# Check the embed page and layout for locale/i18n setup
cat front_end/src/app/(embed)/questions/embed/[id]/page.tsx
echo "=== layout ==="
fd layout front_end/src/app/\(embed\) 2>/dev/null | xargs cat 2>/dev/null
echo "=== embed_question_card ==="
cat front_end/src/app/(embed)/questions/components/embed_question_card.tsx
echo "=== embed_question_plot ==="
cat front_end/src/app/(embed)/questions/components/embed_question_plot.tsx
echo "=== check for useTranslations in embed files ==="
rg -rn "useTranslations|getTranslations|NextIntlClientProvider" front_end/src/app/\(embed\)/
Length of output: 146
🏁 Script executed:
#!/bin/bash
# Use fd to find and cat embed files safely
fd "page.tsx" front_end/src --full-path | grep embed | xargs cat
echo "=== layout files ==="
fd "layout.tsx" front_end/src --full-path | grep embed | xargs cat 2>/dev/null || echo "no layout found"
echo "=== useTranslations check in embed components ==="
fd ".tsx" front_end/src --full-path | grep "(embed)" | xargs grep -l "useTranslations\|getTranslations\|NextIntlClientProvider" 2>/dev/null || echo "none found"
echo "=== embed_question_card ==="
fd "embed_question_card.tsx" front_end/src | xargs cat
echo "=== embed_question_plot ==="
fd "embed_question_plot.tsx" front_end/src | xargs cat
Length of output: 9009
🏁 Script executed:
#!/bin/bash
# Check root layout for NextIntlClientProvider
fd "layout.tsx" front_end/src/app --full-path | head -5 | xargs cat
echo "=== check i18n/middleware config ==="
fd "middleware" front_end/src --full-path | xargs cat 2>/dev/null | head -60
echo "=== check i18n config ==="
fd "i18n" front_end/src --full-path -e ts | head -5 | xargs cat 2>/dev/null
fd "i18n.ts" front_end --full-path | xargs cat 2>/dev/null
Length of output: 5874
🏁 Script executed:
#!/bin/bash
# Check the true root app layout
fd "layout.tsx" front_end/src/app --max-depth 2 | xargs cat
echo "=== next-intl plugin config ==="
cat front_end/next.config.js 2>/dev/null || cat front_end/next.config.ts 2>/dev/null || cat front_end/next.config.mjs 2>/dev/null
echo "=== check if NextIntlClientProvider is anywhere in the tree ==="
rg -rn "NextIntlClientProvider" front_end/src/
Length of output: 11583
@cemreinanc — Embeds do have full localization. Here's what the codebase shows:
- Root layout (
front_end/src/app/layout.tsx) wraps every route — including(embed)— withNextIntlClientProvider messages={messages}. There is no embed-specific override that strips it out. next-intlis wired globally viacreateNextIntlPlugin()innext.config.js, and locale is resolved per-request from the user's cookie /Accept-Languageheader ini18n.ts.- Grep for
NextIntlClientProviderin the(embed)subtree returns nothing — because it inherits from the root, not because it's absent.
So useTranslations() works correctly in embed mode, and the hardcoded "Others" string from buildChoicesWithOthers's English fallback will appear untranslated for non-English users. The original suggested fix remains valid:
const embedChoiceItems = useMemo(() => {
if (!embedMode) return choiceItems;
-
- return buildChoicesWithOthers(choiceItems);
- }, [choiceItems, embedMode]);
+ const inactiveCount = choiceItems.filter((c) => !c.active).length;
+ return buildChoicesWithOthers(
+ choiceItems,
+ t("othersCount", { count: inactiveCount })
+ );
+ }, [choiceItems, embedMode, t]);🐇 🔍 Curious rabbit traces the provider up through the layout tree...
✏️ Learnings added
Learnt from: cemreinanc
Repo: Metaculus/metaculus PR: 4444
File: front_end/src/components/detailed_question_card/detailed_question_card/multiple_choice_chart_card.tsx:253-257
Timestamp: 2026-03-12T21:53:38.992Z
Learning: In the Metaculus frontend (Next.js App Router), the embed route group at `front_end/src/app/(embed)/` does NOT have its own `NextIntlClientProvider`. It inherits translations from the root layout at `front_end/src/app/layout.tsx`, which wraps all routes with `<NextIntlClientProvider messages={messages}>`. Therefore, `useTranslations()` works correctly in all embed components, and i18n strings should always be used instead of hardcoded English text.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: cemreinanc
Repo: Metaculus/metaculus PR: 4444
File: front_end/src/utils/questions/choices.ts:201-214
Timestamp: 2026-03-12T21:47:53.792Z
Learning: In `front_end/src/utils/questions/choices.ts` (`generateChoiceItemsFromMultipleChoiceForecast`), when CP is hidden (`hideCP` or `cpRevealsOn`), selecting items by definition order (first N) instead of by CP rank is intentional. Using CP-based selection when CP is hidden would implicitly reveal which options have high CP, defeating the purpose of hiding it. This behavior is by design for the Metaculus frontend.
Learnt from: aseckin
Repo: Metaculus/metaculus PR: 4001
File: front_end/src/app/(futureeval)/futureeval/components/futureeval-leaderboard-table.tsx:15-22
Timestamp: 2026-01-20T16:49:51.584Z
Learning: The mockTranslate function in front_end/src/app/(futureeval)/futureeval/components/futureeval-leaderboard-table.tsx is an intentional workaround to satisfy the entryLabel function signature from shared AIB utils while keeping FutureEval translation-free.
Comment on lines
+46
to
+66
| const aggTs = inactive[0]?.aggregationTimestamps ?? []; | ||
| const userTs = inactive[0]?.userTimestamps ?? []; | ||
|
|
||
| const sumNullable = (vals: Array<number | null | undefined>) => { | ||
| let sum = 0; | ||
| let hasAny = false; | ||
| for (const v of vals) { | ||
| if (v != null) { | ||
| sum += v; | ||
| hasAny = true; | ||
| } | ||
| } | ||
| return hasAny ? Number(sum.toFixed(6)) : null; | ||
| }; | ||
|
|
||
| const aggregationValues = aggTs.map((_, i) => | ||
| sumNullable(inactive.map((o) => o.aggregationValues[i])) | ||
| ); | ||
| const userValues = userTs.map((_, i) => | ||
| sumNullable(inactive.map((o) => o.userValues[i])) | ||
| ); |
Contributor
There was a problem hiding this comment.
Don't aggregate "Others" by array position.
This assumes every inactive choice shares aggregationTimestamps and userTimestamps with inactive[0], then sums ...Values[i] by index. generateChoiceItemsFromGroupQuestions() in this same file builds those timestamp arrays per question, so different close times or sparse histories will misalign the series and produce incorrect "Others" values.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@front_end/src/utils/questions/choices.ts` around lines 46 - 66, The code
incorrectly assumes all inactive entries share the same
aggregationTimestamps/userTimestamps by using inactive[0] (aggTs, userTs) and
summing aggregationValues[i]/userValues[i]; instead build a unified timestamp
set from all inactive[].aggregationTimestamps and all inactive[].userTimestamps,
create per-entry maps from timestamp->value, and then compute aggregationValues
and userValues by iterating that unified timestamp list and summing values from
each map (using null for missing entries) so timestamps are aligned across
different histories; update the logic that uses aggTs/userTs, sumNullable,
aggregationValues and userValues accordingly and keep
generateChoiceItemsFromGroupQuestions()’s timestamp semantics consistent.
ncarazon
approved these changes
Mar 12, 2026
...rc/app/(main)/questions/[id]/components/multiple_choices_chart_view/choices_legend/index.tsx
Show resolved
Hide resolved
Comment on lines
328
to
330
| const sortedAggregationTimestamps = uniq(aggregationTimestamps).sort( | ||
| (a, b) => a - b | ||
| ); |
Contributor
There was a problem hiding this comment.
Resorts already sorted timestamps.
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.
fixes #4318
Summary by CodeRabbit
New Features
Bug Fixes
Refactor