fixes on labor hub (v2)#4624
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 35 minutes and 23 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR substantially refactors the labor-hub frontend, introducing new flippable chart-timeline card components, replacing a single rating metric with three literature-based scoring fields (felten, mna, aoe), implementing collision-aware label rendering in charts, and reorganizing the main page layout to support dynamic data visualization with historical timeline comparison. Changes
Sequence DiagramsequenceDiagram
participant User
participant FCTimeline as FlippableChartTimelineCard
participant Suspense
participant API as ServerPostsApi
participant Chart as MultiQuestionLineChart
participant Timeline as BasicQuestionContent
User->>FCTimeline: Render with questionId, prefer, title
FCTimeline->>Suspense: Wrap with QuestionCardSkeleton fallback
Suspense->>API: getPost(questionId, true)
alt Data Loading
API-->>Suspense: Loading...
Suspense-->>User: Show QuestionCardSkeleton
else Data Loaded
API-->>FCTimeline: postData received
FCTimeline->>FCTimeline: Compute resolvedDefaultSide from prefer
FCTimeline->>Chart: Render with historicalValues, extraRows
FCTimeline->>Timeline: Render with timelineMarkers, timelineSubtitle
Chart-->>User: Display chart (left side)
Timeline-->>User: Display timeline (right side)
else Fetch Failed
API-->>FCTimeline: Error
FCTimeline-->>User: Show QuestionCard with NoQuestionPlaceholder
end
User->>FCTimeline: Toggle between chart/timeline
FCTimeline->>Chart: Re-render opposite side
FCTimeline->>Timeline: Re-render opposite side
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (8)
front_end/src/app/(main)/labor-hub/components/question_cards/basic_question.tsx (1)
65-86: Optional: consider wrapping subtitle + timeline in a container with consistent spacing.The fragment renders the subtitle
<div>directly aboveGroupTimelinewith no bottom margin on the subtitle. Depending on howGroupTimelinepaints its top edge, the subtitle may sit flush against the chart. If you notice cramped spacing, add a smallmb-*to the subtitle div or wrap both in aflex flex-col gap-*.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/question_cards/basic_question.tsx around lines 65 - 86, The subtitle div and GroupTimeline are rendered adjacently which can produce cramped spacing; update the JSX around timelineSubtitle and GroupTimeline (see timelineSubtitle and GroupTimeline) to ensure consistent spacing by either adding a bottom margin class to the subtitle div (e.g., mb-2/mb-3) or wrapping both elements in a container with flex flex-col and a gap class (e.g., gap-2) so the subtitle always has breathing room above the timeline.front_end/src/app/(main)/labor-hub/components/question_cards/multi_question_data.ts (1)
43-53: Nit: potential label collision when plain-year and fiscal-year labels coexist.A plain
"2025"and a fiscal"2024-25"both normalize to"2025"and will be merged into the same column (vialabelsSet/historicalByLabel/questionByLabel). This is likely the intended semantics (fiscal year ending → ending calendar year), but it means if a single post unexpectedly exposes both formats, one silently overwrites the other inhistoricalByLabel/questionByLabelduring Map construction. Consider a short comment documenting the intent, or an assertion in dev mode.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/question_cards/multi_question_data.ts around lines 43 - 53, normalizeMultiQuestionLabel collapses fiscal labels like "2024-25" to "2025", which can collide with existing plain "2025" labels when building labelsSet/historicalByLabel/questionByLabel; add a brief comment near YEAR_RANGE_LABEL_PATTERN and normalizeMultiQuestionLabel documenting that fiscal-year labels intentionally map to the ending calendar year, and add an optional dev-mode check (assert or console.warn) where labels are inserted into labelsSet/historicalByLabel/questionByLabel to detect and surface unexpected collisions during development.front_end/src/app/(main)/labor-hub/components/table_scroll_wrapper.tsx (1)
14-29: Optional: ResizeObserver only tracks container size, not content size.
ro.observe(el)will fire when the scroll container resizes, but not when the inner children grow/shrink (which is what changesscrollWidth). For dynamically sized content (e.g., after async data populates rows, fonts load, or a column expands),canScrollRightwon't re-evaluate. For the current server-rendered tables this is fine, but if any future consumer renders dynamic content inside, the gradient indicator will go stale.♻️ Proposed refinement
useEffect(() => { const el = ref.current; if (!el) return; const update = () => { setCanScrollLeft(el.scrollLeft > 0); setCanScrollRight(el.scrollLeft + el.clientWidth < el.scrollWidth - 1); }; update(); el.addEventListener("scroll", update, { passive: true }); const ro = new ResizeObserver(update); ro.observe(el); + if (el.firstElementChild) ro.observe(el.firstElementChild); return () => { el.removeEventListener("scroll", update); ro.disconnect(); }; }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/table_scroll_wrapper.tsx around lines 14 - 29, The ResizeObserver ro currently only observes the scroll container (ro.observe(el)), which misses changes to inner content size that affect scrollWidth; update the effect to also detect content changes by either attaching a MutationObserver (observing childList and subtree on ref.current) that calls the existing update(), or use a ResizeObserver on the inner content node(s) (e.g., ref.current.firstElementChild or each child) so that setCanScrollLeft/setCanScrollRight are recalculated when children grow/shrink; ensure you create and disconnect the MutationObserver/extra ResizeObservers in the same cleanup block so update and cleanup live alongside the existing update/removeEventListener/ro.disconnect logic.front_end/src/app/(main)/labor-hub/components/question_cards/multi_line_chart.tsx (2)
103-114: Inconsistent width heuristic between x-tick and data-label collision detection.
computeXTickLabelBoxestimates text width withtext.length * 6.5, whereas the data-label path now uses the more accurateestimateNumericLabelWidthPx(Line 430). For purely numeric year ticks (the common case here), reusingestimateNumericLabelWidthPxwould make the two collision estimators consistent and typically more accurate for narrow digits. Low priority since the heuristic is conservative.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/question_cards/multi_line_chart.tsx around lines 103 - 114, computeXTickLabelBox uses a crude width heuristic via X_TICK_AVG_CHAR_PX causing inconsistency with the data-label path; update computeXTickLabelBox to call the existing estimateNumericLabelWidthPx for numeric xValue ticks (falling back to the current text.length * X_TICK_AVG_CHAR_PX for non-numeric or when estimateNumericLabelWidthPx is not applicable) so both x-tick and data-label collision detection use the same width estimator; locate computeXTickLabelBox and the helper estimateNumericLabelWidthPx and wire them together, preserving the existing center/left/right math and the null early-return behavior.
940-965: Greedy left-to-right label retention may drop the most recent data point.The sort on Line 942 orders points by ascending
x, so when labels collide the retained one is always the earlier one. For timeseries charts readers usually care most about the latest value (right edge). Consider sorting descending (or retaining the rightmost candidate on collision), mirroring what you already do for the x-axis ticks wherelastEntryis always preserved.Proposed change
- const sortedPoints = [...entry.chartData].sort((a, b) => a.x - b.x); + const sortedPoints = [...entry.chartData].sort((a, b) => b.x - a.x);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/question_cards/multi_line_chart.tsx around lines 940 - 965, The current greedy left-to-right retention in the loop over seriesEntries sorts points ascending (sortedPoints) so earlier points win collisions; change the logic to prefer the most recent (rightmost) point by sorting sortedPoints descending by x (or iterate the ascending list in reverse) before computing computeDataLabelBox and checking dataLabelBoxesCollide, so keptBoxes/keptXs will record the latest visible x values for each series id (where result.set is called); ensure placement, formatResolvedYValue, entry.pointRadius and layout usage remain the same.front_end/src/app/(main)/labor-hub/sections/sortable_research_table.tsx (2)
383-403: Inconsistent sign/unit formatting across literature metrics.
feltenis rendered with an explicit+for positives (Line 384),mnahas no sign prefix, andaoehas a%suffix but also no sign prefix. If this is intentional (e.g., only felten is signed change while mna/aoe are non-negative magnitudes), please confirm. Otherwise consolidating the three near-identical cells into a config-driven map keyed by metric would make the format contract explicit and remove duplication:Sketch
const LITERATURE_CELLS = [ { metric: "felten", range: feltenColumnRange, decimals: feltenDecimals, showSign: true, suffix: "" }, { metric: "mna", range: mnaColumnRange, decimals: mnaDecimals, showSign: false, suffix: "" }, { metric: "aoe", range: aoeColumnRange, decimals: aoeDecimals, showSign: false, suffix: "%" }, ] as const;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/sections/sortable_research_table.tsx around lines 383 - 403, The three adjacent metric cells (felten, mna, aoe in the TableCompactCell blocks) use inconsistent sign/suffix formatting (felten prepends "+", mna has no sign, aoe appends "%"), so consolidate behavior by introducing a small config map (e.g., LITERATURE_CELLS) keyed by metric names with properties: range (mnaColumnRange/feltenColumnRange/aoeColumnRange), decimals (mnaDecimals/feltenDecimals/aoeDecimals), showSign (boolean), and suffix (string), then refactor the rendering to iterate that map and call getFullSpectrumStyle(metricValue, range, true) and render the value using toFixed(decimals) with an optional "+" when showSign is true and append suffix; update the TableCompactCell usage to use this unified render path so formatting is consistent and duplication is removed, keeping existing functions/props (getFullSpectrumStyle, TableCompactCell, felten/mna/aoe variables) as references.
101-108:getMaxDecimalscan miscount when numbers stringify in exponential form.
Number.prototype.toString()switches to scientific notation for values with magnitude < 1e-6 (e.g.,0.0000001.toString() === "1e-7"). The current logic would computelength - indexOf('.') - 1on strings like"1e-7"(no dot → returnsmaxunchanged — OK) and"1.5e-7"(dot present →s.length - dot - 1= 4, counting the exponent as decimals, which is wrong). Values in this table look like regular percentages, so it's unlikely to trigger in practice, but a defensive fix makes the helper reusable:Proposed guard
- const s = v.toString(); - const dot = s.indexOf("."); - return dot === -1 ? max : Math.max(max, s.length - dot - 1); + const s = v.toString(); + if (s.includes("e") || s.includes("E")) return max; + const dot = s.indexOf("."); + return dot === -1 ? max : Math.max(max, s.length - dot - 1);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/sections/sortable_research_table.tsx around lines 101 - 108, getMaxDecimals currently miscounts decimals for numbers rendered in exponential form; update getMaxDecimals to detect when s = v.toString() contains an 'e' and, for those values, convert the value to a non-exponential decimal string first (for example with v.toFixed(precision) using a sufficiently large precision), then trim trailing zeros and any trailing decimal point before computing the number of digits after the dot; keep the existing fast path for strings without 'e' (using dot/indexOf) and continue to skip null/NaN values so the function remains robust for regular percentages and exponential inputs.front_end/src/app/(main)/labor-hub/components/question_cards/flippable_chart_timeline_card.tsx (1)
98-100:showLegendandheightprecedence are not caller-overridable viachartProps.
chartPropsomitsshowLegendfrom the caller surface (Line 41), so callers that add anextraRowwill always get a legend and callers without extras never will — there's no escape hatch. Similarly,chartProps?.heighttakes precedence over the dedicatedchartHeightprop, which is the opposite of the typical "explicit prop beats spread" pattern and a little surprising givenchartHeightalso gets passed through toBasicQuestionContent(Line 107) but the explicitchartProps.heightdoes not. Consider droppingheightfromchartProps(omit it likeshowLegend) and relying on the dedicatedchartHeightprop for both sides, to keep one source of truth.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/question_cards/flippable_chart_timeline_card.tsx around lines 98 - 100, The chart currently lets chartProps override showLegend and height (via showLegend={(extraRows?.length ?? 0) > 0} and height={chartProps?.height ?? chartHeight ?? 250}), so callers cannot explicitly control these; update the component to treat showLegend and height as dedicated, caller-overridable props by removing/omitting showLegend and height from chartProps (mirror how showLegend was omitted from the chartProps type) and computing showLegend from extraRows (or an explicit showLegend prop) and height from chartHeight (with fallback default), and ensure BasicQuestionContent and any Chart render use the dedicated chartHeight and computed showLegend values rather than chartProps.height so explicit props always win.
🤖 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)/labor-hub/components/question_cards/flippable_chart_timeline_card.tsx:
- Line 109: The timelineSubtitle string is phrased as a rhetorical question and
should be corrected to proper grammar; update the prop value passed to
timelineSubtitle in the FlippableChartTimelineCard component (the line with
timelineSubtitle="How the forecasts have changed over time?") to either a
declarative statement "How the forecasts have changed over time" or a proper
question "How have the forecasts changed over time?" depending on intended tone;
locate the timelineSubtitle prop in flippable_chart_timeline_card.tsx and
replace the text accordingly.
- Around line 66-78: The error branch currently returns only a QuestionCard with
NoQuestionPlaceholder, dropping the caller-provided note; update the catch
branch in flippable_chart_timeline_card.tsx to render the same fragment used on
success so the note prop/block is preserved — keep the QuestionCard (with
NoQuestionPlaceholder) and then render the note element (the same JSX/variable
named note used on the success path) inside the fragment so callers always see
the note even when ServerPostsApi.getPost fails.
In
`@front_end/src/app/`(main)/labor-hub/components/question_cards/multi_question_line_chart.tsx:
- Around line 220-224: historicalLabelSet is built from raw keys of
row.historicalValues but dataset.columns and getHistoricalForecastDividerX use
normalized labels; update the construction of historicalLabelSet to import and
apply normalizeMultiQuestionLabel to each key (e.g., map
Object.keys(row.historicalValues ?? {}) through normalizeMultiQuestionLabel when
flattening rows) so keys match the normalized column labels used by
getHistoricalForecastDividerX and the divider placement works correctly; ensure
you add the necessary import for normalizeMultiQuestionLabel from
multi_question_data and keep the null/undefined guard on row.historicalValues.
In `@front_end/src/app/`(main)/labor-hub/sections/sortable_research_table.tsx:
- Around line 267-273: The header spacer cells are missing aria-hidden and
should be marked presentational like the body spacer; update the
TableCompactHeaderCell instances that render the blank sticky header and the two
header-row spacer cells (the empty TableCompactHeaderCell with className="w-4"
and the one with colSpan={3} / the other header spacer) to include
aria-hidden="true" so they do not get announced by screen readers, matching the
body spacer cell which already sets aria-hidden="true".
---
Nitpick comments:
In
`@front_end/src/app/`(main)/labor-hub/components/question_cards/basic_question.tsx:
- Around line 65-86: The subtitle div and GroupTimeline are rendered adjacently
which can produce cramped spacing; update the JSX around timelineSubtitle and
GroupTimeline (see timelineSubtitle and GroupTimeline) to ensure consistent
spacing by either adding a bottom margin class to the subtitle div (e.g.,
mb-2/mb-3) or wrapping both elements in a container with flex flex-col and a gap
class (e.g., gap-2) so the subtitle always has breathing room above the
timeline.
In
`@front_end/src/app/`(main)/labor-hub/components/question_cards/flippable_chart_timeline_card.tsx:
- Around line 98-100: The chart currently lets chartProps override showLegend
and height (via showLegend={(extraRows?.length ?? 0) > 0} and
height={chartProps?.height ?? chartHeight ?? 250}), so callers cannot explicitly
control these; update the component to treat showLegend and height as dedicated,
caller-overridable props by removing/omitting showLegend and height from
chartProps (mirror how showLegend was omitted from the chartProps type) and
computing showLegend from extraRows (or an explicit showLegend prop) and height
from chartHeight (with fallback default), and ensure BasicQuestionContent and
any Chart render use the dedicated chartHeight and computed showLegend values
rather than chartProps.height so explicit props always win.
In
`@front_end/src/app/`(main)/labor-hub/components/question_cards/multi_line_chart.tsx:
- Around line 103-114: computeXTickLabelBox uses a crude width heuristic via
X_TICK_AVG_CHAR_PX causing inconsistency with the data-label path; update
computeXTickLabelBox to call the existing estimateNumericLabelWidthPx for
numeric xValue ticks (falling back to the current text.length *
X_TICK_AVG_CHAR_PX for non-numeric or when estimateNumericLabelWidthPx is not
applicable) so both x-tick and data-label collision detection use the same width
estimator; locate computeXTickLabelBox and the helper
estimateNumericLabelWidthPx and wire them together, preserving the existing
center/left/right math and the null early-return behavior.
- Around line 940-965: The current greedy left-to-right retention in the loop
over seriesEntries sorts points ascending (sortedPoints) so earlier points win
collisions; change the logic to prefer the most recent (rightmost) point by
sorting sortedPoints descending by x (or iterate the ascending list in reverse)
before computing computeDataLabelBox and checking dataLabelBoxesCollide, so
keptBoxes/keptXs will record the latest visible x values for each series id
(where result.set is called); ensure placement, formatResolvedYValue,
entry.pointRadius and layout usage remain the same.
In
`@front_end/src/app/`(main)/labor-hub/components/question_cards/multi_question_data.ts:
- Around line 43-53: normalizeMultiQuestionLabel collapses fiscal labels like
"2024-25" to "2025", which can collide with existing plain "2025" labels when
building labelsSet/historicalByLabel/questionByLabel; add a brief comment near
YEAR_RANGE_LABEL_PATTERN and normalizeMultiQuestionLabel documenting that
fiscal-year labels intentionally map to the ending calendar year, and add an
optional dev-mode check (assert or console.warn) where labels are inserted into
labelsSet/historicalByLabel/questionByLabel to detect and surface unexpected
collisions during development.
In `@front_end/src/app/`(main)/labor-hub/components/table_scroll_wrapper.tsx:
- Around line 14-29: The ResizeObserver ro currently only observes the scroll
container (ro.observe(el)), which misses changes to inner content size that
affect scrollWidth; update the effect to also detect content changes by either
attaching a MutationObserver (observing childList and subtree on ref.current)
that calls the existing update(), or use a ResizeObserver on the inner content
node(s) (e.g., ref.current.firstElementChild or each child) so that
setCanScrollLeft/setCanScrollRight are recalculated when children grow/shrink;
ensure you create and disconnect the MutationObserver/extra ResizeObservers in
the same cleanup block so update and cleanup live alongside the existing
update/removeEventListener/ro.disconnect logic.
In `@front_end/src/app/`(main)/labor-hub/sections/sortable_research_table.tsx:
- Around line 383-403: The three adjacent metric cells (felten, mna, aoe in the
TableCompactCell blocks) use inconsistent sign/suffix formatting (felten
prepends "+", mna has no sign, aoe appends "%"), so consolidate behavior by
introducing a small config map (e.g., LITERATURE_CELLS) keyed by metric names
with properties: range (mnaColumnRange/feltenColumnRange/aoeColumnRange),
decimals (mnaDecimals/feltenDecimals/aoeDecimals), showSign (boolean), and
suffix (string), then refactor the rendering to iterate that map and call
getFullSpectrumStyle(metricValue, range, true) and render the value using
toFixed(decimals) with an optional "+" when showSign is true and append suffix;
update the TableCompactCell usage to use this unified render path so formatting
is consistent and duplication is removed, keeping existing functions/props
(getFullSpectrumStyle, TableCompactCell, felten/mna/aoe variables) as
references.
- Around line 101-108: getMaxDecimals currently miscounts decimals for numbers
rendered in exponential form; update getMaxDecimals to detect when s =
v.toString() contains an 'e' and, for those values, convert the value to a
non-exponential decimal string first (for example with v.toFixed(precision)
using a sufficiently large precision), then trim trailing zeros and any trailing
decimal point before computing the number of digits after the dot; keep the
existing fast path for strings without 'e' (using dot/indexOf) and continue to
skip null/NaN values so the function remains robust for regular percentages and
exponential inputs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6c13771c-e25c-4bf7-afa5-c1f9359dae29
📒 Files selected for processing (21)
front_end/src/app/(main)/labor-hub/components/activity_card.tsxfront_end/src/app/(main)/labor-hub/components/mobile_carousel.tsxfront_end/src/app/(main)/labor-hub/components/question_cards/basic_question.tsxfront_end/src/app/(main)/labor-hub/components/question_cards/chart_core/multi_line_chart_model.tsfront_end/src/app/(main)/labor-hub/components/question_cards/flippable_chart_timeline_card.tsxfront_end/src/app/(main)/labor-hub/components/question_cards/flippable_question_card.tsxfront_end/src/app/(main)/labor-hub/components/question_cards/multi_line_chart.tsxfront_end/src/app/(main)/labor-hub/components/question_cards/multi_question_data.tsfront_end/src/app/(main)/labor-hub/components/question_cards/multi_question_line_chart.tsxfront_end/src/app/(main)/labor-hub/components/question_cards/multi_question_table.tsxfront_end/src/app/(main)/labor-hub/components/question_cards/question.tsxfront_end/src/app/(main)/labor-hub/components/question_cards/question_card.tsxfront_end/src/app/(main)/labor-hub/components/table_compact.tsxfront_end/src/app/(main)/labor-hub/components/table_scroll_wrapper.tsxfront_end/src/app/(main)/labor-hub/data.tsfront_end/src/app/(main)/labor-hub/page.tsxfront_end/src/app/(main)/labor-hub/sections/jobs_monitor.tsxfront_end/src/app/(main)/labor-hub/sections/methodology.tsxfront_end/src/app/(main)/labor-hub/sections/research.tsxfront_end/src/app/(main)/labor-hub/sections/sortable_research_table.tsxfront_end/src/components/ui/section_toggle.tsx
| } catch { | ||
| return ( | ||
| <QuestionCard | ||
| title={title || fallbackTitle} | ||
| subtitle={subtitle} | ||
| variant={variant} | ||
| className={className} | ||
| postIds={[questionId]} | ||
| > | ||
| <NoQuestionPlaceholder /> | ||
| </QuestionCard> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Error fallback drops the note block.
When ServerPostsApi.getPost fails, only the QuestionCard is rendered, so a caller-provided note (rendered on the success path at Line 121-125) silently disappears. If the note is context users need regardless of data availability, consider rendering it in the error branch too (wrap in the same <> fragment).
Proposed fix
} catch {
return (
+ <>
<QuestionCard
title={title || fallbackTitle}
subtitle={subtitle}
variant={variant}
className={className}
postIds={[questionId]}
>
<NoQuestionPlaceholder />
</QuestionCard>
+ {note && (
+ <div className="!mt-2 text-sm text-blue-700 dark:text-blue-700-dark">
+ {note}
+ </div>
+ )}
+ </>
);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@front_end/src/app/`(main)/labor-hub/components/question_cards/flippable_chart_timeline_card.tsx
around lines 66 - 78, The error branch currently returns only a QuestionCard
with NoQuestionPlaceholder, dropping the caller-provided note; update the catch
branch in flippable_chart_timeline_card.tsx to render the same fragment used on
success so the note prop/block is preserved — keep the QuestionCard (with
NoQuestionPlaceholder) and then render the note element (the same JSX/variable
named note used on the success path) inside the fragment so callers always see
the note even when ServerPostsApi.getPost fails.
| preferTimeline | ||
| chartHeight={chartHeight} | ||
| timelineMarkers={timelineMarkers} | ||
| timelineSubtitle="How the forecasts have changed over time?" |
There was a problem hiding this comment.
Timeline subtitle reads as a rhetorical question.
"How the forecasts have changed over time?" is phrased as a question but isn't one — it's either a statement ("How the forecasts have changed over time") or an actual question ("How have the forecasts changed over time?"). Minor copy nit.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@front_end/src/app/`(main)/labor-hub/components/question_cards/flippable_chart_timeline_card.tsx
at line 109, The timelineSubtitle string is phrased as a rhetorical question and
should be corrected to proper grammar; update the prop value passed to
timelineSubtitle in the FlippableChartTimelineCard component (the line with
timelineSubtitle="How the forecasts have changed over time?") to either a
declarative statement "How the forecasts have changed over time" or a proper
question "How have the forecasts changed over time?" depending on intended tone;
locate the timelineSubtitle prop in flippable_chart_timeline_card.tsx and
replace the text accordingly.
| const historicalLabelSet = new Set( | ||
| rows.flatMap((row) => Object.keys(row.historicalValues ?? {})) | ||
| rows.flatMap((row) => | ||
| row.questionId != null ? Object.keys(row.historicalValues ?? {}) : [] | ||
| ) | ||
| ); |
There was a problem hiding this comment.
Minor/latent: historicalLabelSet skips normalizeMultiQuestionLabel.
dataset.columns are built from normalized labels in multi_question_data.ts (fiscal-year style "2024-25" → "2025"), but here Object.keys(row.historicalValues ?? {}) is taken raw. getHistoricalForecastDividerX then calls historicalLabelSet.has(label) where label comes from the normalized columns, so fiscal-keyed historicals would never match and the divider would be mis-placed (or absent).
Not currently triggered by callers in this PR (all historical keys are plain years), but easy to make consistent:
🛡️ Proposed fix
- const historicalLabelSet = new Set(
- rows.flatMap((row) =>
- row.questionId != null ? Object.keys(row.historicalValues ?? {}) : []
- )
- );
+ const historicalLabelSet = new Set(
+ rows.flatMap((row) =>
+ row.questionId != null
+ ? Object.keys(row.historicalValues ?? {}).map(
+ normalizeMultiQuestionLabel
+ )
+ : []
+ )
+ );(requires importing normalizeMultiQuestionLabel from ./multi_question_data)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@front_end/src/app/`(main)/labor-hub/components/question_cards/multi_question_line_chart.tsx
around lines 220 - 224, historicalLabelSet is built from raw keys of
row.historicalValues but dataset.columns and getHistoricalForecastDividerX use
normalized labels; update the construction of historicalLabelSet to import and
apply normalizeMultiQuestionLabel to each key (e.g., map
Object.keys(row.historicalValues ?? {}) through normalizeMultiQuestionLabel when
flattening rows) so keys match the normalized column labels used by
getHistoricalForecastDividerX and the divider placement works correctly; ensure
you add the necessary import for normalizeMultiQuestionLabel from
multi_question_data and keep the null/undefined guard on row.historicalValues.
Cleanup: Preview Environment RemovedThe preview environment for this PR has been destroyed.
Cleanup triggered by PR close at 2026-04-17T17:11:26Z |
1 participant
Summary by CodeRabbit
Release Notes
New Features
Improvements