labor hub hot fixes for launch#4633
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 51 minutes and 43 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 (1)
📝 WalkthroughWalkthroughAdded new helper functions to extract and compute job forecast data by year: Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 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)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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: 3
🧹 Nitpick comments (1)
front_end/src/app/(main)/labor-hub/sections/key_insights.tsx (1)
69-84: All fetches share failure: a single reject discards successful data too.
Promise.allrejects if any of the three fetches fails, so a transient failure infetchJobsDatawill also dropoverall2035andinsightsData, forcing the hard-coded fallback for unrelated copy (hours-worked, youth unemployment, etc.). ConsiderPromise.allSettled(or separate try/catch per fetch) so each insight degrades independently.♻️ Proposed change
- try { - const [overallData, fetchedInsights, { jobs }] = await Promise.all([ - fetchOverallData(), - fetchKeyInsightsData(), - fetchJobsData(), - ]); - overall2035 = overallData.find((d) => d.year === 2035)?.value ?? null; - insightsData = fetchedInsights; - const extremes = getTopExtremeJobsForYear(jobs, "2035"); - mostVulnerable2035 = extremes.mostVulnerable; - leastVulnerable2035 = extremes.leastVulnerable; - } catch (error) { - logError(error); - } + const [overallRes, insightsRes, jobsRes] = await Promise.allSettled([ + fetchOverallData(), + fetchKeyInsightsData(), + fetchJobsData(), + ]); + if (overallRes.status === "fulfilled") { + overall2035 = + overallRes.value.find((d) => d.year === 2035)?.value ?? null; + } else logError(overallRes.reason); + if (insightsRes.status === "fulfilled") { + insightsData = insightsRes.value; + } else logError(insightsRes.reason); + if (jobsRes.status === "fulfilled") { + const extremes = getTopExtremeJobsForYear(jobsRes.value.jobs, "2035"); + mostVulnerable2035 = extremes.mostVulnerable; + leastVulnerable2035 = extremes.leastVulnerable; + } else logError(jobsRes.reason);🤖 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/key_insights.tsx around lines 69 - 84, The current Promise.all of the three fetches causes any single failure to reject them all; update the code that calls Promise.all for fetchJobsData, fetchOverall2035 (or fetchOverall2035Data), and fetchInsightsData to use Promise.allSettled (or perform each fetch in its own try/catch) so each result is handled independently: map settled results to success values or fallbacks and only replace the specific insight with its fallback when that specific fetch failed, leaving successful fetches intact.
🤖 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/helpers/fetch_jobs_data.ts:
- Around line 53-81: The function getTopExtremeJobsForYear should normalize the
incoming limit before using it: validate that limit is finite, coerce to a
non-negative integer (e.g., Math.floor) and clamp it to at most the number of
available rows (rows.length) so slice(0, k) and the leastVulnerable loop can't
misbehave for negative, fractional, NaN or Infinity inputs; update the code that
computes k (and any uses of limit later) to use this sanitized value rather than
the raw limit parameter.
In `@front_end/src/app/`(main)/labor-hub/sections/key_insights.tsx:
- Around line 120-133: getTopExtremeJobsForYear currently returns the top N jobs
by value regardless of sign, so the UI sentence using leastVulnerable2035
assumes growth; update the rendering logic that builds the "... while
{leastVulnerableText} are projected to grow" phrase to first check whether all
entries in leastVulnerable2035 (from getTopExtremeJobsForYear) have positive
values (e.g., leastVulnerable2035.every(j => j.value > 0)) or use the same
sign/threshold logic used by the chart, and choose alternate phrasing (e.g.,
"are projected to decline the least" or neutral phrasing) when the condition is
false so the sentence remains factually correct.
- Around line 51-57: The plural wording branch is applied even when one of the
job lists has only one item; update the conditional that chooses the hard-coded
paragraph to require both job lists to have at least two items (e.g., check
topDeclineJobs.length >= 2 && topGrowJobs.length >= 2 or use the formatted
lists' lengths) before rendering the "are all expected..." sentence; if either
list has <2 items, fall back to the existing singular-aware branches (the same
change should be applied to the second block around lines 120-133). Ensure you
reference formatJobList and the topDeclineJobs/topGrowJobs (or their local
variable names) when making the check.
---
Nitpick comments:
In `@front_end/src/app/`(main)/labor-hub/sections/key_insights.tsx:
- Around line 69-84: The current Promise.all of the three fetches causes any
single failure to reject them all; update the code that calls Promise.all for
fetchJobsData, fetchOverall2035 (or fetchOverall2035Data), and fetchInsightsData
to use Promise.allSettled (or perform each fetch in its own try/catch) so each
result is handled independently: map settled results to success values or
fallbacks and only replace the specific insight with its fallback when that
specific fetch failed, leaving successful fetches intact.
🪄 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: 6d16b6e5-2f58-4e1d-891e-35d06d9ef1dc
📒 Files selected for processing (4)
front_end/src/app/(main)/labor-hub/helpers/fetch_jobs_data.tsfront_end/src/app/(main)/labor-hub/sections/key_insights.tsxfront_end/src/app/(main)/labor-hub/sections/overview.tsxfront_end/src/app/(main)/labor-hub/sections/research.tsx
| export function getTopExtremeJobsForYear( | ||
| jobs: JobWithPost[], | ||
| yearLabel: string, | ||
| limit: number = VULNERABILITY_CHIP_JOB_LIMIT | ||
| ): { mostVulnerable: JobForecast[]; leastVulnerable: JobForecast[] } { | ||
| const rows: JobForecast[] = []; | ||
| for (const job of jobs) { | ||
| const v = getJobValueForYear(job, yearLabel); | ||
| if (v == null) continue; | ||
| rows.push({ name: job.name, value: v }); | ||
| } | ||
|
|
||
| if (!rows.length) { | ||
| return { mostVulnerable: [], leastVulnerable: [] }; | ||
| } | ||
|
|
||
| const sorted = [...rows].sort((a, b) => a.value - b.value); | ||
| const n = sorted.length; | ||
| const k = Math.min(limit, n); | ||
| const mostVulnerable = sorted.slice(0, k); | ||
| const mostNames = new Set(mostVulnerable.map((j) => j.name)); | ||
| const leastVulnerable: JobForecast[] = []; | ||
| for (let i = n - 1; i >= 0 && leastVulnerable.length < limit; i--) { | ||
| const j = sorted[i]; | ||
| if (j == null) continue; | ||
| if (!mostNames.has(j.name)) leastVulnerable.push(j); | ||
| } | ||
|
|
||
| return { mostVulnerable, leastVulnerable }; |
There was a problem hiding this comment.
Normalize limit before slicing.
limit is typed as number, so callers can still pass negative, fractional, or non-finite values at runtime. For example, limit = -1 makes slice(0, -1) return nearly every row as “most vulnerable.” Clamp it to a non-negative integer before using it.
Proposed fix
export function getTopExtremeJobsForYear(
jobs: JobWithPost[],
yearLabel: string,
limit: number = VULNERABILITY_CHIP_JOB_LIMIT
): { mostVulnerable: JobForecast[]; leastVulnerable: JobForecast[] } {
+ const safeLimit = Number.isFinite(limit)
+ ? Math.max(0, Math.floor(limit))
+ : VULNERABILITY_CHIP_JOB_LIMIT;
const rows: JobForecast[] = [];
for (const job of jobs) {
const v = getJobValueForYear(job, yearLabel);
if (v == null) continue;
rows.push({ name: job.name, value: v });
@@
const sorted = [...rows].sort((a, b) => a.value - b.value);
const n = sorted.length;
- const k = Math.min(limit, n);
+ const k = Math.min(safeLimit, n);
const mostVulnerable = sorted.slice(0, k);
const mostNames = new Set(mostVulnerable.map((j) => j.name));
const leastVulnerable: JobForecast[] = [];
- for (let i = n - 1; i >= 0 && leastVulnerable.length < limit; i--) {
+ for (let i = n - 1; i >= 0 && leastVulnerable.length < safeLimit; i--) {🤖 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/helpers/fetch_jobs_data.ts around lines
53 - 81, The function getTopExtremeJobsForYear should normalize the incoming
limit before using it: validate that limit is finite, coerce to a non-negative
integer (e.g., Math.floor) and clamp it to at most the number of available rows
(rows.length) so slice(0, k) and the leastVulnerable loop can't misbehave for
negative, fractional, NaN or Infinity inputs; update the code that computes k
(and any uses of limit later) to use this sanitized value rather than the raw
limit parameter.
| function formatJobList(jobs: JobForecast[]): string | null { | ||
| const names = jobs.map((j) => j.name); | ||
| if (names.length === 0) return null; | ||
| if (names.length === 1) return names[0] ?? null; | ||
| if (names.length === 2) return `${names[0]} and ${names[1]}`; | ||
| return `${names.slice(0, -1).join(", ")}, and ${names[names.length - 1]}`; | ||
| } |
There was a problem hiding this comment.
Grammar breaks for single-item lists.
formatJobList can return a single name (length === 1), but the surrounding copy reads "{X} are all expected to see the largest decreases ... while {Y} are projected to grow." With one job, this becomes "Software developers are all expected..." — plural/"all" doesn't agree with a singular subject. Given VULNERABILITY_CHIP_JOB_LIMIT almost always yields ≥3 items this is largely defensive, but since you wrote explicit 0/1/2 branches it's worth making the fallback condition stricter (e.g., require both lists to have ≥2 items) so the hard-coded paragraph is used in degenerate cases.
- const mostVulnerableText = formatJobList(mostVulnerable2035);
- const leastVulnerableText = formatJobList(leastVulnerable2035);
+ const mostVulnerableText =
+ mostVulnerable2035.length >= 2 ? formatJobList(mostVulnerable2035) : null;
+ const leastVulnerableText =
+ leastVulnerable2035.length >= 2 ? formatJobList(leastVulnerable2035) : null;Also applies to: 120-133
🤖 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/key_insights.tsx around lines 51
- 57, The plural wording branch is applied even when one of the job lists has
only one item; update the conditional that chooses the hard-coded paragraph to
require both job lists to have at least two items (e.g., check
topDeclineJobs.length >= 2 && topGrowJobs.length >= 2 or use the formatted
lists' lengths) before rendering the "are all expected..." sentence; if either
list has <2 items, fall back to the existing singular-aware branches (the same
change should be applied to the second block around lines 120-133). Ensure you
reference formatJobList and the topDeclineJobs/topGrowJobs (or their local
variable names) when making the check.
| {mostVulnerableText && leastVulnerableText ? ( | ||
| <> | ||
| {mostVulnerableText} are all expected to see the largest decreases | ||
| in employment rates, while {leastVulnerableText} are projected to | ||
| grow. | ||
| </> | ||
| ) : ( | ||
| <> | ||
| Software developers, lawyers and law clerks, and laborers and | ||
| material movers are all expected to see the largest decreases in | ||
| employment rates, while registered nurses, K-12 teachers, and | ||
| restaurant servers are projected to grow. | ||
| </> | ||
| )} |
There was a problem hiding this comment.
Dynamic "least vulnerable" copy asserts growth without checking sign.
getTopExtremeJobsForYear returns the limit highest-valued jobs regardless of whether their forecast values are positive. If the overall outlook worsens and even the top-ranked jobs have values ≤ 0, the rendered sentence "... while {leastVulnerableText} are projected to grow" will be factually wrong. Consider guarding the phrasing on leastVulnerable2035.every(j => j.value > 0) (or pulling the threshold from the same logic that powers the chart) before using the "projected to grow" wording.
🤖 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/key_insights.tsx around lines
120 - 133, getTopExtremeJobsForYear currently returns the top N jobs by value
regardless of sign, so the UI sentence using leastVulnerable2035 assumes growth;
update the rendering logic that builds the "... while {leastVulnerableText} are
projected to grow" phrase to first check whether all entries in
leastVulnerable2035 (from getTopExtremeJobsForYear) have positive values (e.g.,
leastVulnerable2035.every(j => j.value > 0)) or use the same sign/threshold
logic used by the chart, and choose alternate phrasing (e.g., "are projected to
decline the least" or neutral phrasing) when the condition is false so the
sentence remains factually correct.
1 participant
Summary by CodeRabbit
New Features
Documentation
Style