[codex] Refresh Frontier AI Governance lander#2241
Conversation
📝 WalkthroughWalkthroughThis pull request extends the course landing page system with flexible section placement and ordering capabilities. The Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
render
Bot
temporarily deployed
to
codex/faigc-lander-updates - bluedot-preview PR #2241
|
@claude review please |
|
Claude finished @somepersonnamedjosh's task —— View job Code Review: Frontier AI Governance lander updatesTodo List:
Overall AssessmentThis is a well-structured PR that refreshes the Frontier AI Governance course lander with improved content organization and new functionality. The changes are primarily content updates and component enhancements, with good attention to TypeScript typing and React best practices. ✅ StrengthsNew FieldBuildingSection Component
Enhanced Component Flexibility
Content Updates
|
Greptile SummaryThis PR refreshes the Frontier AI Governance course lander with updated copy, reordered sections, and unit-linked curriculum cards. It also introduces a new
Confidence Score: 4/5Safe to merge; changes are content-heavy with well-scoped, low-risk layout refactoring The placement-override logic uses mutually exclusive boolean checks that are straightforward to reason about, and existing snapshot tests confirm shared components render correctly. Score is 4 rather than 5 because the new FieldBuildingSection component and the modified CourseOutcomesSection link-rendering path have no automated test coverage. apps/website/src/components/lander/components/FieldBuildingSection.tsx and apps/website/src/components/lander/components/CourseOutcomesSection.tsx lack test coverage Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[CourseLander entry] --> B[HeroSection]
B --> C{alumniLogos?}
C -->|yes| D[AlumniLogosSection]
C -->|no| E[GraduateSection]
D & E --> F{personas?}
F -->|yes| G[PersonasSection]
F -->|no| H[WhoIsThisForSection?]
G & H --> J{testimonialsPlacement\n=== 'beforeOutcomes'?}
J -->|yes| K[TestimonialCarousel — early]
J -->|no| L[skip]
K & L --> M{courseOutcomes AND\nplacement != 'beforeStructure'?}
M -->|yes| N[CourseOutcomesSection — default]
M -->|no| O[skip]
N & O --> P[PathwaysSection?]
P --> Q[CourseBenefitsSection?]
Q --> R[PrerequisitesSection?]
R --> S[AlumniStoryCarousel?]
S --> T[CaseStudiesSection?]
T --> U{courseOutcomes AND\nplacement === 'beforeStructure'?}
U -->|yes| V[CourseOutcomesSection — late]
U -->|no| W[skip]
V & W --> X[CourseInformationSection?]
X --> Y{testimonialsPlacement\n!== 'beforeOutcomes'?}
Y -->|yes| Z[TestimonialCarousel — default]
Y -->|no| AA[skip]
Z & AA --> AB[QuoteSection?]
AB --> AC[PartnerSection?]
AC --> AD[FieldBuildingSection? ← NEW]
AD --> AE[FAQSection?]
AE --> AF[LandingBanner]
Reviews (1): Last reviewed commit: "feat(website): refresh frontier ai gover..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const FieldBuildingSection = ({ | ||
| id, | ||
| title, | ||
| intro, | ||
| roles, | ||
| }: FieldBuildingSectionProps) => { | ||
| return ( | ||
| <section id={id} className="w-full bg-white"> | ||
| <div className="max-w-max-width mx-auto px-5 min-[680px]:px-8 min-[1024px]:px-12 min-[1280px]:px-44 xl:px-40 py-12 min-[680px]:py-16 min-[1280px]:py-24 xl:py-24 flex flex-col items-center gap-8 md:gap-10"> | ||
| <div className="max-w-[840px] text-center"> | ||
| <H2 className="text-[28px] min-[680px]:text-[32px] xl:text-[36px] font-semibold leading-[125%] text-bluedot-navy tracking-[-0.01em]"> | ||
| {title} | ||
| </H2> | ||
| <P className="mt-4 text-[16px] min-[680px]:text-[17px] leading-[1.6] text-bluedot-navy/70"> | ||
| {intro} | ||
| </P> | ||
| </div> | ||
|
|
||
| <div className="w-[calc(100vw-40px)] min-[680px]:w-[calc(100vw-64px)] lg:w-[928px] xl:w-[1120px] bg-white border border-bluedot-navy/10 rounded-xl flex flex-col overflow-hidden"> | ||
| {roles.map((role, index) => ( | ||
| <div key={role.title}> | ||
| <div className="flex flex-col md:flex-row gap-3 md:gap-8 px-5 md:px-8 py-5 md:py-6"> | ||
| <div className="md:w-[160px] shrink-0"> | ||
| <P className="text-[16px] font-semibold leading-[1.3] text-bluedot-navy"> | ||
| {role.title} | ||
| </P> | ||
| </div> | ||
|
|
||
| <div className="flex-1 min-w-0"> | ||
| <P className="text-[15px] leading-[1.65] text-bluedot-navy/80"> | ||
| {role.description} | ||
| </P> | ||
| <Link | ||
| href={role.linkUrl} | ||
| className="mt-3 inline-flex items-center gap-1 text-[15px] font-medium text-bluedot-normal hover:underline" | ||
| > | ||
| {role.linkText} | ||
| <span aria-hidden="true">→</span> | ||
| </Link> | ||
| </div> | ||
| </div> | ||
|
|
||
| {index < roles.length - 1 && <div className="w-full h-px bg-bluedot-navy/10" />} | ||
| </div> | ||
| ))} | ||
| </div> | ||
| </div> | ||
| </section> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
The new FieldBuildingSection component has no corresponding test file. Every other component in this directory (CourseBenefitsSection, PersonasSection, WhoIsThisForSection, etc.) has a dedicated .test.tsx. Consider adding a FieldBuildingSection.test.tsx covering at least a snapshot render and the divider logic (index < roles.length - 1).
Rule Used: Consider adding tests for any new functionality in... (source)
Learnt From
bluedotimpact/bluedot#956
bluedotimpact/bluedot#969
bluedotimpact/bluedot#958
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| const FieldBuildingSection = ({ | ||
| id, | ||
| title, | ||
| intro, | ||
| roles, | ||
| }: FieldBuildingSectionProps) => { | ||
| return ( | ||
| <section id={id} className="w-full bg-white"> | ||
| <div className="max-w-max-width mx-auto px-5 min-[680px]:px-8 min-[1024px]:px-12 min-[1280px]:px-44 xl:px-40 py-12 min-[680px]:py-16 min-[1280px]:py-24 xl:py-24 flex flex-col items-center gap-8 md:gap-10"> | ||
| <div className="max-w-[840px] text-center"> | ||
| <H2 className="text-[28px] min-[680px]:text-[32px] xl:text-[36px] font-semibold leading-[125%] text-bluedot-navy tracking-[-0.01em]"> | ||
| {title} | ||
| </H2> | ||
| <P className="mt-4 text-[16px] min-[680px]:text-[17px] leading-[1.6] text-bluedot-navy/70"> | ||
| {intro} | ||
| </P> | ||
| </div> | ||
|
|
||
| <div className="w-[calc(100vw-40px)] min-[680px]:w-[calc(100vw-64px)] lg:w-[928px] xl:w-[1120px] bg-white border border-bluedot-navy/10 rounded-xl flex flex-col overflow-hidden"> | ||
| {roles.map((role, index) => ( | ||
| <div key={role.title}> | ||
| <div className="flex flex-col md:flex-row gap-3 md:gap-8 px-5 md:px-8 py-5 md:py-6"> | ||
| <div className="md:w-[160px] shrink-0"> | ||
| <P className="text-[16px] font-semibold leading-[1.3] text-bluedot-navy"> | ||
| {role.title} | ||
| </P> | ||
| </div> | ||
|
|
||
| <div className="flex-1 min-w-0"> | ||
| <P className="text-[15px] leading-[1.65] text-bluedot-navy/80"> | ||
| {role.description} | ||
| </P> | ||
| <Link | ||
| href={role.linkUrl} | ||
| className="mt-3 inline-flex items-center gap-1 text-[15px] font-medium text-bluedot-normal hover:underline" | ||
| > | ||
| {role.linkText} | ||
| <span aria-hidden="true">→</span> | ||
| </Link> | ||
| </div> | ||
| </div> | ||
|
|
||
| {index < roles.length - 1 && <div className="w-full h-px bg-bluedot-navy/10" />} | ||
| </div> | ||
| ))} | ||
| </div> | ||
| </div> | ||
| </section> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
All other lander sub-components (CourseBenefitsSection, FAQSection, HeroSection, WhoIsThisForSection, etc.) have a .stories.tsx file. A FieldBuildingSection.stories.tsx would make the component discoverable in Storybook and allow visual verification of the stacked-role layout.
Rule Used: Consider adding Storybook stories for new componen... (source)
Learnt From
bluedotimpact/bluedot#956
bluedotimpact/bluedot#969
bluedotimpact/bluedot#958
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| export type CourseOutcome = { | ||
| icon: IconType; | ||
| title: string; | ||
| description: ReactNode; | ||
| linkUrl?: string; | ||
| linkText?: string; |
There was a problem hiding this comment.
CourseOutcomesSection was modified in this PR to add optional linkUrl and linkText fields to CourseOutcome and conditionally render a per-card navigation link. There is currently no CourseOutcomesSection.test.tsx, so neither the with-link nor the without-link rendering path is covered. Consider adding tests for both variants.
Rule Used: Consider adding tests for any new functionality in... (source)
Learnt From
bluedotimpact/bluedot#956
bluedotimpact/bluedot#969
bluedotimpact/bluedot#958
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/website/src/components/lander/components/FieldBuildingSection.tsx (1)
37-38: Consider a more robust key for role items.Using
role.titleas the React key assumes titles are unique. If two roles share the same title, React will encounter duplicate keys. Consider using a combination of title and index, or adding anidfield toFieldBuildingRole.💡 Alternative key approach
- {roles.map((role, index) => ( - <div key={role.title}> + {roles.map((role, index) => ( + <div key={`${role.title}-${index}`}>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/website/src/components/lander/components/FieldBuildingSection.tsx` around lines 37 - 38, The current map in FieldBuildingSection uses role.title as the React key which can collide; update the key to be robust by using a stable unique identifier (e.g., add an id to the FieldBuildingRole type and use role.id) or combine title and index (e.g., `${role.title}-${index}`) in the roles.map callback; locate the roles.map(...) block and change the key prop on the enclosing <div key=...> accordingly, and if choosing an id approach, add the id to FieldBuildingRole and populate it where roles are created.apps/website/src/components/lander/components/PathwaysSection.tsx (1)
44-47: Consider semantic markup for the intro content.The
introchanged from<P>to a plain<div>to accommodateReactNode. This is functionally correct, but if callers typically pass plain text, the text won't have paragraph semantics. Consider whether consumers should wrap their content in<P>or if this component should provide guidance via JSDoc.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/website/src/components/lander/components/PathwaysSection.tsx` around lines 44 - 47, PathwaysSection currently renders the intro prop inside a bare <div>, losing paragraph semantics for plain text; update the component to preserve semantics by rendering a <p> (or wrapping the content in a <p>) when intro is a string (e.g., if (typeof intro === 'string') return <p className="...">{intro}</p>), otherwise render the ReactNode as-is, and add a brief JSDoc on the intro prop describing that plain text will be wrapped in a paragraph while consumers can pass custom nodes for full control.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/website/src/components/lander/components/FieldBuildingSection.tsx`:
- Around line 37-38: The current map in FieldBuildingSection uses role.title as
the React key which can collide; update the key to be robust by using a stable
unique identifier (e.g., add an id to the FieldBuildingRole type and use
role.id) or combine title and index (e.g., `${role.title}-${index}`) in the
roles.map callback; locate the roles.map(...) block and change the key prop on
the enclosing <div key=...> accordingly, and if choosing an id approach, add the
id to FieldBuildingRole and populate it where roles are created.
In `@apps/website/src/components/lander/components/PathwaysSection.tsx`:
- Around line 44-47: PathwaysSection currently renders the intro prop inside a
bare <div>, losing paragraph semantics for plain text; update the component to
preserve semantics by rendering a <p> (or wrapping the content in a <p>) when
intro is a string (e.g., if (typeof intro === 'string') return <p
className="...">{intro}</p>), otherwise render the ReactNode as-is, and add a
brief JSDoc on the intro prop describing that plain text will be wrapped in a
paragraph while consumers can pass custom nodes for full control.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d1af88a9-ea41-4185-9f3a-fe5528d5c70c
⛔ Files ignored due to path filters (3)
apps/website/src/components/homepage/__snapshots__/HomeHeroContent.test.tsx.snapis excluded by!**/*.snapapps/website/src/components/lander/__snapshots__/AgiStrategyLander.test.tsx.snapis excluded by!**/*.snapapps/website/src/components/lander/__snapshots__/FutureOfAiLander.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (6)
apps/website/src/components/lander/CourseLander.tsxapps/website/src/components/lander/components/CourseOutcomesSection.tsxapps/website/src/components/lander/components/FieldBuildingSection.tsxapps/website/src/components/lander/components/GraduateSection.tsxapps/website/src/components/lander/components/PathwaysSection.tsxapps/website/src/components/lander/course-content/AiGovernanceContent.tsx
1 participant
Summary
Validation
cd apps/website && npm testnpm audit --audit-level=high --omit=devSecurity notes
npm auditreports existing vulnerabilities in the repo dependency tree, including critical/high advisories infast-xml-parser,happy-dom,next,@trpc/server,axios, and parts of the AWS SDK treeNotes
gh auth statusis currently invalid in the local environment, so the PR was opened through the GitHub connector instead of the CLI