feat(ai-media): add ai-media Next.js app and staged subtitles workflow using Mux AI#8698
feat(ai-media): add ai-media Next.js app and staged subtitles workflow using Mux AI#8698
Conversation
WalkthroughAdds a new Next.js app "ai-media" with full scaffolding: configs, global styles, client UI for coverage reports and language selection, an API route for languages, Mux AI type declarations, a workflow devkit and a three-stage subtitles workflow (generate → post-process → upload), scripts, tests, and docs. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant HomePage
participant SubtitlesWorkflow
participant GenerateStage
participant PostProcessStage
participant UploadStage
participant MuxAI as "@mux/ai/primitives"
User->>HomePage: trigger runSubtitlesWorkflow(assetId, playbackId, language)
HomePage->>SubtitlesWorkflow: runSubtitlesWorkflow(input)
SubtitlesWorkflow->>GenerateStage: run generateMuxSubtitles
GenerateStage->>MuxAI: generateSubtitles(asset, language, format)
MuxAI-->>GenerateStage: SubtitleFile (generated)
GenerateStage-->>SubtitlesWorkflow: generated SubtitleFile
SubtitlesWorkflow->>PostProcessStage: run postProcessSubtitles
PostProcessStage->>MuxAI: improveSubtitles(subtitles, instructions, model)
MuxAI-->>PostProcessStage: SubtitleFile (improved)
PostProcessStage-->>SubtitlesWorkflow: improved SubtitleFile
SubtitlesWorkflow->>UploadStage: run uploadMuxSubtitles
UploadStage->>MuxAI: uploadSubtitles(asset, subtitles)
MuxAI-->>UploadStage: UploadSubtitlesResult
UploadStage-->>SubtitlesWorkflow: UploadSubtitlesResult
SubtitlesWorkflow-->>HomePage: SubtitlesWorkflowResult
HomePage-->>User: render result/status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
🚥 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 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 |
|
View your CI Pipeline Execution ↗ for commit 1e5fdf2
☁️ Nx Cloud last updated this comment at |
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/ai-media/tsconfig.json`:
- Around line 22-33: The tsconfig include array contains a duplicated pattern
".next/types/**/*.ts" which is redundant; open the tsconfig.json and remove the
second occurrence of ".next/types/**/*.ts" from the "include" array so each glob
is listed only once (look for the "include" array entry and the duplicate
".next/types/**/*.ts" string).
🧹 Nitpick comments (7)
apps/ai-media/src/app/layout.tsx (1)
1-14: Typemetadataand extract aRootLayoutPropsalias.This strengthens typing and aligns with the codebase conventions and Next.js best practices.
♻️ Suggested refactor
-import type { ReactNode } from 'react' +import type { Metadata } from 'next' +import type { ReactNode } from 'react' -export const metadata = { +export const metadata: Metadata = { title: 'AI Media — Subtitles Workflow', description: 'Mux AI subtitles workflow stages and development checklist.' } -export default function RootLayout({ - children -}: { - children: ReactNode -}) { +type RootLayoutProps = { + children: ReactNode +} + +export default function RootLayout({ children }: RootLayoutProps) {As per coding guidelines, define a type if possible. Other layout files in the codebase (e.g.,
apps/player/src/app/layout.tsx,apps/short-links/src/app/[hostname]/[...pathname]/not-found.tsx) follow this pattern with theMetadatatype from Next.js.apps/ai-media/src/workflows/subtitles/__tests__/subtitlesWorkflow.spec.ts (2)
24-39: Add mock reset for test isolation.Consider adding
jest.clearAllMocks()inbeforeEachto ensure call counts and arguments are reset between tests. This prevents state leakage if additional test cases are added later.♻️ Proposed fix
describe('subtitles workflow', () => { beforeEach(() => { + jest.clearAllMocks() mockGenerateSubtitles.mockResolvedValue({ language: 'en',
41-52: Consider adding argument verification to strengthen the test.The test verifies each mock was called once, but doesn't validate the arguments passed between stages. Verifying that
improveSubtitlesreceives the output fromgenerateSubtitles, anduploadSubtitlesreceives the output fromimproveSubtitles, would ensure the workflow correctly chains stage outputs.💡 Example assertions
expect(mockImproveSubtitles).toHaveBeenCalledWith( expect.objectContaining({ subtitles: expect.objectContaining({ language: 'en', format: 'vtt' }) }) )apps/ai-media/src/app/page.tsx (1)
3-47: Consider using MUI components instead of raw HTML elements.Per coding guidelines, apps should use MUI over HTML elements and avoid CSS classes. The current implementation uses raw HTML (
<section>,<h1>,<ol>, etc.) andclassName="stage".If this is intended as an internal documentation page that won't be user-facing, the current approach may be acceptable. Otherwise, consider refactoring to use MUI components like
Typography,Box,List, andListItemfor consistency with the rest of the codebase.💡 Example refactor using MUI
import { Box, Typography, List, ListItem, ListItemText } from '@mui/material' import { subtitlesWorkflowStages } from '../workflows/subtitles/plan' export default function HomePage() { return ( <Box component="main"> <Box component="section" sx={{ mb: 4 }}> <Typography variant="h4" component="h1"> AI Media — Subtitles Workflow </Typography> <Typography> This app documents and runs the <code>subtitles</code> workflow... </Typography> </Box> {/* ... similar pattern for other sections */} </Box> ) }As per coding guidelines: "Always use MUI over HTML elements; avoid using CSS or tags."
apps/ai-media/src/workflows/subtitles/plan.ts (1)
1-19: Define a type for the workflow stage structure.Per coding guidelines, types should be defined when possible. Adding a type for the stage objects would improve maintainability and provide better IDE support.
♻️ Proposed fix
+type WorkflowStage = { + name: string + path: string + description: string +} + -export const subtitlesWorkflowStages = [ +export const subtitlesWorkflowStages: WorkflowStage[] = [ { name: 'Generate raw subtitles',As per coding guidelines: "Define a type if possible."
apps/ai-media/src/workflows/subtitles/workflow.ts (2)
10-52: Consider consolidating duplicated orchestration logic.The
stagesarray (lines 10-36) and the inline executor function (lines 41-51) both define how to run the three workflow steps. This duplication means changes must be made in two places.If the
stagesarray is intended for individual stage execution while the executor handles full workflow runs, consider having the executor iterate over thestagesarray instead of duplicating the step calls.♻️ Suggested approach to reduce duplication
If the DevKit supports it, the executor could leverage the stages:
export const subtitlesWorkflow = createWorkflow( 'subtitles', stages, - async (input: SubtitlesWorkflowInput, context: WorkflowContext) => { - const generated = await generateMuxSubtitles(input, context) - const improved = await postProcessSubtitles(generated, context) - const upload = await uploadMuxSubtitles(input, improved, context) - - return { - generated, - improved, - upload - } - } + async (input: SubtitlesWorkflowInput, context: WorkflowContext) => { + // Consider using stages array for execution to maintain single source of truth + let state = { request: input, subtitles: null as unknown } + for (const stage of stages) { + state = { ...state, subtitles: await stage.run(state, context) } + } + // Or keep explicit calls but remove the unused stages array + } )Alternatively, if the explicit calls are preferred for clarity, consider removing the
stagesarray if it's not used elsewhere.
16-35: Define explicit types for stage inputs.The inline type casts (e.g., lines 20-21, 28-31) are verbose and could become stale if step function signatures change. Per coding guidelines, consider defining named types for stage inputs.
♻️ Proposed refactor to use named types
In
types.ts, add:export interface PostProcessStageInput { subtitles: SubtitleFile } export interface UploadStageInput { request: SubtitlesWorkflowInput subtitles: SubtitleFile }Then in
workflow.ts:+import type { PostProcessStageInput, UploadStageInput } from './types' { name: subtitlesWorkflowStages[1].name, run: async (input, context) => - postProcessSubtitles( - (input as { subtitles: Awaited<ReturnType<typeof generateMuxSubtitles>> }) - .subtitles, - context - ) + postProcessSubtitles((input as PostProcessStageInput).subtitles, context) }, { name: subtitlesWorkflowStages[2].name, run: async (input, context) => { - const stageInput = input as { - request: SubtitlesWorkflowInput - subtitles: Awaited<ReturnType<typeof postProcessSubtitles>> - } + const stageInput = input as UploadStageInput return uploadMuxSubtitles(stageInput.request, stageInput.subtitles, context) } }
| "include": [ | ||
| "**/*.ts", | ||
| "**/*.tsx", | ||
| "**/*.js", | ||
| "**/*.jsx", | ||
| "**/*.d.ts", | ||
| ".next/types/**/*.ts", | ||
| "../../dist/apps/ai-media/.next/types/**/*.ts", | ||
| "next-env.d.ts", | ||
| ".next/types/**/*.ts", | ||
| "eslint.config.mjs" | ||
| ], |
There was a problem hiding this comment.
Duplicate include entry detected.
Line 28 and Line 31 both include .next/types/**/*.ts. This redundancy should be removed for cleaner configuration.
🔧 Proposed fix
"include": [
"**/*.ts",
"**/*.tsx",
"**/*.js",
"**/*.jsx",
"**/*.d.ts",
".next/types/**/*.ts",
"../../dist/apps/ai-media/.next/types/**/*.ts",
"next-env.d.ts",
- ".next/types/**/*.ts",
"eslint.config.mjs"
],📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "include": [ | |
| "**/*.ts", | |
| "**/*.tsx", | |
| "**/*.js", | |
| "**/*.jsx", | |
| "**/*.d.ts", | |
| ".next/types/**/*.ts", | |
| "../../dist/apps/ai-media/.next/types/**/*.ts", | |
| "next-env.d.ts", | |
| ".next/types/**/*.ts", | |
| "eslint.config.mjs" | |
| ], | |
| "include": [ | |
| "**/*.ts", | |
| "**/*.tsx", | |
| "**/*.js", | |
| "**/*.jsx", | |
| "**/*.d.ts", | |
| ".next/types/**/*.ts", | |
| "../../dist/apps/ai-media/.next/types/**/*.ts", | |
| "next-env.d.ts", | |
| "eslint.config.mjs" | |
| ], |
🤖 Prompt for AI Agents
In `@apps/ai-media/tsconfig.json` around lines 22 - 33, The tsconfig include array
contains a duplicated pattern ".next/types/**/*.ts" which is redundant; open the
tsconfig.json and remove the second occurrence of ".next/types/**/*.ts" from the
"include" array so each glob is listed only once (look for the "include" array
entry and the duplicate ".next/types/**/*.ts" string).
- Added CoverageBar component for visualizing subtitle coverage statistics. - Introduced CoverageReportClient to manage and display collections of videos with subtitle statuses. - Created LanguageSelector for selecting languages dynamically. - Added API route for fetching available languages. - Updated global styles for improved UI consistency. - Introduced a new SVG asset for branding. - Refactored layout and main page to integrate new components and improve user experience.
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In `@apps/ai-media/src/app/api/languages/route.ts`:
- Around line 3-10: The API route is using a client-prefixed env var; update the
environment variable reference from NEXT_PUBLIC_GATEWAY_URL to a server-only
name (e.g., GATEWAY_URL) used by the GET function: replace uses of
NEXT_PUBLIC_GATEWAY_URL in this file with the server-only variable name, ensure
the constant GATEWAY_URL reads process.env.GATEWAY_URL, and update any
deployment/.env configuration to set GATEWAY_URL accordingly so the route can
access it server-side.
In `@apps/ai-media/src/app/coverage-diagram/page.tsx`:
- Around line 6-32: Replace the raw HTML/layout in page.tsx with MUI components:
import and use Container for the outer wrapper, Stack (or Box) for the vertical
layout and grid spacing, and Typography for the h1/h2 headings, moving the
inline style objects into sx props on those MUI components; keep the CoverageBar
usages (counts, activeFilter, onFilter, mode, forceHover) unchanged but place
them inside the MUI Stack/Box children so spacing/gap is controlled via sx
instead of inline styles.
In `@apps/ai-media/src/app/CoverageBar.tsx`:
- Around line 79-81: In CoverageBar.tsx the template literals building the
className for the stat segments (the JSX that uses `segment.className` and
`activeFilter === segment.key`) include an extra `$` before the `{`, producing a
literal `$` in class names and preventing the `is-active` class from applying;
fix both occurrences by removing the stray `$` so the template uses
`${segment.className}` and `${activeFilter === segment.key ? ' is-active' : ''}`
(check the two className usages around the `segment.className`/`activeFilter`
logic and correct them).
In `@apps/ai-media/src/app/CoverageReportClient.tsx`:
- Around line 91-187: The file contains duplicated formatPercent and CoverageBar
definitions (formatPercent, CoverageBar) that already exist in
./CoverageBar.tsx; remove the local implementations and import the shared
CoverageBar (and formatPercent if needed) from ./CoverageBar.tsx, update any
usages to the imported CoverageBar, and reconcile label differences by using the
exported component's props or parameterizing labels (e.g., pass a labels prop or
adapt callers) so the UI text (Human/AI/No vs Edited/Auto/None) is consistent;
ensure you delete the local formatPercent and CoverageBar declarations and add
the import at the top.
- Around line 21-23: Remove the duplicate local type definitions for
SubtitleStatus and CoverageFilter in CoverageReportClient.tsx and instead import
the existing types from the shared coverageTypes module; update the top of the
file to import SubtitleStatus and CoverageFilter and delete the type aliases,
then ensure all references in CoverageReportClient (e.g., any props, state, or
function signatures) use the imported types.
In `@apps/ai-media/src/app/LanguageSelector.tsx`:
- Around line 170-238: The custom dropdown markup in LanguageSelector (the
span/button/div/input/list mapping around isOpen, setIsOpen, searchValue,
searchInputRef, loadedOptions and handleChange) must be replaced with MUI
primitives: use MUI Button for the trigger (preserve aria-haspopup/aria-expanded
and setIsOpen toggle), use Popover (anchorEl derived from the trigger) instead
of the raw "control-dropdown" div for the listbox, replace the search input with
MUI TextField (bind value to searchValue and ref to searchInputRef), render
options using MUI List and ListItemButton (use aria-selected and onClick that
closes Popover, clears searchValue and calls handleChange with the option id),
and move styling into MUI sx props or styled components rather than CSS class
names; keep accessibility attributes (role/listbox/option, aria-labels) intact
and ensure keys use option.id and selected state uses value comparison.
- Around line 153-223: The component sets setIsLoading(true) unconditionally in
handleChange and constructs an unsafe synthetic ChangeEvent when option buttons
call it, which causes a stuck loading state if the selected language is
re-chosen. Change handleChange to accept a plain languageId string (e.g.,
handleChange(nextValue: string)) and inside it only call setIsLoading(true) and
startTransition/router.push when nextValue !== value (the current prop); update
the native <select> onChange to pass event.target.value and update the option
button onClick to call handleChange(option.id) directly; remove the ChangeEvent
import and any casting to synthetic events so the handler is value-based and
avoids the stuck loading state.
🧹 Nitpick comments (8)
apps/ai-media/src/app/layout.tsx (1)
10-14: Extract a named props type for RootLayout.
This keeps the component signature consistent with TS guidelines and improves reuse.As per coding guidelines, "**/*.{ts,tsx}: Define a type if possible."♻️ Proposed refactor
-import type { ReactNode } from 'react' +import type { ReactNode } from 'react' +type RootLayoutProps = { + children: ReactNode +} + export default function RootLayout({ children -}: { - children: ReactNode -}) { +}: RootLayoutProps) {apps/ai-media/src/app/api/languages/route.ts (1)
48-57: Extract the gateway response type alias.
A named type improves readability and reuse in this route.As per coding guidelines, "**/*.{ts,tsx}: Define a type if possible."♻️ Proposed refactor
+type GatewayLanguagesResponse = { + data?: { + languages?: Array<{ + id: string + slug?: string + nativeName?: Array<{ value: string }> + name?: Array<{ value: string; language?: { id: string } }> + }> + } +} + - const payload = (await response.json()) as { - data?: { - languages?: Array<{ - id: string - slug?: string - nativeName?: Array<{ value: string }> - name?: Array<{ value: string; language?: { id: string } }> - }> - } - } + const payload = (await response.json()) as GatewayLanguagesResponseapps/ai-media/src/app/page.tsx (1)
74-80: Reuse the shared SubtitleStatus type instead of re-declaring the union.
This avoids duplication and keeps status typing consistent across the app.As per coding guidelines, "**/*.{ts,tsx}: Define a type if possible."♻️ Proposed refactor
import { languageSlugs } from '@core/prisma/languages/__generated__/languageSlugs' import { CoverageReportClient } from './CoverageReportClient' +import type { SubtitleStatus } from './coverageTypes' @@ -function getSubtitleStatus(subtitles?: Subtitle[] | null): 'human' | 'ai' | 'none' { +function getSubtitleStatus(subtitles?: Subtitle[] | null): SubtitleStatus {apps/ai-media/src/app/CoverageBar.tsx (1)
64-66: Redundant ternary with identical branches.Both branches return the same string. Either differentiate the text based on mode or simplify to a constant.
♻️ Proposed fix
- const helperText = isExplore - ? 'Click a segment to filter.' - : 'Click a segment to filter.' + const helperText = isExplore + ? 'Click a segment to filter.' + : 'Click a segment to select videos by status.'Or if both modes truly share the same text:
- const helperText = isExplore - ? 'Click a segment to filter.' - : 'Click a segment to filter.' + const helperText = 'Click a segment to filter.'apps/ai-media/src/app/CoverageReportClient.tsx (4)
147-149: Redundant ternary with identical branches.Same issue as in
CoverageBar.tsx— both branches return the same string.♻️ Proposed fix
- const helperText = isExplore - ? 'Click a segment to filter.' - : 'Click a segment to filter.' + const helperText = 'Click a segment to filter.'
612-616: Use Next.jsImagecomponent for optimization.Raw
<img>tags bypass Next.js image optimization (lazy loading, responsive sizing, format conversion). Use thenext/imagecomponent instead.♻️ Proposed fix
+import Image from 'next/image' ... - <img - src="/jesusfilm-sign.svg" - alt="Jesus Film Project" - className="header-logo" - /> + <Image + src="/jesusfilm-sign.svg" + alt="Jesus Film Project" + className="header-logo" + width={120} + height={40} + priority + />Adjust
widthandheightto match the actual SVG dimensions.Based on learnings: "Use Next.js Image component with proper loading strategies for image optimization."
505-518:allVideoIdsis computed but never used.The
allVideoIdsmemo (lines 505-508) is defined but not referenced anywhere in the component. Consider removing it to reduce unnecessary computation.♻️ Proposed fix
- const allVideoIds = useMemo( - () => collections.flatMap((collection) => collection.videos.map((video) => video.id)), - [collections] - ) -
548-550:handleSelectAllMissingis defined but never called.This handler adds all missing subtitle video IDs to the selection, but there's no UI element that invokes it. Either wire it up or remove the dead code.
| const GATEWAY_URL = process.env.NEXT_PUBLIC_GATEWAY_URL | ||
|
|
||
| export async function GET() { | ||
| if (!GATEWAY_URL) { | ||
| return NextResponse.json( | ||
| { error: 'NEXT_PUBLIC_GATEWAY_URL is not set.' }, | ||
| { status: 500 } | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find environment configuration files in apps/ai-media
fd -e ts -e js -e env -e json "env" "apps/ai-media" --type f | head -20Repository: JesusFilm/core
Length of output: 85
🏁 Script executed:
# Check the structure of apps/ai-media
ls -la "apps/ai-media/" | head -20Repository: JesusFilm/core
Length of output: 800
🏁 Script executed:
# Look for env-related files and configurations
find "apps/ai-media" -type f \( -name "*.env*" -o -name "*env*" \) -o -name ".env*" 2>/dev/null | head -20Repository: JesusFilm/core
Length of output: 85
🏁 Script executed:
# Search for GATEWAY_URL usage across the codebase
rg "GATEWAY_URL" --type ts --type tsx --type js --type jsx -B 2 -A 2Repository: JesusFilm/core
Length of output: 85
🏁 Script executed:
# Check the actual file in question
cat -n "apps/ai-media/src/app/api/languages/route.ts"Repository: JesusFilm/core
Length of output: 2589
🏁 Script executed:
# Search for env.ts or similar validation in apps/ai-media
find "apps/ai-media" -name "env.ts" -o -name "env.js" -o -name "environment.ts"Repository: JesusFilm/core
Length of output: 40
🏁 Script executed:
# Check if NEXT_PUBLIC_GATEWAY_URL is accessed from client-side code
rg "NEXT_PUBLIC_GATEWAY_URL|GATEWAY_URL" "apps/ai-media/src" -B 2 -A 2Repository: JesusFilm/core
Length of output: 2952
🏁 Script executed:
# Check package.json to understand app type
cat "apps/ai-media/package.json" 2>/dev/null || echo "No package.json found"Repository: JesusFilm/core
Length of output: 79
🏁 Script executed:
# Look for any .env files or env configuration
ls -la "apps/ai-media/"* 2>/dev/null | grep -i env || echo "No env files found"Repository: JesusFilm/core
Length of output: 134
Use a server-only environment variable in the API route.
Remove the NEXT_PUBLIC_ prefix since this API route is server-only. NEXT_PUBLIC_ variables are intended for values that can be accessed by the client, not for server-only contexts.
Suggested fix
-const GATEWAY_URL = process.env.NEXT_PUBLIC_GATEWAY_URL
+const GATEWAY_URL = process.env.GATEWAY_URL
export async function GET() {
if (!GATEWAY_URL) {
return NextResponse.json(
- { error: 'NEXT_PUBLIC_GATEWAY_URL is not set.' },
+ { error: 'GATEWAY_URL is not set.' },
{ status: 500 }
)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const GATEWAY_URL = process.env.NEXT_PUBLIC_GATEWAY_URL | |
| export async function GET() { | |
| if (!GATEWAY_URL) { | |
| return NextResponse.json( | |
| { error: 'NEXT_PUBLIC_GATEWAY_URL is not set.' }, | |
| { status: 500 } | |
| ) | |
| const GATEWAY_URL = process.env.GATEWAY_URL | |
| export async function GET() { | |
| if (!GATEWAY_URL) { | |
| return NextResponse.json( | |
| { error: 'GATEWAY_URL is not set.' }, | |
| { status: 500 } | |
| ) |
🤖 Prompt for AI Agents
In `@apps/ai-media/src/app/api/languages/route.ts` around lines 3 - 10, The API
route is using a client-prefixed env var; update the environment variable
reference from NEXT_PUBLIC_GATEWAY_URL to a server-only name (e.g., GATEWAY_URL)
used by the GET function: replace uses of NEXT_PUBLIC_GATEWAY_URL in this file
with the server-only variable name, ensure the constant GATEWAY_URL reads
process.env.GATEWAY_URL, and update any deployment/.env configuration to set
GATEWAY_URL accordingly so the route can access it server-side.
| return ( | ||
| <main style={{ maxWidth: 720, margin: '0 auto', padding: '40px 20px' }}> | ||
| <h1 style={{ margin: '0 0 24px', fontSize: '1.6rem' }}> | ||
| Coverage Diagram Mock | ||
| </h1> | ||
| <section style={{ display: 'grid', gap: '24px' }}> | ||
| <div> | ||
| <h2 style={{ margin: '0 0 10px', fontSize: '1rem' }}>Normal state</h2> | ||
| <CoverageBar | ||
| counts={counts} | ||
| activeFilter="all" | ||
| onFilter={() => undefined} | ||
| mode="explore" | ||
| /> | ||
| </div> | ||
| <div> | ||
| <h2 style={{ margin: '0 0 10px', fontSize: '1rem' }}>Hover state</h2> | ||
| <CoverageBar | ||
| counts={counts} | ||
| activeFilter="all" | ||
| onFilter={() => undefined} | ||
| mode="explore" | ||
| forceHover | ||
| /> | ||
| </div> | ||
| </section> | ||
| </main> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if the file exists and read its content
cd /tmp && find . -name "page.tsx" -path "*coverage-diagram*" 2>/dev/null | head -5Repository: JesusFilm/core
Length of output: 40
🏁 Script executed:
# Search for the file in the repo
git ls-files | grep -E "ai-media.*coverage-diagram.*page\.tsx"Repository: JesusFilm/core
Length of output: 105
🏁 Script executed:
# Check the directory structure of apps
git ls-files | grep "^apps/" | cut -d'/' -f1-2 | sort | uniqRepository: JesusFilm/core
Length of output: 491
🏁 Script executed:
# Look for any style guidelines in apps/ai-media
git ls-files | grep -E "^apps/ai-media.*\.(md|mdc)$"Repository: JesusFilm/core
Length of output: 81
🏁 Script executed:
# Read the actual file content
cat -n apps/ai-media/src/app/coverage-diagram/page.tsxRepository: JesusFilm/core
Length of output: 1286
🏁 Script executed:
# Read the README to understand app's UI framework preference
cat apps/ai-media/README.mdRepository: JesusFilm/core
Length of output: 303
🏁 Script executed:
# Check if MUI is used anywhere in ai-media
rg "@mui" apps/ai-media/src --type tsx --type ts | head -20Repository: JesusFilm/core
Length of output: 85
🏁 Script executed:
# Check what components ai-media uses
rg "^import.*from" apps/ai-media/src --type tsx --type ts | head -20Repository: JesusFilm/core
Length of output: 85
🏁 Script executed:
# Check if MUI is used anywhere in ai-media
rg "@mui" apps/ai-media/src --no-type | head -20Repository: JesusFilm/core
Length of output: 493
🏁 Script executed:
# Check what's installed in the project (look at package.json)
cat package.json | grep -A 50 '"dependencies"' | grep -E "mui|tailwind|styled"Repository: JesusFilm/core
Length of output: 425
🏁 Script executed:
# Look at other files in ai-media to understand UI pattern
git ls-files apps/ai-media/src | head -20Repository: JesusFilm/core
Length of output: 976
🏁 Script executed:
# Check layout.tsx to see the UI pattern
cat -n apps/ai-media/src/app/layout.tsxRepository: JesusFilm/core
Length of output: 603
🏁 Script executed:
# Check main page.tsx to see the UI pattern
cat -n apps/ai-media/src/app/page.tsx | head -60Repository: JesusFilm/core
Length of output: 1948
🏁 Script executed:
# Check CoverageBar.tsx to see how components are structured
cat -n apps/ai-media/src/app/CoverageBar.tsx | head -80Repository: JesusFilm/core
Length of output: 2657
Replace inline-styled HTML with MUI components.
This page uses raw HTML tags and inline styles for layout. Per the app guidelines for apps/**/*.{js,jsx,ts,tsx}, use MUI (Container, Stack, Typography, etc.) and move styling to sx/theme instead.
Suggested approach
+import { Box, Container, Stack, Typography } from '@mui/material'
import { CoverageBar } from '../CoverageBar'
export default function CoverageDiagramPage() {
const counts = { human: 86, ai: 0, none: 14 }
return (
- <main style={{ maxWidth: 720, margin: '0 auto', padding: '40px 20px' }}>
- <h1 style={{ margin: '0 0 24px', fontSize: '1.6rem' }}>
- Coverage Diagram Mock
- </h1>
- <section style={{ display: 'grid', gap: '24px' }}>
- <div>
- <h2 style={{ margin: '0 0 10px', fontSize: '1rem' }}>Normal state</h2>
+ <Container maxWidth="md" sx={{ py: 5 }}>
+ <Typography variant="h4" sx={{ mb: 3 }}>
+ Coverage Diagram Mock
+ </Typography>
+ <Stack spacing={3}>
+ <Box>
+ <Typography variant="subtitle1" sx={{ mb: 1 }}>
+ Normal state
+ </Typography>
<CoverageBar
counts={counts}
activeFilter="all"
onFilter={() => undefined}
mode="explore"
/>
- </div>
- <div>
- <h2 style={{ margin: '0 0 10px', fontSize: '1rem' }}>Hover state</h2>
+ </Box>
+ <Box>
+ <Typography variant="subtitle1" sx={{ mb: 1 }}>
+ Hover state
+ </Typography>
<CoverageBar
counts={counts}
activeFilter="all"
onFilter={() => undefined}
mode="explore"
forceHover
/>
- </div>
- </section>
- </main>
+ </Box>
+ </Stack>
+ </Container>
)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return ( | |
| <main style={{ maxWidth: 720, margin: '0 auto', padding: '40px 20px' }}> | |
| <h1 style={{ margin: '0 0 24px', fontSize: '1.6rem' }}> | |
| Coverage Diagram Mock | |
| </h1> | |
| <section style={{ display: 'grid', gap: '24px' }}> | |
| <div> | |
| <h2 style={{ margin: '0 0 10px', fontSize: '1rem' }}>Normal state</h2> | |
| <CoverageBar | |
| counts={counts} | |
| activeFilter="all" | |
| onFilter={() => undefined} | |
| mode="explore" | |
| /> | |
| </div> | |
| <div> | |
| <h2 style={{ margin: '0 0 10px', fontSize: '1rem' }}>Hover state</h2> | |
| <CoverageBar | |
| counts={counts} | |
| activeFilter="all" | |
| onFilter={() => undefined} | |
| mode="explore" | |
| forceHover | |
| /> | |
| </div> | |
| </section> | |
| </main> | |
| import { Box, Container, Stack, Typography } from '@mui/material' | |
| import { CoverageBar } from '../CoverageBar' | |
| export default function CoverageDiagramPage() { | |
| const counts = { human: 86, ai: 0, none: 14 } | |
| return ( | |
| <Container maxWidth="md" sx={{ py: 5 }}> | |
| <Typography variant="h4" sx={{ mb: 3 }}> | |
| Coverage Diagram Mock | |
| </Typography> | |
| <Stack spacing={3}> | |
| <Box> | |
| <Typography variant="subtitle1" sx={{ mb: 1 }}> | |
| Normal state | |
| </Typography> | |
| <CoverageBar | |
| counts={counts} | |
| activeFilter="all" | |
| onFilter={() => undefined} | |
| mode="explore" | |
| /> | |
| </Box> | |
| <Box> | |
| <Typography variant="subtitle1" sx={{ mb: 1 }}> | |
| Hover state | |
| </Typography> | |
| <CoverageBar | |
| counts={counts} | |
| activeFilter="all" | |
| onFilter={() => undefined} | |
| mode="explore" | |
| forceHover | |
| /> | |
| </Box> | |
| </Stack> | |
| </Container> | |
| ) | |
| } |
🤖 Prompt for AI Agents
In `@apps/ai-media/src/app/coverage-diagram/page.tsx` around lines 6 - 32, Replace
the raw HTML/layout in page.tsx with MUI components: import and use Container
for the outer wrapper, Stack (or Box) for the vertical layout and grid spacing,
and Typography for the h1/h2 headings, moving the inline style objects into sx
props on those MUI components; keep the CoverageBar usages (counts,
activeFilter, onFilter, mode, forceHover) unchanged but place them inside the
MUI Stack/Box children so spacing/gap is controlled via sx instead of inline
styles.
| className={`stat-segment ${segment.className}$${ | ||
| activeFilter === segment.key ? ' is-active' : '' | ||
| }`} |
There was a problem hiding this comment.
Extra $ in template literals produces broken class names.
Lines 79 and 97 have $${ instead of ${, which inserts a literal $ into the class string. This breaks the .is-active class application.
🐛 Proposed fix
- className={`stat-segment ${segment.className}$${
+ className={`stat-segment ${segment.className}${
activeFilter === segment.key ? ' is-active' : ''
}`}- className={`stat-legend-item stat-legend-item--${segment.key}$${
+ className={`stat-legend-item stat-legend-item--${segment.key}${
activeFilter === segment.key ? ' is-active' : ''
}`}Also applies to: 97-99
🤖 Prompt for AI Agents
In `@apps/ai-media/src/app/CoverageBar.tsx` around lines 79 - 81, In
CoverageBar.tsx the template literals building the className for the stat
segments (the JSX that uses `segment.className` and `activeFilter ===
segment.key`) include an extra `$` before the `{`, producing a literal `$` in
class names and preventing the `is-active` class from applying; fix both
occurrences by removing the stray `$` so the template uses
`${segment.className}` and `${activeFilter === segment.key ? ' is-active' : ''}`
(check the two className usages around the `segment.className`/`activeFilter`
logic and correct them).
| type SubtitleStatus = 'human' | 'ai' | 'none' | ||
|
|
||
| type CoverageFilter = 'all' | SubtitleStatus |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Duplicate type definitions.
SubtitleStatus and CoverageFilter are already exported from ./coverageTypes.ts. Import them instead of redefining to maintain a single source of truth.
♻️ Proposed fix
import { LanguageSelector } from './LanguageSelector'
+import { CoverageFilter, SubtitleStatus } from './coverageTypes'
-type SubtitleStatus = 'human' | 'ai' | 'none'
-
-type CoverageFilter = 'all' | SubtitleStatus📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| type SubtitleStatus = 'human' | 'ai' | 'none' | |
| type CoverageFilter = 'all' | SubtitleStatus | |
| import { LanguageSelector } from './LanguageSelector' | |
| import { CoverageFilter, SubtitleStatus } from './coverageTypes' |
🤖 Prompt for AI Agents
In `@apps/ai-media/src/app/CoverageReportClient.tsx` around lines 21 - 23, Remove
the duplicate local type definitions for SubtitleStatus and CoverageFilter in
CoverageReportClient.tsx and instead import the existing types from the shared
coverageTypes module; update the top of the file to import SubtitleStatus and
CoverageFilter and delete the type aliases, then ensure all references in
CoverageReportClient (e.g., any props, state, or function signatures) use the
imported types.
| function formatPercent(count: number, total: number): number { | ||
| if (total === 0) return 0 | ||
| return Math.round((count / total) * 100) | ||
| } | ||
|
|
||
| function CoverageBar({ | ||
| counts, | ||
| activeFilter, | ||
| onFilter, | ||
| onSelectStatus, | ||
| mode | ||
| }: { | ||
| counts: { human: number; ai: number; none: number } | ||
| activeFilter: CoverageFilter | ||
| onFilter: (filter: CoverageFilter) => void | ||
| onSelectStatus?: (status: SubtitleStatus) => void | ||
| mode: Mode | ||
| }) { | ||
| const total = counts.human + counts.ai + counts.none | ||
| const segments: Array<{ | ||
| key: SubtitleStatus | ||
| label: string | ||
| percent: number | ||
| className: string | ||
| }> = [ | ||
| { | ||
| key: 'human', | ||
| label: 'Human', | ||
| percent: formatPercent(counts.human, total), | ||
| className: 'stat-segment--human' | ||
| }, | ||
| { | ||
| key: 'ai', | ||
| label: 'AI', | ||
| percent: formatPercent(counts.ai, total), | ||
| className: 'stat-segment--ai' | ||
| }, | ||
| { | ||
| key: 'none', | ||
| label: 'No', | ||
| percent: Math.max(0, 100 - formatPercent(counts.human, total) - formatPercent(counts.ai, total)), | ||
| className: 'stat-segment--none' | ||
| } | ||
| ] | ||
|
|
||
| const isExplore = mode === 'explore' | ||
| const isInteractive = Boolean(onSelectStatus) || isExplore | ||
|
|
||
| const handleSegmentClick = (status: SubtitleStatus) => { | ||
| if (isExplore) { | ||
| onFilter(status) | ||
| return | ||
| } | ||
| onSelectStatus?.(status) | ||
| } | ||
|
|
||
| const helperText = isExplore | ||
| ? 'Click a segment to filter.' | ||
| : 'Click a segment to filter.' | ||
|
|
||
| return ( | ||
| <div className={`coverage-bar${isInteractive ? ' is-interactive' : ''}`}> | ||
| <p className="coverage-hint">{helperText}</p> | ||
| <div className="stat-bar" aria-label="Subtitle coverage"> | ||
| {segments.map((segment) => ( | ||
| <button | ||
| key={segment.key} | ||
| type="button" | ||
| className={`stat-segment ${segment.className}${ | ||
| activeFilter === segment.key ? ' is-active' : '' | ||
| }`} | ||
| style={{ width: `${segment.percent}%` }} | ||
| title={`${segment.label} translated: ${counts[segment.key]}`} | ||
| aria-pressed={activeFilter === segment.key} | ||
| onClick={() => handleSegmentClick(segment.key)} | ||
| disabled={!isInteractive} | ||
| /> | ||
| ))} | ||
| </div> | ||
| <div className="stat-legend"> | ||
| {segments.map((segment) => ( | ||
| <button | ||
| key={segment.key} | ||
| type="button" | ||
| className={`stat-legend-item stat-legend-item--${segment.key}${ | ||
| activeFilter === segment.key ? ' is-active' : '' | ||
| }`} | ||
| onClick={() => handleSegmentClick(segment.key)} | ||
| disabled={!isInteractive} | ||
| > | ||
| {segment.label} {segment.percent}% | ||
| </button> | ||
| ))} | ||
| </div> | ||
| </div> | ||
| ) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Duplicate formatPercent and CoverageBar component.
Both formatPercent (lines 91-94) and the entire CoverageBar component (lines 96-187) are duplicated from ./CoverageBar.tsx. Import and reuse the existing module to avoid drift and maintenance burden.
♻️ Proposed fix
Add the import at the top of the file:
import { LanguageSelector } from './LanguageSelector'
+import { CoverageBar } from './CoverageBar'
+import { CoverageFilter, SubtitleStatus } from './coverageTypes'Then remove lines 91-187 (the local formatPercent and CoverageBar definitions).
Note: The local CoverageBar uses slightly different labels ("Human"/"AI"/"No" vs "Edited"/"Auto"/"None" in the exported version). Consolidate to one version and parameterize labels if needed.
🤖 Prompt for AI Agents
In `@apps/ai-media/src/app/CoverageReportClient.tsx` around lines 91 - 187, The
file contains duplicated formatPercent and CoverageBar definitions
(formatPercent, CoverageBar) that already exist in ./CoverageBar.tsx; remove the
local implementations and import the shared CoverageBar (and formatPercent if
needed) from ./CoverageBar.tsx, update any usages to the imported CoverageBar,
and reconcile label differences by using the exported component's props or
parameterizing labels (e.g., pass a labels prop or adapt callers) so the UI text
(Human/AI/No vs Edited/Auto/None) is consistent; ensure you delete the local
formatPercent and CoverageBar declarations and add the import at the top.
| return ( | ||
| <span className="control-select-shell" ref={shellRef}> | ||
| <button | ||
| type="button" | ||
| className={className} | ||
| aria-haspopup="listbox" | ||
| aria-expanded={isOpen} | ||
| onClick={() => setIsOpen((prev) => !prev)} | ||
| > | ||
| <span className="control-select-text">{selectedLabel}</span> | ||
| <span className="control-chevron" aria-hidden="true" /> | ||
| </button> | ||
| {(isLoading || isPending) && ( | ||
| <span className="control-loading" aria-live="polite"> | ||
| Loading… | ||
| </span> | ||
| )} | ||
| {isOpen && ( | ||
| <div className="control-dropdown" role="listbox" aria-label="Language"> | ||
| <div className="control-search"> | ||
| <input | ||
| type="search" | ||
| value={searchValue} | ||
| ref={searchInputRef} | ||
| onChange={(event) => setSearchValue(event.target.value)} | ||
| placeholder="Search languages..." | ||
| aria-label="Search languages" | ||
| className="control-search-input" | ||
| /> | ||
| </div> | ||
| {loadedOptions | ||
| .filter((option) => { | ||
| const query = searchValue.trim().toLowerCase() | ||
| if (!query) return true | ||
| return ( | ||
| option.englishLabel.toLowerCase().includes(query) || | ||
| option.nativeLabel.toLowerCase().includes(query) | ||
| ) | ||
| }) | ||
| .map((option) => { | ||
| const isSelected = option.id === value | ||
| return ( | ||
| <button | ||
| key={option.id} | ||
| type="button" | ||
| role="option" | ||
| aria-selected={isSelected} | ||
| className={`control-option${isSelected ? ' is-selected' : ''}`} | ||
| onClick={() => { | ||
| setIsOpen(false) | ||
| setSearchValue('') | ||
| handleChange({ | ||
| target: { value: option.id } | ||
| } as ChangeEvent<HTMLSelectElement>) | ||
| }} | ||
| > | ||
| <span className="control-option-english"> | ||
| {option.englishLabel} | ||
| </span> | ||
| {option.nativeLabel && | ||
| option.nativeLabel !== option.englishLabel && ( | ||
| <span className="control-option-native"> | ||
| {option.nativeLabel} | ||
| </span> | ||
| )} | ||
| </button> | ||
| ) | ||
| })} | ||
| </div> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if apps/ai-media has its own guidelines
find apps/ai-media -name "AGENTS.md" -o -name "*.mdc" | head -20Repository: JesusFilm/core
Length of output: 40
🏁 Script executed:
# Also check the general structure of apps/ai-media
ls -la apps/ai-media/ 2>/dev/null || echo "Directory listing failed"Repository: JesusFilm/core
Length of output: 800
🏁 Script executed:
# Verify the LanguageSelector.tsx file exists and inspect it
if [ -f apps/ai-media/src/app/LanguageSelector.tsx ]; then
wc -l apps/ai-media/src/app/LanguageSelector.tsx
else
echo "File not found at expected path"
# Try broader search
find . -name "LanguageSelector.tsx" -type f
fiRepository: JesusFilm/core
Length of output: 104
🏁 Script executed:
# Read the full LanguageSelector.tsx file
cat -n apps/ai-media/src/app/LanguageSelector.tsxRepository: JesusFilm/core
Length of output: 8860
🏁 Script executed:
# Check for any ai-media specific cursor rules or guidelines
find . -path "*cursor*" -o -path "*AGENTS*" | grep -i "ai-media" || echo "No specific ai-media rules found"Repository: JesusFilm/core
Length of output: 90
🏁 Script executed:
# Check workspace.json or project config for ai-media
cat apps/ai-media/project.json 2>/dev/null | head -50Repository: JesusFilm/core
Length of output: 1330
🏁 Script executed:
# Check if MUI is in workspace dependencies
cat package.json | grep -A 20 '"dependencies"' | grep -i mui || echo "Checking root package.json for MUI"Repository: JesusFilm/core
Length of output: 92
🏁 Script executed:
# Check if MUI is listed in root or workspace package.json
if [ -f package.json ]; then
grep -i "@mui" package.json || echo "No MUI found in root package.json"
fiRepository: JesusFilm/core
Length of output: 390
🏁 Script executed:
# Check other components in apps/ai-media to understand the UI pattern
find apps/ai-media/src -name "*.tsx" -o -name "*.jsx" | head -10Repository: JesusFilm/core
Length of output: 297
🏁 Script executed:
# Sample a few other component files to see what UI framework they use
find apps/ai-media/src -name "*.tsx" -type f | head -5 | xargs -I {} sh -c 'echo "=== {} ===" && head -20 {}'Repository: JesusFilm/core
Length of output: 2774
Use MUI components instead of raw HTML and CSS classes for the dropdown.
This custom dropdown violates the app guideline requiring MUI primitives. Replace the HTML elements (button, div, input) and CSS classes with MUI components: Button, Popover, TextField, List, and ListItemButton.
Per coding guidelines: "apps/**/*.{js,jsx,ts,tsx}: Always use MUI over HTML elements; avoid using CSS or tags."
🤖 Prompt for AI Agents
In `@apps/ai-media/src/app/LanguageSelector.tsx` around lines 170 - 238, The
custom dropdown markup in LanguageSelector (the span/button/div/input/list
mapping around isOpen, setIsOpen, searchValue, searchInputRef, loadedOptions and
handleChange) must be replaced with MUI primitives: use MUI Button for the
trigger (preserve aria-haspopup/aria-expanded and setIsOpen toggle), use Popover
(anchorEl derived from the trigger) instead of the raw "control-dropdown" div
for the listbox, replace the search input with MUI TextField (bind value to
searchValue and ref to searchInputRef), render options using MUI List and
ListItemButton (use aria-selected and onClick that closes Popover, clears
searchValue and calls handleChange with the option id), and move styling into
MUI sx props or styled components rather than CSS class names; keep
accessibility attributes (role/listbox/option, aria-labels) intact and ensure
keys use option.id and selected state uses value comparison.
Co-authored-by: Cursor <cursoragent@cursor.com>
…n' of https://github.com/JesusFilm/core into cursor/vm-26-create-ai-media-app-for-subtitle-generation
There was a problem hiding this comment.
Actionable comments posted: 13
🤖 Fix all issues with AI agents
In `@apps/ai-media/scripts/export-country-language-populations.mjs`:
- Around line 56-57: The GraphQL response payload is never checked for
GraphQL-level errors so payload.errors or a missing payload.data can silently
produce an empty languages array; update the code after awaiting response.json()
to (1) check if payload.errors exists and is non-empty and, if so, log the
errors (include payload.errors details) and exit/throw, and (2) verify
payload.data and payload.data.languages exist before using them (log a clear
error and exit/throw if missing) so the const languages = ... line only runs on
a valid GraphQL data response.
In `@apps/ai-media/src/app/CoverageReportClient.tsx`:
- Around line 1300-1332: The current useEffect logic uses a length-only check
(collections.length > cached.length) after readCachedCollections(languageKey),
which prevents updating the cache when the server-side payload is smaller
(deleted items); change the logic to prefer fresh non-empty server data or
compare content via a stable hash: call readCachedCollections(languageKey) and
readCacheMeta(languageKey) as before, but replace the length check with either
(a) if collections is non-empty always writeCachedCollections(languageKey,
collections) and setCachedCollections(collections) and setCacheMeta from
writeCachedCollections, or (b) compute a deterministic content hash of
collections and compare it to a stored hash in meta and update when hashes
differ; update references in this effect to use writeCachedCollections,
setCachedCollections, setCacheMeta, and languageKey accordingly.
- Around line 417-419: The helperText ternary in CoverageReportClient.tsx (const
helperText using isExplore) currently returns the same string for both branches;
change the else branch to a distinct message for select mode (e.g., when
isExplore is false return "Click a segment to select videos by status.") so the
UI communicates filtering vs selecting behavior; update the const helperText
(and any related tooltip/label usages) to use the new select-mode string.
- Around line 146-167: The function writeCachedCollections currently returns
undefined when run on the server (it does a bare return), which conflicts with
its declared return type CollectionsCacheMeta | null; update the early exit in
writeCachedCollections (the typeof window === 'undefined' branch) to return null
instead of a bare return so the function always returns either a
CollectionsCacheMeta or null and satisfies strict null checking.
- Around line 754-767: The span returned in CoverageReportClient.tsx is using
onFocus/onBlur but isn't focusable; make the element keyboard-accessible by
adding tabIndex={0} and an appropriate role (e.g., role="button" or
role="group") to the outer <span> that has
onMouseEnter/onMouseLeave/onFocus/onBlur so those handlers can fire for keyboard
users; optionally also consider adding onKeyDown to invoke
onHoverStart/onHoverEnd for Enter/Space if it should behave like a button.
- Around line 1419-1429: statusIdMap currently groups videos using
video.subtitleStatus and doesn't re-compute when reportType changes; update the
useMemo for statusIdMap to call getReportStatus(video) (instead of
video.subtitleStatus) when computing the key and add reportType to the
dependency array so the map updates when the active report type changes;
reference the existing symbols statusIdMap, cachedCollections, getReportStatus,
reportType, and video.subtitleStatus when making this change.
- Around line 1174-1194: This span uses onFocus/onBlur but is aria-hidden and
not focusable, so either remove the focus handlers if the tile is purely
decorative, or make the tile keyboard-accessible: add tabIndex={0} (and remove
aria-hidden or set aria-hidden={false}), keep the onFocus/onBlur that call
onHoverVideo, and also add ARIA role (e.g., role="button" or role="option" as
appropriate) and keyboard handlers (onKeyDown to handle Enter/Space) so keyboard
users can trigger the same behavior; locate the span with the title
interpolation and the onHoverVideo usage to apply the change.
- Around line 1437-1445: handleToggleCollection, handleSelectByStatus, and
handleSelectByStatusInCollection compute selection decisions using the captured
selectedSet, causing stale-toggle bugs when multiple updates are batched; change
each to compute whether to add/remove inside the setSelectedIds functional
updater (use the incoming prev state) instead of using selectedSet, e.g., move
the allSelected or status-check logic into the updater callback and then return
the new array (using Set semantics to dedupe) so the decision is based on the
latest prev state.
- Around line 1016-1033: The section element rendering the collection card
(className="collection-card") should not be treated as a button — remove
role="button" and tabIndex={0} from that <section> and stop using onKeyDown on
it; keep the existing dedicated expand/collapse button (the button tied to
onToggleExpanded and isExpanded around line ~1101) as the only
keyboard-accessible control for expand/collapse. Ensure the onClick handler on
the section no longer invokes onToggleExpanded(collection.id) for plain clicks
that should be handled by the inner toggle button (you may keep click guard
logic for links/inputs/tiles but do not call onToggleExpanded from the section),
and leave the named handlers (onClick, onKeyDown, onToggleExpanded,
collection.id, isExpanded) intact elsewhere so the explicit button continues to
manage expansion.
- Around line 1457-1471: The handler handleSelectByStatusInCollection currently
compares video.subtitleStatus only, causing wrong selections for non-subtitle
reports; change the filter to use the report-aware status (e.g.,
getReportStatus(video) === status) or pass reportType into the handler and
compare against the appropriate field for that type; also update the useCallback
dependency array to include reportType (or remove closure use by accepting
reportType as a parameter) so the memoized function reflects the active report
mode and selectedSet remains correct.
In `@apps/ai-media/src/app/LanguageGeoSelector.tsx`:
- Around line 614-641: The className template strings for the continent and
country buttons in LanguageGeoSelector are concatenating 'is-active' without a
leading space, breaking styling; update the continent button's className (where
draftContinents.has(continent.id) is checked) and the country button's className
(where draftCountries.has(country.id) is checked) so the conditional inserts "
is-active" (with a leading space) instead of "is-active" when true (e.g., change
the string to `' is-active'` or otherwise ensure a separating space is
included); no other logic changes are needed (toggleContinent, toggleCountry,
draftContinents, draftCountries remain the same).
In `@apps/ai-media/src/app/LanguageSelector.tsx`:
- Around line 112-133: The code incorrectly casts the fetch result to string[][]
and calls .map(), causing a TypeError; instead parse the response as the
GeoPayload shape and iterate payload.languages (not payload directly) when
building nextOptions. Update the type assertion on the response (replace
string[][] with GeoPayload), extract the languages array (e.g., const {
languages } = payload), then map over languages to produce nextOptions using the
existing logic (preserve id, englishLabel, nativeLabel, filtering and sorting)
so it matches the pattern used in LanguageGeoSelector.tsx and avoids the silent
fallback.
In `@apps/ai-media/src/app/page.tsx`:
- Around line 317-324: Move the response.json() call to after validating
response.ok so JSON parse errors don't mask HTTP errors: first check response.ok
on the response object and if it's not ok read response.text() (or include
response.status and statusText) and throw a descriptive Error with HTTP status;
only then call await response.json() to populate payload and continue the
existing payload.errors check (refer to the response and payload variables in
this block inside page.tsx).
🧹 Nitpick comments (10)
.codex/environments/environment.toml (1)
19-25: Consider adding commented placeholders for required secrets.The
.envtemplate looks complete at first glance, but the subtitles workflow requires Mux and OpenAI credentials (e.g.,MUX_TOKEN_ID,MUX_TOKEN_SECRET,OPENAI_API_KEY) to function. Without placeholders, a developer may assume the generated.envis sufficient and only discover missing keys at runtime.Suggested improvement
cat <<EOF > "$ENV_FILE" NODE_ENV=development NEXT_PUBLIC_GATEWAY_URL=https://api-gateway.central.jesusfilm.org/ NEXT_PUBLIC_WATCH_URL=https://jesusfilm.org/watch + +# Required for Mux AI subtitle workflows (do not commit real values) +# MUX_TOKEN_ID= +# MUX_TOKEN_SECRET= +# OPENAI_API_KEY= EOFapps/ai-media/src/app/LanguageSelector.tsx (1)
171-175:router.pushis unreachable dead code.In a
'use client'component,windowis always defined when this code executes, so thetypeof window !== 'undefined'check is always true androuter.pushis never reached.Simplify
setIsLoading(true) - if (typeof window !== 'undefined') { - window.location.href = nextUrl - return - } - router.push(nextUrl) + window.location.href = nextUrl }apps/ai-media/src/app/LanguageGeoSelector.tsx (2)
506-512: Dead code —typeof windowcheck is always true insidesetTimeout.The callback is scheduled via
window.setTimeout, sowindowis guaranteed to exist. Therouter.pushbranch on line 511 is unreachable.Simplify
navigationTimeoutRef.current = window.setTimeout(() => { - if (typeof window !== 'undefined') { - window.location.href = nextUrl - return - } - router.push(nextUrl) + window.location.href = nextUrl }, 250)
559-733: Raw HTML elements used throughout instead of MUI components.The entire component renders with raw
<div>,<button>,<input>,<label>,<details>, and<p>elements styled with CSS classes. As per coding guidelines: "apps/**/*.{js,jsx,ts,tsx}: Always use MUI over HTML elements; avoid using CSS or tags."apps/ai-media/src/app/page.tsx (1)
9-16:formatLanguageLabelis duplicated fromLanguageSelector.tsx.This function is identical to
formatSlugFallbackinLanguageSelector.tsx(lines 8-17). Consider extracting it into a shared utility to avoid drift.apps/ai-media/src/app/CoverageReportClient.tsx (5)
1-19: Coding guidelines require MUI components instead of raw HTML elements.The entire file uses raw HTML elements (
<div>,<button>,<span>,<section>,<header>,<input>,<label>,<a>,<img>,<p>,<h2>) with CSS class-based styling. The repository coding guidelines forapps/**state: "Always use MUI over HTML elements; avoid using CSS or tags." Consider migrating to MUI components (Box,Button,Typography,Stack,Checkbox,Select, etc.) or documenting an explicit exception for this app if MUI is intentionally excluded. As per coding guidelines, "Always use MUI over HTML elements; avoid using CSS or tags."
459-540: Custom listbox missing keyboard navigation.The
ReportTypeSelectorusesrole="listbox"but lacks arrow-key navigation between options — a requirement of the WAI-ARIA listbox pattern. Keyboard-only users can only Tab through the options (or can't reach them at all if the dropdown renders conditionally without focus trapping). Consider using a native<select>, an MUISelect, or addingonKeyDownarrow-key handling withroving tabindex.
1257-1289: Consider extracting custom hooks for the complex state logic.
CoverageReportClientmanages ~15 state variables, multiple effects, and several memoized callbacks spanning ~470 lines. Consider extracting cohesive slices into custom hooks (e.g.,useCollectionCache,useVideoSelection,usePagination) to reduce cognitive load and improve testability.
1487-1493:handleTranslateis a stub — logs to console only.This is presumably a placeholder. Ensure a follow-up task tracks wiring this to the actual translation workflow.
Would you like me to open an issue to track implementing the actual translation workflow invocation?
83-83: 24-hour TTL withsessionStorageis effectively unreachable.
sessionStorageis cleared when the tab/window is closed, so aCACHE_TTL_MSof 24 hours will rarely take effect. If long-lived caching is the intent, consider usinglocalStorageinstead. If tab-scoped caching is the intent, the TTL constant is misleading and could be shortened or removed.
| const payload = await response.json() | ||
| const languages = payload?.data?.languages ?? [] |
There was a problem hiding this comment.
GraphQL errors are silently ignored.
The script checks response.ok but never inspects payload.errors or payload.data. If the gateway returns a 200 with GraphQL-level errors (partial or complete), the script will silently produce an empty or incomplete output file.
Suggested defensive check
const payload = await response.json()
+
+if (payload.errors?.length) {
+ console.error('GraphQL errors:', payload.errors.map(e => e.message).join(', '))
+ process.exit(1)
+}
+
const languages = payload?.data?.languages ?? []📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const payload = await response.json() | |
| const languages = payload?.data?.languages ?? [] | |
| const payload = await response.json() | |
| if (payload.errors?.length) { | |
| console.error('GraphQL errors:', payload.errors.map(e => e.message).join(', ')) | |
| process.exit(1) | |
| } | |
| const languages = payload?.data?.languages ?? [] |
🤖 Prompt for AI Agents
In `@apps/ai-media/scripts/export-country-language-populations.mjs` around lines
56 - 57, The GraphQL response payload is never checked for GraphQL-level errors
so payload.errors or a missing payload.data can silently produce an empty
languages array; update the code after awaiting response.json() to (1) check if
payload.errors exists and is non-empty and, if so, log the errors (include
payload.errors details) and exit/throw, and (2) verify payload.data and
payload.data.languages exist before using them (log a clear error and exit/throw
if missing) so the const languages = ... line only runs on a valid GraphQL data
response.
| function writeCachedCollections( | ||
| languageId: string, | ||
| collections: ClientCollection[] | ||
| ): CollectionsCacheMeta | null { | ||
| if (typeof window === 'undefined') return | ||
| try { | ||
| const timestamp = Date.now() | ||
| const dataKey = buildCollectionCacheKey(languageId, timestamp) | ||
| const metaKey = `${COLLECTION_CACHE_PREFIX}-${languageId}` | ||
| const meta: CollectionsCacheMeta = { | ||
| dataKey, | ||
| lastUpdated: timestamp, | ||
| expiresAt: timestamp + CACHE_TTL_MS | ||
| } | ||
| window.sessionStorage.setItem(dataKey, JSON.stringify(collections)) | ||
| window.sessionStorage.setItem(metaKey, JSON.stringify(meta)) | ||
| return meta | ||
| } catch (error) { | ||
| console.warn('Unable to write cached collections', error) | ||
| return null | ||
| } | ||
| } |
There was a problem hiding this comment.
Return type mismatch: undefined returned instead of null.
Line 150 has a bare return which yields undefined, but the declared return type is CollectionsCacheMeta | null. This will be a TypeScript error under strictNullChecks.
🐛 Proposed fix
- if (typeof window === 'undefined') return
+ if (typeof window === 'undefined') return null📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function writeCachedCollections( | |
| languageId: string, | |
| collections: ClientCollection[] | |
| ): CollectionsCacheMeta | null { | |
| if (typeof window === 'undefined') return | |
| try { | |
| const timestamp = Date.now() | |
| const dataKey = buildCollectionCacheKey(languageId, timestamp) | |
| const metaKey = `${COLLECTION_CACHE_PREFIX}-${languageId}` | |
| const meta: CollectionsCacheMeta = { | |
| dataKey, | |
| lastUpdated: timestamp, | |
| expiresAt: timestamp + CACHE_TTL_MS | |
| } | |
| window.sessionStorage.setItem(dataKey, JSON.stringify(collections)) | |
| window.sessionStorage.setItem(metaKey, JSON.stringify(meta)) | |
| return meta | |
| } catch (error) { | |
| console.warn('Unable to write cached collections', error) | |
| return null | |
| } | |
| } | |
| function writeCachedCollections( | |
| languageId: string, | |
| collections: ClientCollection[] | |
| ): CollectionsCacheMeta | null { | |
| if (typeof window === 'undefined') return null | |
| try { | |
| const timestamp = Date.now() | |
| const dataKey = buildCollectionCacheKey(languageId, timestamp) | |
| const metaKey = `${COLLECTION_CACHE_PREFIX}-${languageId}` | |
| const meta: CollectionsCacheMeta = { | |
| dataKey, | |
| lastUpdated: timestamp, | |
| expiresAt: timestamp + CACHE_TTL_MS | |
| } | |
| window.sessionStorage.setItem(dataKey, JSON.stringify(collections)) | |
| window.sessionStorage.setItem(metaKey, JSON.stringify(meta)) | |
| return meta | |
| } catch (error) { | |
| console.warn('Unable to write cached collections', error) | |
| return null | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@apps/ai-media/src/app/CoverageReportClient.tsx` around lines 146 - 167, The
function writeCachedCollections currently returns undefined when run on the
server (it does a bare return), which conflicts with its declared return type
CollectionsCacheMeta | null; update the early exit in writeCachedCollections
(the typeof window === 'undefined' branch) to return null instead of a bare
return so the function always returns either a CollectionsCacheMeta or null and
satisfies strict null checking.
| const helperText = isExplore | ||
| ? 'Click a segment to filter.' | ||
| : 'Click a segment to filter.' |
There was a problem hiding this comment.
Ternary always returns the same string.
Both branches produce 'Click a segment to filter.', making the conditional redundant. The select-mode branch likely needs distinct text (e.g., "Click a segment to select videos by status.").
Proposed fix
- const helperText = isExplore
- ? 'Click a segment to filter.'
- : 'Click a segment to filter.'
+ const helperText = isExplore
+ ? 'Click a segment to filter.'
+ : 'Click a segment to select videos by status.'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const helperText = isExplore | |
| ? 'Click a segment to filter.' | |
| : 'Click a segment to filter.' | |
| const helperText = isExplore | |
| ? 'Click a segment to filter.' | |
| : 'Click a segment to select videos by status.' |
🤖 Prompt for AI Agents
In `@apps/ai-media/src/app/CoverageReportClient.tsx` around lines 417 - 419, The
helperText ternary in CoverageReportClient.tsx (const helperText using
isExplore) currently returns the same string for both branches; change the else
branch to a distinct message for select mode (e.g., when isExplore is false
return "Click a segment to select videos by status.") so the UI communicates
filtering vs selecting behavior; update the const helperText (and any related
tooltip/label usages) to use the new select-mode string.
| return ( | ||
| <span | ||
| className={`tile tile--video tile--${status} tile--explore`} | ||
| title={title} | ||
| onMouseEnter={onHoverStart} | ||
| onMouseLeave={onHoverEnd} | ||
| onFocus={onHoverStart} | ||
| onBlur={onHoverEnd} | ||
| > | ||
| <span className="tile-checkbox" aria-hidden="true"> | ||
| <span className="tile-checkbox-box" /> | ||
| </span> | ||
| </span> | ||
| ) |
There was a problem hiding this comment.
Non-focusable <span> has onFocus/onBlur handlers that will never fire.
Without tabIndex={0}, the <span> can't receive focus, so the onFocus/onBlur handlers are dead code. Either add tabIndex={0} and a meaningful role so keyboard users can trigger hover details, or remove the focus/blur handlers.
Proposed fix (make focusable)
<span
className={`tile tile--video tile--${status} tile--explore`}
title={title}
+ tabIndex={0}
+ role="img"
+ aria-label={title}
onMouseEnter={onHoverStart}
onMouseLeave={onHoverEnd}
onFocus={onHoverStart}
onBlur={onHoverEnd}
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return ( | |
| <span | |
| className={`tile tile--video tile--${status} tile--explore`} | |
| title={title} | |
| onMouseEnter={onHoverStart} | |
| onMouseLeave={onHoverEnd} | |
| onFocus={onHoverStart} | |
| onBlur={onHoverEnd} | |
| > | |
| <span className="tile-checkbox" aria-hidden="true"> | |
| <span className="tile-checkbox-box" /> | |
| </span> | |
| </span> | |
| ) | |
| return ( | |
| <span | |
| className={`tile tile--video tile--${status} tile--explore`} | |
| title={title} | |
| tabIndex={0} | |
| role="img" | |
| aria-label={title} | |
| onMouseEnter={onHoverStart} | |
| onMouseLeave={onHoverEnd} | |
| onFocus={onHoverStart} | |
| onBlur={onHoverEnd} | |
| > | |
| <span className="tile-checkbox" aria-hidden="true"> | |
| <span className="tile-checkbox-box" /> | |
| </span> | |
| </span> | |
| ) |
🤖 Prompt for AI Agents
In `@apps/ai-media/src/app/CoverageReportClient.tsx` around lines 754 - 767, The
span returned in CoverageReportClient.tsx is using onFocus/onBlur but isn't
focusable; make the element keyboard-accessible by adding tabIndex={0} and an
appropriate role (e.g., role="button" or role="group") to the outer <span> that
has onMouseEnter/onMouseLeave/onFocus/onBlur so those handlers can fire for
keyboard users; optionally also consider adding onKeyDown to invoke
onHoverStart/onHoverEnd for Enter/Space if it should behave like a button.
| <section | ||
| className="collection-card" | ||
| key={collection.id} | ||
| tabIndex={0} | ||
| role="button" | ||
| aria-expanded={isExpanded} | ||
| onKeyDown={(event) => { | ||
| if (event.key === 'Enter' || event.key === ' ') { | ||
| event.preventDefault() | ||
| onToggleExpanded(collection.id) | ||
| } | ||
| }} | ||
| onClick={(event) => { | ||
| const target = event.target as HTMLElement | ||
| if (target.closest('a, button, input, select, textarea')) return | ||
| if (target.closest('.tile')) return | ||
| onToggleExpanded(collection.id) | ||
| }} |
There was a problem hiding this comment.
Accessibility anti-pattern: role="button" on a container with nested interactive elements.
A <section> with role="button" makes screen readers announce the entire card (including its nested buttons, links, and checkboxes) as a single button. This makes the nested controls unreachable or confusing for assistive technology users. The expand/collapse toggle already exists as a dedicated button (line 1101). Remove role="button" and tabIndex={0} from the section, and rely on the existing toggle button for keyboard access.
Proposed fix
<section
className="collection-card"
key={collection.id}
- tabIndex={0}
- role="button"
aria-expanded={isExpanded}
- onKeyDown={(event) => {
- if (event.key === 'Enter' || event.key === ' ') {
- event.preventDefault()
- onToggleExpanded(collection.id)
- }
- }}
- onClick={(event) => {
- const target = event.target as HTMLElement
- if (target.closest('a, button, input, select, textarea')) return
- if (target.closest('.tile')) return
- onToggleExpanded(collection.id)
- }}
>🤖 Prompt for AI Agents
In `@apps/ai-media/src/app/CoverageReportClient.tsx` around lines 1016 - 1033, The
section element rendering the collection card (className="collection-card")
should not be treated as a button — remove role="button" and tabIndex={0} from
that <section> and stop using onKeyDown on it; keep the existing dedicated
expand/collapse button (the button tied to onToggleExpanded and isExpanded
around line ~1101) as the only keyboard-accessible control for expand/collapse.
Ensure the onClick handler on the section no longer invokes
onToggleExpanded(collection.id) for plain clicks that should be handled by the
inner toggle button (you may keep click guard logic for links/inputs/tiles but
do not call onToggleExpanded from the section), and leave the named handlers
(onClick, onKeyDown, onToggleExpanded, collection.id, isExpanded) intact
elsewhere so the explicit button continues to manage expansion.
| const handleToggleCollection = useCallback((collection: ClientCollection) => { | ||
| const collectionIds = collection.videos.map((video) => video.id) | ||
| const allSelected = collectionIds.every((id) => selectedSet.has(id)) | ||
| if (allSelected) { | ||
| setSelectedIds((prev) => prev.filter((id) => !collectionIds.includes(id))) | ||
| } else { | ||
| setSelectedIds((prev) => Array.from(new Set([...prev, ...collectionIds]))) | ||
| } | ||
| }, [selectedSet]) |
There was a problem hiding this comment.
Stale closure: toggle decision based on potentially outdated selectedSet.
allSelected is computed from the captured selectedSet, but setSelectedIds uses a functional updater. If multiple collection toggles are batched before a re-render, the second call's allSelected check may be stale, causing it to add when it should remove (or vice versa). Move the allSelected check inside the updater callback to use the latest prev state.
Proposed fix
const handleToggleCollection = useCallback((collection: ClientCollection) => {
const collectionIds = collection.videos.map((video) => video.id)
- const allSelected = collectionIds.every((id) => selectedSet.has(id))
- if (allSelected) {
- setSelectedIds((prev) => prev.filter((id) => !collectionIds.includes(id)))
- } else {
- setSelectedIds((prev) => Array.from(new Set([...prev, ...collectionIds])))
- }
- }, [selectedSet])
+ setSelectedIds((prev) => {
+ const prevSet = new Set(prev)
+ const allSelected = collectionIds.every((id) => prevSet.has(id))
+ if (allSelected) {
+ return prev.filter((id) => !collectionIds.includes(id))
+ }
+ return Array.from(new Set([...prev, ...collectionIds]))
+ })
+ }, [])The same pattern applies to handleSelectByStatus (lines 1447-1455) and handleSelectByStatusInCollection (lines 1457-1471).
🤖 Prompt for AI Agents
In `@apps/ai-media/src/app/CoverageReportClient.tsx` around lines 1437 - 1445,
handleToggleCollection, handleSelectByStatus, and
handleSelectByStatusInCollection compute selection decisions using the captured
selectedSet, causing stale-toggle bugs when multiple updates are batched; change
each to compute whether to add/remove inside the setSelectedIds functional
updater (use the incoming prev state) instead of using selectedSet, e.g., move
the allSelected or status-check logic into the updater callback and then return
the new array (using Set semantics to dedupe) so the decision is based on the
latest prev state.
| const handleSelectByStatusInCollection = useCallback(( | ||
| collection: ClientCollection, | ||
| status: CoverageStatus | ||
| ) => { | ||
| const statusIds = collection.videos | ||
| .filter((video) => video.subtitleStatus === status) | ||
| .map((video) => video.id) | ||
| const allSelected = | ||
| statusIds.length > 0 && statusIds.every((id) => selectedSet.has(id)) | ||
| if (allSelected) { | ||
| setSelectedIds((prev) => prev.filter((id) => !statusIds.includes(id))) | ||
| return | ||
| } | ||
| setSelectedIds((prev) => Array.from(new Set([...prev, ...statusIds]))) | ||
| }, [selectedSet]) |
There was a problem hiding this comment.
Bug: handleSelectByStatusInCollection always filters by subtitleStatus regardless of active report type.
Line 1462 uses video.subtitleStatus === status, but for voiceover/meta reports it should use the report-specific status. This causes the wrong set of videos to be selected when operating in non-subtitle report modes.
🐛 Proposed fix
const handleSelectByStatusInCollection = useCallback((
collection: ClientCollection,
status: CoverageStatus
) => {
const statusIds = collection.videos
- .filter((video) => video.subtitleStatus === status)
+ .filter((video) => getReportStatus(video) === status)
.map((video) => video.id)Note: getReportStatus reads reportType from the closure — you'll need to either add it to the dependency array or inline the logic using a reportType parameter.
🤖 Prompt for AI Agents
In `@apps/ai-media/src/app/CoverageReportClient.tsx` around lines 1457 - 1471, The
handler handleSelectByStatusInCollection currently compares video.subtitleStatus
only, causing wrong selections for non-subtitle reports; change the filter to
use the report-aware status (e.g., getReportStatus(video) === status) or pass
reportType into the handler and compare against the appropriate field for that
type; also update the useCallback dependency array to include reportType (or
remove closure use by accepting reportType as a parameter) so the memoized
function reflects the active report mode and selectedSet remains correct.
| <button | ||
| type="button" | ||
| className={`geo-filter-button${ | ||
| draftContinents.has(continent.id) ? 'is-active' : '' | ||
| }`} | ||
| onClick={(event) => { | ||
| event.preventDefault() | ||
| event.stopPropagation() | ||
| toggleContinent(continent.id) | ||
| }} | ||
| aria-pressed={draftContinents.has(continent.id)} | ||
| > | ||
| {continent.name} | ||
| </button> | ||
| <span className="geo-accordion-count"> | ||
| {countries.length} | ||
| </span> | ||
| </summary> | ||
| <div className="geo-accordion-panel"> | ||
| {countries.map((country) => ( | ||
| <button | ||
| key={country.id} | ||
| type="button" | ||
| className={`geo-filter-button geo-filter-button--country${ | ||
| draftCountries.has(country.id) ? 'is-active' : '' | ||
| }`} | ||
| onClick={() => toggleCountry(country.id)} | ||
| aria-pressed={draftCountries.has(country.id)} |
There was a problem hiding this comment.
Missing space before is-active class — active state styling is broken.
Both the continent and country button className templates concatenate 'is-active' directly onto the base class without a separating space:
- Line 616: produces
"geo-filter-buttonis-active"instead of"geo-filter-button is-active" - Line 637: produces
"geo-filter-button geo-filter-button--countryis-active"instead of"geo-filter-button geo-filter-button--country is-active"
This means selected continent/country filters will never receive their active styling.
Proposed fix
className={`geo-filter-button${
- draftContinents.has(continent.id) ? 'is-active' : ''
+ draftContinents.has(continent.id) ? ' is-active' : ''
}`} className={`geo-filter-button geo-filter-button--country${
- draftCountries.has(country.id) ? 'is-active' : ''
+ draftCountries.has(country.id) ? ' is-active' : ''
}`}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <button | |
| type="button" | |
| className={`geo-filter-button${ | |
| draftContinents.has(continent.id) ? 'is-active' : '' | |
| }`} | |
| onClick={(event) => { | |
| event.preventDefault() | |
| event.stopPropagation() | |
| toggleContinent(continent.id) | |
| }} | |
| aria-pressed={draftContinents.has(continent.id)} | |
| > | |
| {continent.name} | |
| </button> | |
| <span className="geo-accordion-count"> | |
| {countries.length} | |
| </span> | |
| </summary> | |
| <div className="geo-accordion-panel"> | |
| {countries.map((country) => ( | |
| <button | |
| key={country.id} | |
| type="button" | |
| className={`geo-filter-button geo-filter-button--country${ | |
| draftCountries.has(country.id) ? 'is-active' : '' | |
| }`} | |
| onClick={() => toggleCountry(country.id)} | |
| aria-pressed={draftCountries.has(country.id)} | |
| <button | |
| type="button" | |
| className={`geo-filter-button${ | |
| draftContinents.has(continent.id) ? ' is-active' : '' | |
| }`} | |
| onClick={(event) => { | |
| event.preventDefault() | |
| event.stopPropagation() | |
| toggleContinent(continent.id) | |
| }} | |
| aria-pressed={draftContinents.has(continent.id)} | |
| > | |
| {continent.name} | |
| </button> | |
| <span className="geo-accordion-count"> | |
| {countries.length} | |
| </span> | |
| </summary> | |
| <div className="geo-accordion-panel"> | |
| {countries.map((country) => ( | |
| <button | |
| key={country.id} | |
| type="button" | |
| className={`geo-filter-button geo-filter-button--country${ | |
| draftCountries.has(country.id) ? ' is-active' : '' | |
| }`} | |
| onClick={() => toggleCountry(country.id)} | |
| aria-pressed={draftCountries.has(country.id)} |
🤖 Prompt for AI Agents
In `@apps/ai-media/src/app/LanguageGeoSelector.tsx` around lines 614 - 641, The
className template strings for the continent and country buttons in
LanguageGeoSelector are concatenating 'is-active' without a leading space,
breaking styling; update the continent button's className (where
draftContinents.has(continent.id) is checked) and the country button's className
(where draftCountries.has(country.id) is checked) so the conditional inserts "
is-active" (with a leading space) instead of "is-active" when true (e.g., change
the string to `' is-active'` or otherwise ensure a separating space is
included); no other logic changes are needed (toggleContinent, toggleCountry,
draftContinents, draftCountries remain the same).
| const payload = (await response.json()) as string[][] | ||
|
|
||
| const nextOptions = payload.map((entry) => { | ||
| const [languageIdSlugNative, ...names] = entry | ||
| const [id] = languageIdSlugNative.split(':') | ||
| const englishName = | ||
| names.find((name) => name.startsWith('529:'))?.split(':')[1] ?? '' | ||
| const nativeLabel = | ||
| languageIdSlugNative.split(':').slice(2).join(':') ?? '' | ||
|
|
||
| return { | ||
| id, | ||
| englishLabel: englishName.trim() || formatSlugFallback(id), | ||
| nativeLabel: nativeLabel.trim() | ||
| } | ||
| }) | ||
| .filter( | ||
| (language) => | ||
| language.englishLabel.trim().length > 0 && | ||
| !/^\d+$/.test(language.englishLabel.trim()) | ||
| ) | ||
| .sort((a, b) => a.englishLabel.localeCompare(b.englishLabel)) |
There was a problem hiding this comment.
API response type mismatch — dynamic fetch always fails.
The /api/languages route returns a GeoPayload object ({ continents, countries, languages }), but this code casts the response as string[][] and calls .map() on it. Since plain objects don't have .map(), this throws a TypeError that is silently caught at line 138, making the component always fall back to the initial options prop.
Compare with LanguageGeoSelector.tsx line 279 which correctly types the same endpoint response as GeoPayload.
Proposed fix — parse GeoPayload correctly
- const payload = (await response.json()) as string[][]
-
- const nextOptions = payload.map((entry) => {
- const [languageIdSlugNative, ...names] = entry
- const [id] = languageIdSlugNative.split(':')
- const englishName =
- names.find((name) => name.startsWith('529:'))?.split(':')[1] ?? ''
- const nativeLabel =
- languageIdSlugNative.split(':').slice(2).join(':') ?? ''
-
- return {
- id,
- englishLabel: englishName.trim() || formatSlugFallback(id),
- nativeLabel: nativeLabel.trim()
- }
- })
- .filter(
- (language) =>
- language.englishLabel.trim().length > 0 &&
- !/^\d+$/.test(language.englishLabel.trim())
- )
- .sort((a, b) => a.englishLabel.localeCompare(b.englishLabel))
+ const payload = (await response.json()) as {
+ languages?: Array<{
+ id: string
+ englishLabel: string
+ nativeLabel: string
+ }>
+ }
+
+ const nextOptions = (payload.languages ?? [])
+ .map((language) => ({
+ id: language.id,
+ englishLabel:
+ language.englishLabel.trim() || formatSlugFallback(language.id),
+ nativeLabel: language.nativeLabel.trim()
+ }))
+ .filter(
+ (language) =>
+ language.englishLabel.length > 0 &&
+ !/^\d+$/.test(language.englishLabel)
+ )
+ .sort((a, b) => a.englishLabel.localeCompare(b.englishLabel))🤖 Prompt for AI Agents
In `@apps/ai-media/src/app/LanguageSelector.tsx` around lines 112 - 133, The code
incorrectly casts the fetch result to string[][] and calls .map(), causing a
TypeError; instead parse the response as the GeoPayload shape and iterate
payload.languages (not payload directly) when building nextOptions. Update the
type assertion on the response (replace string[][] with GeoPayload), extract the
languages array (e.g., const { languages } = payload), then map over languages
to produce nextOptions using the existing logic (preserve id, englishLabel,
nativeLabel, filtering and sorting) so it matches the pattern used in
LanguageGeoSelector.tsx and avoids the silent fallback.
| const payload = (await response.json()) as CollectionsResponse | ||
|
|
||
| if (!response.ok || payload.errors?.length) { | ||
| const message = | ||
| payload.errors?.map((error) => error.message).join(', ') ?? | ||
| 'Unknown error' | ||
| throw new Error(message) | ||
| } |
There was a problem hiding this comment.
Parse JSON after checking response.ok to avoid misleading errors.
response.json() is called on line 317 before response.ok is checked on line 319. If the gateway returns a non-OK response with a non-JSON body (e.g., a 502 HTML error page from a load balancer), response.json() throws a SyntaxError. While the outer try-catch handles it, the resulting error message will be a confusing JSON parse error instead of the actual HTTP status.
Proposed fix
+ if (!response.ok) {
+ throw new Error(`Gateway responded with ${response.status}`)
+ }
+
const payload = (await response.json()) as CollectionsResponse
- if (!response.ok || payload.errors?.length) {
- const message =
- payload.errors?.map((error) => error.message).join(', ') ??
- 'Unknown error'
+ if (payload.errors?.length) {
+ const message = payload.errors.map((error) => error.message).join(', ')
throw new Error(message)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const payload = (await response.json()) as CollectionsResponse | |
| if (!response.ok || payload.errors?.length) { | |
| const message = | |
| payload.errors?.map((error) => error.message).join(', ') ?? | |
| 'Unknown error' | |
| throw new Error(message) | |
| } | |
| if (!response.ok) { | |
| throw new Error(`Gateway responded with ${response.status}`) | |
| } | |
| const payload = (await response.json()) as CollectionsResponse | |
| if (payload.errors?.length) { | |
| const message = payload.errors.map((error) => error.message).join(', ') | |
| throw new Error(message) | |
| } |
🤖 Prompt for AI Agents
In `@apps/ai-media/src/app/page.tsx` around lines 317 - 324, Move the
response.json() call to after validating response.ok so JSON parse errors don't
mask HTTP errors: first check response.ok on the response object and if it's not
ok read response.text() (or include response.status and statusText) and throw a
descriptive Error with HTTP status; only then call await response.json() to
populate payload and continue the existing payload.errors check (refer to the
response and payload variables in this block inside page.tsx).
1 participant
Motivation
@mux/ai/primitivesand a small Workflow DevKit.Description
apps/ai-mediawith app layout, styles, project config, lint/test/type-check targets, and a documentation UI page listing workflow stages.src/workflows/devkit.tsand a three-stagesubtitlesworkflow split into clear files:step1/generateSubtitles.ts,step2/postProcessSubtitles.ts, andstep3/uploadSubtitles.ts, plusworkflow.tsandindex.ts.src/types/mux-ai.d.tsandsrc/workflows/subtitles/types.ts, and aplan.tsdescribing stages; tests live insrc/workflows/subtitles/__tests__.@mux/aito workspace dependencies and updatepackage.json/pnpm-lock.yaml, and include a PRDprds/AI-MEDIA-SUBTITLES.md; minor ESLint/tsconfig adjustments made so the new app builds and tests cleanly.Testing
pnpm exec nx lint ai-mediaand it passed successfully.pnpm exec nx type-check ai-mediaand it passed successfully.pnpm exec nx test ai-mediaand the subtitles workflow test passed (mocked@mux/ai/primitives).pnpm exec nx serve ai-media(available athttp://localhost:4900) and captured a screenshot of the workflow page to validate rendering.Codex Task
Summary by CodeRabbit
New Features
Documentation
Tests
Local dev