fix: missing familyRelative field in screening#1534
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
📝 WalkthroughWalkthroughAdds a FamilyRelativeEntity type, makes FamilyDetail generic with a relation discriminator, updates entity selection and label rendering (caption → alias/name fallback), and passes relation to FamilyDetail from MatchDetails. ChangesFamily Relative Matching Support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/app-builder/src/components/Screenings/MatchCard/Associations.tsx (1)
15-15:⚠️ Potential issue | 🟠 Major | ⚖️ Poor tradeoffEarly null check prevents rendering caption-only persons.
The guard
if (!properties?.name?.[0]) return null;on line 15 returns before reaching the caption rendering logic on line 34. Since the data model allows persons with acaptionbut nonamefield, this silently skips valid entities that should be displayable. The same issue exists in FamilyDetail.tsx.Adjust the check to:
if (!properties?.name?.[0] && !properties?.caption?.length)to only skip persons missing both name and caption. Or move the check after the caption condition if caption-only rendering is the intended fallback.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/app-builder/src/components/Screenings/MatchCard/Associations.tsx` at line 15, The early guard in Associations.tsx currently returns when properties?.name?.[0] is missing, which prevents rendering entities that have only a caption; update the check so it only returns when both name and caption are absent — replace `if (!properties?.name?.[0]) return null;` with `if (!properties?.name?.[0] && !properties?.caption?.length) return null;` (or alternatively move that guard below the caption-rendering branch) so caption-only persons are rendered; apply the same change in FamilyDetail.tsx where the identical guard exists.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/app-builder/src/components/Screenings/MatchCard/FamilyDetail.tsx`:
- Line 36: The current early-return in FamilyDetail.tsx (the guard checking
properties.name?.[0]) wrongly drops valid PersonEntity objects that only have a
required caption; update the guard so it only returns null when there is truly
nothing to render: check for caption or fallback name/alias (e.g., if
(!properties.caption && !properties.name?.[0] && !properties.alias?.[0]) return
null) or remove the name-only guard and let the existing render logic in the
component (which handles caption/name/alias display) decide what to show; locate
the guard around properties.name, and modify it accordingly in the FamilyDetail
component.
In `@packages/app-builder/src/models/screening.ts`:
- Line 100: The property name endData is a typo; rename it to endDate in the
model so it matches the parallel FamilyPersonEntity shape and what FamilyDetail
expects (member.properties.endDate). Update the type/interface that currently
declares endData?: string[] (and any usages/imports referencing that property)
to use endDate?: string[] to restore correct end date wiring to the UI.
---
Outside diff comments:
In `@packages/app-builder/src/components/Screenings/MatchCard/Associations.tsx`:
- Line 15: The early guard in Associations.tsx currently returns when
properties?.name?.[0] is missing, which prevents rendering entities that have
only a caption; update the check so it only returns when both name and caption
are absent — replace `if (!properties?.name?.[0]) return null;` with `if
(!properties?.name?.[0] && !properties?.caption?.length) return null;` (or
alternatively move that guard below the caption-rendering branch) so
caption-only persons are rendered; apply the same change in FamilyDetail.tsx
where the identical guard exists.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f6527ef8-03e6-4d16-8648-08e9dcd4544f
📒 Files selected for processing (4)
packages/app-builder/src/components/Screenings/MatchCard/Associations.tsxpackages/app-builder/src/components/Screenings/MatchCard/FamilyDetail.tsxpackages/app-builder/src/components/Screenings/MatchDetails.tsxpackages/app-builder/src/models/screening.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: e2e
- GitHub Check: check / main
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Import internal app-builder models using the path alias@app-builder/models/*
Import UI components fromui-design-systemusing direct imports likeimport { Button, Modal, Select, cn } from 'ui-design-system'
Use TanStack Query hooks for server state management, importing from@app-builder/queries/*
Use TanStack Form for form handling
Use Zod for schema validation
Files:
packages/app-builder/src/components/Screenings/MatchDetails.tsxpackages/app-builder/src/components/Screenings/MatchCard/Associations.tsxpackages/app-builder/src/models/screening.tspackages/app-builder/src/components/Screenings/MatchCard/FamilyDetail.tsx
packages/app-builder/src/components/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
Organize app-builder source code into feature-specific component folders (Cases/, Decisions/, etc.) within the components directory
Files:
packages/app-builder/src/components/Screenings/MatchDetails.tsxpackages/app-builder/src/components/Screenings/MatchCard/Associations.tsxpackages/app-builder/src/components/Screenings/MatchCard/FamilyDetail.tsx
**/*.{ts,tsx,js,jsx,json,css}
📄 CodeRabbit inference engine (CLAUDE.md)
Run
bun run format:writeto format all files according to Biome standards
Files:
packages/app-builder/src/components/Screenings/MatchDetails.tsxpackages/app-builder/src/components/Screenings/MatchCard/Associations.tsxpackages/app-builder/src/models/screening.tspackages/app-builder/src/components/Screenings/MatchCard/FamilyDetail.tsx
packages/app-builder/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Run
bun run type-checkin app-builder and ui-design-system packages to verify TypeScript types
Files:
packages/app-builder/src/components/Screenings/MatchDetails.tsxpackages/app-builder/src/components/Screenings/MatchCard/Associations.tsxpackages/app-builder/src/models/screening.tspackages/app-builder/src/components/Screenings/MatchCard/FamilyDetail.tsx
packages/app-builder/src/models/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use type definitions with adapters in the models directory
Files:
packages/app-builder/src/models/screening.ts
🧠 Learnings (2)
📚 Learning: 2026-05-11T13:00:53.337Z
Learnt from: william-schlegel
Repo: checkmarble/marble-frontend PR: 1503
File: packages/app-builder/src/components/ContinuousScreening/context/ListAndTopicDatasetConfigurationBridge.tsx:13-20
Timestamp: 2026-05-11T13:00:53.337Z
Learning: In checkmarble/marble-frontend, calls to `createSharp` from the `sharpstate` library should be treated as if they were a React hook. In React `.tsx` components, call `createSharp` unconditionally at the top level of the component function body (not inside conditionals or nested functions). Do not place `createSharp` inside `useMemo`, `useCallback`, `useEffect`, or any other hook, and do not suggest wrapping it in `useMemo`—that is incorrect and should be flagged during review.
Applied to files:
packages/app-builder/src/components/Screenings/MatchDetails.tsxpackages/app-builder/src/components/Screenings/MatchCard/Associations.tsxpackages/app-builder/src/components/Screenings/MatchCard/FamilyDetail.tsx
📚 Learning: 2026-05-12T19:51:39.619Z
Learnt from: Pascal-Delange
Repo: checkmarble/marble-frontend PR: 1522
File: packages/app-builder/src/components/Cases/CaseAlerts.tsx:449-449
Timestamp: 2026-05-12T19:51:39.619Z
Learning: In React (.tsx) files, when rendering translated strings that include dynamic count values, always use i18n interpolation rather than appending the count as a separate raw React text node. Prefer `t('translation.key', { count })` (or the project’s equivalent) and include `{{count}}` (or the interpolation placeholder expected by the i18n setup) inside the translation string so each locale controls placement/order. Avoid patterns like `t('key') + ' (' + count + ')'` or rendering `t('key')` followed by `(${count})` as separate nodes, since this can break RTL layout (e.g., Arabic).
Applied to files:
packages/app-builder/src/components/Screenings/MatchDetails.tsxpackages/app-builder/src/components/Screenings/MatchCard/Associations.tsxpackages/app-builder/src/components/Screenings/MatchCard/FamilyDetail.tsx
🔇 Additional comments (2)
packages/app-builder/src/components/Screenings/MatchDetails.tsx (1)
98-104: LGTM!packages/app-builder/src/components/Screenings/MatchCard/FamilyDetail.tsx (1)
33-33: Remove this suggestion—the type assertion is necessary and correct.TypeScript cannot infer the type without the assertion because it cannot correlate the runtime
relationcheck with the actual type ofmemberin the union. WhileFamilyPersonEntity.properties.relativeandFamilyRelativeEntity.properties.personare both typed asPersonEntity[], they're optional properties (PersonEntity[] | undefined). The assertion handles both the optionality and the type correlation that TypeScript's type narrowing alone cannot resolve in this context.> Likely an incorrect or invalid review comment.
| const entities = member.properties[relation === 'familyPerson' ? 'relative' : 'person'] as PersonEntity[]; | ||
|
|
||
| return entities?.map(({ id, properties }, idx) => { | ||
| if (!properties.name?.[0]) return null; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for PersonEntity usage patterns to understand if name is always required
rg -nP --type=ts -C3 'PersonEntity.*caption' packages/app-builder/src/Repository: checkmarble/marble-frontend
Length of output: 53
🏁 Script executed:
# First, let's read the FamilyDetail.tsx file to see the actual code context
cat -n packages/app-builder/src/components/Screenings/MatchCard/FamilyDetail.tsx | head -70Repository: checkmarble/marble-frontend
Length of output: 3961
🏁 Script executed:
# Search for the type definition of properties/PersonEntity to understand if name is required
fd -e ts -e tsx . packages/app-builder/src | xargs rg -l 'interface.*PersonEntity|type.*PersonEntity' | head -5Repository: checkmarble/marble-frontend
Length of output: 198
🏁 Script executed:
# Look for caption usage patterns to see if caption-only entities exist
rg -n --type=ts 'caption' packages/app-builder/src/components/Screenings/MatchCard/FamilyDetail.tsx -B2 -A2Repository: checkmarble/marble-frontend
Length of output: 531
🏁 Script executed:
# Read the PersonEntity type definition
rg -n 'type PersonEntity|interface PersonEntity' packages/app-builder/src/models/screening.ts -A20Repository: checkmarble/marble-frontend
Length of output: 658
🏁 Script executed:
# Also check if there are any tests or examples showing caption-only entities
rg -n 'caption.*:' packages/app-builder/src/models/screening.ts | head -10Repository: checkmarble/marble-frontend
Length of output: 164
Fix early return to allow caption-only entities.
The guard at line 36 filters out any entity missing name[0], but the PersonEntity type defines caption as required and name as optional. An entity with a caption but no name is valid, yet the current early return prevents it from rendering. The rendering logic (lines 50–56) correctly handles caption-first display, so entities should not be filtered solely on missing name.
Consider adjusting the guard to check for caption or fallback fields:
if (!properties.caption && !properties.name?.[0] && !properties.alias?.[0]) return null;
Or, if caption is guaranteed to exist (per the type), simply rely on the conditional rendering to handle name/alias fallback without the name-specific guard.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/app-builder/src/components/Screenings/MatchCard/FamilyDetail.tsx` at
line 36, The current early-return in FamilyDetail.tsx (the guard checking
properties.name?.[0]) wrongly drops valid PersonEntity objects that only have a
required caption; update the guard so it only returns null when there is truly
nothing to render: check for caption or fallback name/alias (e.g., if
(!properties.caption && !properties.name?.[0] && !properties.alias?.[0]) return
null) or remove the name-only guard and let the existing render logic in the
component (which handles caption/name/alias display) decide what to show; locate
the guard around properties.name, and modify it accordingly in the FamilyDetail
component.
| schema: 'Family'; | ||
| properties: { | ||
| person?: PersonEntity[]; | ||
| endData?: string[]; |
There was a problem hiding this comment.
Typo in property name: endData should be endDate.
The property is named endData but should be endDate to match the parallel FamilyPersonEntity structure (line 88) and standard date field naming. This typo will prevent end dates from displaying in the UI, as FamilyDetail expects member.properties.endDate (see FamilyDetail.tsx line 63).
🔧 Proposed fix
- endData?: string[];
+ endDate?: string[];📝 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.
| endData?: string[]; | |
| endDate?: string[]; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/app-builder/src/models/screening.ts` at line 100, The property name
endData is a typo; rename it to endDate in the model so it matches the parallel
FamilyPersonEntity shape and what FamilyDetail expects
(member.properties.endDate). Update the type/interface that currently declares
endData?: string[] (and any usages/imports referencing that property) to use
endDate?: string[] to restore correct end date wiring to the UI.
| <div className="text-sm text-grey-70 font-medium">{properties.caption}</div> | ||
| ) : ( | ||
| <div className="col-span-full flex w-full flex-wrap gap-1"> | ||
| <span>{properties.alias?.[0] ?? properties.name?.[0]}</span> |
There was a problem hiding this comment.
The opposite please: if name caption is not there but name is, let's show name
ChibiBlasphem
force-pushed
the
fix/missing-enriched-screening-data-display
branch
from
38d5992 to
d669c67
Compare
2 participants
Summary by CodeRabbit
Improvements
New Features