Feat/mar 1888 manual search results saving and recall#1555
Conversation
184bc47 to
a3e90a2
Compare
|
Warning Review limit reached
More reviews will be available in 55 minutes and 17 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (65)
📝 WalkthroughWalkthroughThis PR refactors i18n infrastructure to derive language state from ChangesI18n Provider and Localization Infrastructure
Dataset/Topic Selection Core Refactoring
UI Component Library Enhancements
Freeform Search Refactoring and Saved Search Feature
Screening Category and Data Model Updates
Continuous Screening and Data Visualization
Localization and Route Providers
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes This PR introduces substantial interconnected changes across i18n infrastructure, dataset/topic selection mechanics (composite keys throughout), multiple new UI components, complete freeform search refactoring with saved search persistence, and screening category system extensions. The complexity stems from the pervasive impact of the key refactoring on many dependent layers, numerous cross-file coordinations, and the interplay between form context, language awareness, and selection state management. Possibly related PRs
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
|
There was a problem hiding this comment.
Actionable comments posted: 11
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/FreeformSearch/DatasetsPopover.tsx (1)
94-94:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse i18n interpolation for the count/empty suffix instead of string concatenation.
At Line 94, concatenating
(${count})/(${...})aftert(...)can break RTL ordering; move these into translation keys with interpolation ({ count }) so each locale controls placement.Based on learnings: “when rendering translated strings that include dynamic count values, always use i18n interpolation … avoid rendering
t('key')followed by(${count})as separate nodes, since this can break RTL layout.”🤖 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/FreeformSearch/DatasetsPopover.tsx` at line 94, The JSX currently concatenates a translated base string with a count suffix (`{isEmpty ? \` (${t('scenarios:sanction.lists.no_lists_selected')})\` : \` (${count})\`}`) which breaks RTL; change this to use i18n interpolation keys so locale controls placement. Create/consume translation keys that include the suffix with an interpolated value (e.g. `scenarios:sanction.lists.selected_count` -> `({count})` or a localized equivalent, and `scenarios:sanction.lists.no_lists_selected_count` for the empty case), then replace the concatenation with a single `t(...)` call passing `{ count }` (or no vars for the explicit empty key) where `isEmpty` chooses the appropriate key; update the JSX in DatasetsPopover.tsx to call those keys instead of concatenating `t(...)` + `(${count})`.
🧹 Nitpick comments (6)
packages/app-builder/src/components/Screenings/FreeformSearch/entity-search-form-context.tsx (1)
45-47: ⚡ Quick winTighten form/store typing instead of casting through
unknown/never.These casts hide integration errors that TypeScript could catch at compile-time. A typed provider contract here will save thee from subtle runtime woes.
Suggested typing hardening
-import { useStore } from '`@tanstack/react-form`'; +import { type FormApi, useStore } from '`@tanstack/react-form`'; @@ -export function EntitySearchFormProvider({ form, children }: { form: unknown; children: ReactNode }) { +export function EntitySearchFormProvider({ + form, + children, +}: { + form: EntitySearchFormInstance; + children: ReactNode; +}) { return ( - <EntitySearchFormContext.Provider value={form as EntitySearchFormInstance}> + <EntitySearchFormContext.Provider value={form}> {children} </EntitySearchFormContext.Provider> ); } @@ export function useEntitySearchFormStore<TSelected>(selector: (state: EntitySearchFormState) => TSelected): TSelected { const { store } = useEntitySearchForm(); - return useStore(store as never, selector); + return useStore(store as FormApi<EntitySearchFormValues>['store'], selector); }Also applies to: 57-59
🤖 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/FreeformSearch/entity-search-form-context.tsx` around lines 45 - 47, Change the loose typing on EntitySearchFormProvider to accept a properly typed form prop instead of using unknown/never casts: update the provider signature to require the concrete interface (EntitySearchFormInstance) for the form param and ensure children is typed as ReactNode, then pass the form directly into EntitySearchFormContext.Provider without "as" casting; likewise, replace similar casts at the other occurrence (the block around lines that referenced the same pattern) so all uses consistently accept and expose EntitySearchFormInstance rather than unknown/never.packages/app-builder/src/server-fns/continuous-screening.ts (1)
2-2: ⚡ Quick winImport
sanitizeTruthyDatasetsfrom a utility module, not a component barrel.For server-fns, keep imports on server-safe utility boundaries to avoid accidental UI coupling as the barrel evolves.
Suggested import adjustment
-import { sanitizeTruthyDatasets } from '`@app-builder/components/ListAndTopicConfiguration`'; +import { sanitizeTruthyDatasets } from '`@app-builder/components/ListAndTopicConfiguration/dataset-utils`';🤖 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/server-fns/continuous-screening.ts` at line 2, The import of sanitizeTruthyDatasets in continuous-screening.ts currently comes from the component barrel '`@app-builder/components/ListAndTopicConfiguration`', which may pull UI code into server fns; change the import to the server-safe utility module that actually exports sanitizeTruthyDatasets (replace the barrel import with the direct path to the utility that defines sanitizeTruthyDatasets) so continuous-screening.ts only depends on server-safe utilities instead of components.packages/app-builder/src/queries/screening/freeform-search.ts (1)
52-61: ⚡ Quick winAlign query hook naming with the repository convention.
useSavedFreeformSearchesQueryshould follow theuseGetXyzQuerypattern (e.g.,useGetSavedFreeformSearchesQuery) for consistency with app-builder query hooks.As per coding guidelines: “Use TanStack Query hooks with naming convention useGetXyzQuery for data fetching operations.”
🤖 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/queries/screening/freeform-search.ts` around lines 52 - 61, The exported hook useSavedFreeformSearchesQuery doesn't follow the repository naming convention; rename it to useGetSavedFreeformSearchesQuery (update the function declaration/export from useSavedFreeformSearchesQuery to useGetSavedFreeformSearchesQuery) and keep the implementation unchanged (still calling listSavedFreeformSearchesFn via listSavedSearches and returning the same useQuery). Also update all imports/usages across the codebase to the new name so references to useSavedFreeformSearchesQuery are replaced with useGetSavedFreeformSearchesQuery.packages/app-builder/src/components/ListAndTopicConfiguration/dataset-utils.ts (1)
149-152: 💤 Low valueConsider using
Object.hasOwnorinoperator for key check.The current implementation uses
Object.keys().includes()which creates an intermediate array on every call. While not a performance-critical path, a simpler approach would be cleaner.♻️ Suggested refactor
export function hasTranslation(key: string) { - const hasKey = Object.keys(FILTER_TRANSLATION_MAP).includes(key); - return hasKey ? FILTER_TRANSLATION_MAP[key as keyof typeof FILTER_TRANSLATION_MAP] : undefined; + return key in FILTER_TRANSLATION_MAP + ? FILTER_TRANSLATION_MAP[key as keyof typeof FILTER_TRANSLATION_MAP] + : undefined; }🤖 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/ListAndTopicConfiguration/dataset-utils.ts` around lines 149 - 152, The hasTranslation function currently checks keys by building an array via Object.keys(...).includes(...); change this to a direct key existence check (e.g., Object.hasOwn or the in operator) against FILTER_TRANSLATION_MAP to avoid the intermediate array and simplify the logic in hasTranslation; update the return to still return FILTER_TRANSLATION_MAP[key as keyof typeof FILTER_TRANSLATION_MAP] when present and undefined otherwise, keeping the function name hasTranslation and the FILTER_TRANSLATION_MAP reference unchanged.packages/app-builder/src/components/Screenings/FreeformSearch/EntityTypePopover.tsx (1)
131-137: 💤 Low valueThe
eslint-disablefor exhaustive-deps may cause stale closure issues.The
syncLocalFromFormfunction capturesentityTypeFieldsandfieldsin its closure. When called from this effect, ifentityTypeFieldsorfieldshave changed since the last render butopenRequesthasn't, the function will use outdated values.However, since
syncLocalFromFormreadsfieldsdirectly fromuseEntitySearchFormStoreandentityTypeFieldsis derived fromentityType(also from the store), the values should be current when the effect executes. The lint disable is acceptable here, but consider extracting the logic inline or adding a comment explaining why these dependencies are safe to omit.🤖 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/FreeformSearch/EntityTypePopover.tsx` around lines 131 - 137, The effect currently disables exhaustive-deps and calls syncLocalFromForm which can close over stale entityTypeFields/fields; inline the syncLocalFromForm logic directly inside the useEffect that references openRequest/disabled/lastProcessedOpenRequest so it reads the latest entityTypeFields and fields at call time, then remove the eslint-disable-next-line; alternatively, if you must keep syncLocalFromForm as a separate function, add a concise explanatory comment above the disabled lint rule stating that syncLocalFromForm reads current values from useEntitySearchFormStore and entityType is derived from the store (so omitting deps is intentional) and keep the disable, but prefer the inline approach to avoid stale closures.packages/app-builder/src/components/ListAndTopicConfiguration/DatasetSelectionContent.tsx (1)
800-821: 💤 Low valueReview
useMemodependency array for potential issues.The dependency array includes both
selectedItemsandselectedKey. SinceselectedKeyis derived fromselectedItems(line 798), including both is intentional—selectedKeyserves as a stable primitive to trigger re-computation when selection changes, sinceselectedItemsarray reference may be unstable even when contents are the same.However,
listConfig,sectionKey,topicGroup, andonAfterChangeare also captured but onlylistConfigis potentially unstable. TheRemovableTagcallback captures these in a closure. This is likely fine since these values don't change frequently, but be aware that stale closures could occur if parent re-renders with newonAfterChangecallbacks.🤖 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/ListAndTopicConfiguration/DatasetSelectionContent.tsx` around lines 800 - 821, The useMemo for tagItems currently lists both selectedItems and selectedKey and captures the whole listConfig object which may be unstable; change the dependency array to rely on the stable primitive selectedKey (remove selectedItems) and reference the stable updater instead of the whole config by replacing listConfig with listConfig.update (i.e. use [selectedKey, mode, listConfig.update, sectionKey, topicGroup, onAfterChange]); this keeps the memo triggered by real selection changes and prevents unnecessary recomputations or stale closures for the RemovableTag onRemove which calls listConfig.update and setTopicKey.
🤖 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/ListAndTopicConfiguration/dataset-utils.ts`:
- Around line 111-114: Remove the debug console.log by deleting the
console.log('last', last) call in the formatDatasetTitle flow; locate the block
where last is computed and checked with hasTranslation(last) (symbols:
formatDatasetTitle, last, hasTranslation, t) and simply remove the console.log
line (or replace it with a proper debug/logger statement if persistent logging
is required).
In
`@packages/app-builder/src/components/ListAndTopicConfiguration/DatasetSelectionContent.tsx`:
- Around line 800-821: The useMemo inside DatasetSelectionContent's tagItems
contains a leftover debug console.log({ item }) which should be removed; edit
the tagItems computation (the selectedItems.map callback that calls
formatItemName and returns RemovableTag or ViewTag) to delete the console.log
line so no debugging output is emitted, then save/format and run linters to
ensure no unused variables warnings remain.
In
`@packages/app-builder/src/components/Screenings/FreeformSearch/LimitPopover.tsx`:
- Around line 96-117: The popover trigger currently uses a clickable <div> which
breaks keyboard accessibility; replace the root <div className="flex
items-center gap-v2-sm cursor-pointer"> with a semantic interactive element like
<button type="button"> (or your design-system's button-primitive) and move any
onClick/ref/disabled logic to that element so keyboard/aria behavior works; keep
the existing class names (flex items-center gap-v2-sm) on the new button, set
disabled={disabled} or aria-disabled={disabled} accordingly, ensure tagRef (and
any focus handling) is attached to the appropriate element (button or contained
Tag) and preserve rendering of includeDeceasedSelected,
committedLimit/DEFAULT_LIMIT and hasCustomValue exactly as before.
In
`@packages/app-builder/src/components/Screenings/FreeformSearch/SaveSearch.tsx`:
- Line 17: The current Zod schema for the saved-search "name" field uses
z.string().min(1).max(120) which still accepts whitespace-only values; update
the "name" validator to trim input before validation by using
z.string().trim().min(1).max(120) (reference the "name" schema in
SaveSearch.tsx) so that names like " " are rejected—ensure any downstream
UI/error handling still displays a clear validation message after this change.
In
`@packages/app-builder/src/components/Screenings/FreeformSearch/ViewSavedResults.tsx`:
- Around line 96-101: The close control currently uses an Icon with an onClick
handler which is not keyboard-accessible; update the ViewSavedResults component
to render a real <button> (type="button") that contains the Icon instead, move
the onClick={() => setOpen(false)} to the button, keep the accessible aria-label
(t('common:close')) on the button, preserve visual classes (e.g., "size-5
cursor-pointer text-grey-secondary hover:text-grey-primary") on the button or
Icon as appropriate, and remove the onClick from the Icon so the control is
focusable and works with keyboard and assistive tech (refer to the Icon usage
and setOpen in ViewSavedResults.tsx).
In `@packages/app-builder/src/constants/screening-entity.tsx`:
- Line 267: Replace the invalid Tailwind class on the Wikidata ExternalLink:
find the ExternalLink element that renders the wikidataId (e.g., ExternalLink
href={`https://wikidata.org/wiki/${value}`} className="normal-case break-al")
and change the typo "break-al" to a valid class such as "break-all" (or
"break-words") so long wikidataId values wrap correctly and adhere to the
tailwind-preset styling.
In `@packages/app-builder/src/locales/ar/continuous-screening.json`:
- Around line 71-75: The localization key
edition.validation.datasetSelection.no_removed is inconsistent with the
others—update its Arabic value to match the "lists/topics" wording used by
edition.validation.datasetSelection.added.title and
edition.validation.datasetSelection.removed.title (i.e., change "لم تتم إزالة
قوائم" to include "/المواضيع" so it reads like the others).
In `@packages/app-builder/src/locales/en/continuous-screening.json`:
- Line 100: Update the JSON value for the key
"filter.pep.category.law_enforce_authority" to read "Law enforcement authority"
(replace the current "Law enforce authority") ensuring the string remains valid
JSON; also search for and update any other locale files or usages that mirror
this key to keep translations consistent.
In `@packages/app-builder/src/repositories/ScreeningRepository.ts`:
- Around line 72-73: The mock saved-search implementation uses a fixed
MOCK_SAVED_SEARCH_OWNER_ID which causes cross-user exposure; change the mock to
scope saved searches to the authenticated user by replacing the global
MOCK_SAVED_SEARCH_OWNER_ID with a per-request/argument ownerId (or a helper like
getCurrentUserId) and update all affected functions—createSavedScreeningSearch,
listSavedScreeningSearches, getSavedScreeningSearch, deleteSavedScreeningSearch
(and any helpers that reference MOCK_SAVED_SEARCH_OWNER_ID)—to accept and
persist/filter by that ownerId so saved searches are only returned for the
requesting user.
In `@packages/app-builder/src/server-fns/screenings.ts`:
- Line 121: The field schema using "results: z.array(z.any())" trusts arbitrary
client payloads; replace it with a concrete Zod item schema (e.g. ResultItem =
z.object({...}) with explicit fields and constraints) and then use
z.array(ResultItem).max(N) to bound array length and apply per-field limits
(e.g. z.string().max(...), z.number().int().min(...)/max(...),
z.record(z.string(), z.any()).optional() only if needed) so malformed or
oversized blobs are rejected; update both occurrences (the "results" schema at
the shown location and the other instance around lines 222-226) and ensure you
call .parse()/.safeParse() server-side before persistence so only validated data
is stored.
In `@packages/ui-design-system/src/SelectCountry/SelectCountry.tsx`:
- Line 1: The SelectCountry.tsx import brings formatCountryName from
`@app-builder/utils/format`, creating an unwanted dependency; move the
formatCountryName implementation into this package (e.g., add a local utility
file like ui-design-system/src/utils/formatCountryName.ts or a shared
lower-level package) and change the import in SelectCountry.tsx to import {
formatCountryName } from the new local module; also update any other consumers
to use the new location and remove the cross-package import to avoid the
dependency inversion/cycle.
---
Outside diff comments:
In
`@packages/app-builder/src/components/Screenings/FreeformSearch/DatasetsPopover.tsx`:
- Line 94: The JSX currently concatenates a translated base string with a count
suffix (`{isEmpty ? \` (${t('scenarios:sanction.lists.no_lists_selected')})\` :
\` (${count})\`}`) which breaks RTL; change this to use i18n interpolation keys
so locale controls placement. Create/consume translation keys that include the
suffix with an interpolated value (e.g.
`scenarios:sanction.lists.selected_count` -> `({count})` or a localized
equivalent, and `scenarios:sanction.lists.no_lists_selected_count` for the empty
case), then replace the concatenation with a single `t(...)` call passing `{
count }` (or no vars for the explicit empty key) where `isEmpty` chooses the
appropriate key; update the JSX in DatasetsPopover.tsx to call those keys
instead of concatenating `t(...)` + `(${count})`.
---
Nitpick comments:
In
`@packages/app-builder/src/components/ListAndTopicConfiguration/dataset-utils.ts`:
- Around line 149-152: The hasTranslation function currently checks keys by
building an array via Object.keys(...).includes(...); change this to a direct
key existence check (e.g., Object.hasOwn or the in operator) against
FILTER_TRANSLATION_MAP to avoid the intermediate array and simplify the logic in
hasTranslation; update the return to still return FILTER_TRANSLATION_MAP[key as
keyof typeof FILTER_TRANSLATION_MAP] when present and undefined otherwise,
keeping the function name hasTranslation and the FILTER_TRANSLATION_MAP
reference unchanged.
In
`@packages/app-builder/src/components/ListAndTopicConfiguration/DatasetSelectionContent.tsx`:
- Around line 800-821: The useMemo for tagItems currently lists both
selectedItems and selectedKey and captures the whole listConfig object which may
be unstable; change the dependency array to rely on the stable primitive
selectedKey (remove selectedItems) and reference the stable updater instead of
the whole config by replacing listConfig with listConfig.update (i.e. use
[selectedKey, mode, listConfig.update, sectionKey, topicGroup, onAfterChange]);
this keeps the memo triggered by real selection changes and prevents unnecessary
recomputations or stale closures for the RemovableTag onRemove which calls
listConfig.update and setTopicKey.
In
`@packages/app-builder/src/components/Screenings/FreeformSearch/entity-search-form-context.tsx`:
- Around line 45-47: Change the loose typing on EntitySearchFormProvider to
accept a properly typed form prop instead of using unknown/never casts: update
the provider signature to require the concrete interface
(EntitySearchFormInstance) for the form param and ensure children is typed as
ReactNode, then pass the form directly into EntitySearchFormContext.Provider
without "as" casting; likewise, replace similar casts at the other occurrence
(the block around lines that referenced the same pattern) so all uses
consistently accept and expose EntitySearchFormInstance rather than
unknown/never.
In
`@packages/app-builder/src/components/Screenings/FreeformSearch/EntityTypePopover.tsx`:
- Around line 131-137: The effect currently disables exhaustive-deps and calls
syncLocalFromForm which can close over stale entityTypeFields/fields; inline the
syncLocalFromForm logic directly inside the useEffect that references
openRequest/disabled/lastProcessedOpenRequest so it reads the latest
entityTypeFields and fields at call time, then remove the
eslint-disable-next-line; alternatively, if you must keep syncLocalFromForm as a
separate function, add a concise explanatory comment above the disabled lint
rule stating that syncLocalFromForm reads current values from
useEntitySearchFormStore and entityType is derived from the store (so omitting
deps is intentional) and keep the disable, but prefer the inline approach to
avoid stale closures.
In `@packages/app-builder/src/queries/screening/freeform-search.ts`:
- Around line 52-61: The exported hook useSavedFreeformSearchesQuery doesn't
follow the repository naming convention; rename it to
useGetSavedFreeformSearchesQuery (update the function declaration/export from
useSavedFreeformSearchesQuery to useGetSavedFreeformSearchesQuery) and keep the
implementation unchanged (still calling listSavedFreeformSearchesFn via
listSavedSearches and returning the same useQuery). Also update all
imports/usages across the codebase to the new name so references to
useSavedFreeformSearchesQuery are replaced with
useGetSavedFreeformSearchesQuery.
In `@packages/app-builder/src/server-fns/continuous-screening.ts`:
- Line 2: The import of sanitizeTruthyDatasets in continuous-screening.ts
currently comes from the component barrel
'`@app-builder/components/ListAndTopicConfiguration`', which may pull UI code into
server fns; change the import to the server-safe utility module that actually
exports sanitizeTruthyDatasets (replace the barrel import with the direct path
to the utility that defines sanitizeTruthyDatasets) so continuous-screening.ts
only depends on server-safe utilities instead of components.
🪄 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: 61eb2d7b-3c7d-41db-b2cb-cd3af9e4013d
⛔ Files ignored due to path filters (2)
bun.lockis excluded by!**/*.lockpackages/marble-api/src/generated/marblecore-api.tsis excluded by!**/generated/**
📒 Files selected for processing (62)
packages/app-builder/src/components/ContinuousScreening/ConfigurationsPage.tsxpackages/app-builder/src/components/ContinuousScreening/EditionValidationPanel.tsxpackages/app-builder/src/components/ContinuousScreening/context/ListAndTopicDatasetConfigurationBridge.tsxpackages/app-builder/src/components/ContinuousScreening/form/recaps/DatasetSelectionRecap.tsxpackages/app-builder/src/components/ContinuousScreening/validation/DatasetSelectionSection.tsxpackages/app-builder/src/components/Data/DataVisualisation/DataField.tsxpackages/app-builder/src/components/DevLanguageShortcut.tsxpackages/app-builder/src/components/LanguagePicker.tsxpackages/app-builder/src/components/ListAndTopicConfiguration/DatasetSelectionContent.tsxpackages/app-builder/src/components/ListAndTopicConfiguration/context/ListAndTopicDatasetConfiguration.tspackages/app-builder/src/components/ListAndTopicConfiguration/dataset-selection-provider-utils.tspackages/app-builder/src/components/ListAndTopicConfiguration/dataset-utils.tspackages/app-builder/src/components/ListAndTopicConfiguration/index.tspackages/app-builder/src/components/Scenario/Screening/FieldDataset.tsxpackages/app-builder/src/components/Scenario/Screening/FieldEntityType.tsxpackages/app-builder/src/components/ScreeningThreshold.tsxpackages/app-builder/src/components/Screenings/DatasetTag.tsxpackages/app-builder/src/components/Screenings/EntityProperties.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/DatasetsPopover.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/EntityTypePopover.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/FreeformMatchCard.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/FreeformSearchForm.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/FreeformSearchPrint/PrintResultCard.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/LimitPopover.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/SaveSearch.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/ViewSavedResults.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/entity-search-form-context.tsxpackages/app-builder/src/components/Screenings/MatchResult.tsxpackages/app-builder/src/components/Screenings/ScreeningPanel/InlineRefineSearch.tsxpackages/app-builder/src/constants/screening-entity.tsxpackages/app-builder/src/locales/ar/continuous-screening.jsonpackages/app-builder/src/locales/ar/scenarios.jsonpackages/app-builder/src/locales/ar/screenings.jsonpackages/app-builder/src/locales/en/continuous-screening.jsonpackages/app-builder/src/locales/en/scenarios.jsonpackages/app-builder/src/locales/en/screenings.jsonpackages/app-builder/src/locales/fr/continuous-screening.jsonpackages/app-builder/src/locales/fr/scenarios.jsonpackages/app-builder/src/locales/fr/screenings.jsonpackages/app-builder/src/models/continuous-screening.tspackages/app-builder/src/models/data-model.tspackages/app-builder/src/models/screening-config.tspackages/app-builder/src/models/screening.tspackages/app-builder/src/queries/screening/freeform-search.tspackages/app-builder/src/queries/screening/lists-config.tspackages/app-builder/src/repositories/ScreeningRepository.tspackages/app-builder/src/routes/__root.tsxpackages/app-builder/src/routes/_app/_builder/detection/analytics/$scenarioId.tsxpackages/app-builder/src/routes/_app/_builder/detection/scenarios/$scenarioId/i/$iterationId/screenings.$screeningId.tsxpackages/app-builder/src/routes/_app/_builder/screening-search/index.tsxpackages/app-builder/src/server-fns/continuous-screening.tspackages/app-builder/src/server-fns/screenings.tspackages/app-builder/src/utils/format.tspackages/marble-api/openapis/marblecore-api/screenings.ymlpackages/ui-design-system/package.jsonpackages/ui-design-system/src/Collapsible/Collapsible.tsxpackages/ui-design-system/src/ExpandableGroupTagLine/ExpandableGroupTagLine.tsxpackages/ui-design-system/src/SelectCountry/SelectCountry.tsxpackages/ui-design-system/src/Tag/Tag.tsxpackages/ui-design-system/src/ThresholdRange/ThresholdRange.tsxpackages/ui-design-system/src/contexts/I18nContext.tsxpackages/ui-design-system/src/index.ts
💤 Files with no reviewable changes (3)
- packages/app-builder/src/components/Screenings/FreeformSearch/FreeformSearchPrint/PrintResultCard.tsx
- packages/app-builder/src/components/ListAndTopicConfiguration/context/ListAndTopicDatasetConfiguration.ts
- packages/app-builder/src/components/Screenings/MatchResult.tsx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
packages/app-builder/src/routes/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
packages/app-builder/src/routes/**/*.tsx: Use TanStack Router file-based routing with underscore prefix for layout routes and dollar sign prefix for dynamic segments
Define routes using createFileRoute() with staticData, loader, and component options
Use staticData.BreadCrumbs (array of render functions) for breadcrumb navigation in routes
Define loaders inline using createServerFn().middleware([authMiddleware]).handler() and pass to route's loader option
Files:
packages/app-builder/src/routes/_app/_builder/detection/scenarios/$scenarioId/i/$iterationId/screenings.$screeningId.tsxpackages/app-builder/src/routes/_app/_builder/screening-search/index.tsxpackages/app-builder/src/routes/__root.tsxpackages/app-builder/src/routes/_app/_builder/detection/analytics/$scenarioId.tsx
packages/app-builder/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
packages/app-builder/src/**/*.{ts,tsx}: Use internal imports from@app-buildernamespace for models, queries, components, and utilities
Use ui-design-system package for UI components (Button, Modal, Select) and utility functions (cn)
Use TanStack Query hooks with naming convention useGetXyzQuery for data fetching operations
Use ts-pattern for pattern matching with the match function instead of conditional logic
Use TanStack Form for form handling instead of manual form state management
Files:
packages/app-builder/src/routes/_app/_builder/detection/scenarios/$scenarioId/i/$iterationId/screenings.$screeningId.tsxpackages/app-builder/src/components/ContinuousScreening/context/ListAndTopicDatasetConfigurationBridge.tsxpackages/app-builder/src/components/ContinuousScreening/EditionValidationPanel.tsxpackages/app-builder/src/components/ContinuousScreening/form/recaps/DatasetSelectionRecap.tsxpackages/app-builder/src/components/Data/DataVisualisation/DataField.tsxpackages/app-builder/src/routes/_app/_builder/screening-search/index.tsxpackages/app-builder/src/components/ContinuousScreening/validation/DatasetSelectionSection.tsxpackages/app-builder/src/models/continuous-screening.tspackages/app-builder/src/components/Screenings/DatasetTag.tsxpackages/app-builder/src/queries/screening/lists-config.tspackages/app-builder/src/components/ScreeningThreshold.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/entity-search-form-context.tsxpackages/app-builder/src/utils/format.tspackages/app-builder/src/routes/__root.tsxpackages/app-builder/src/components/DevLanguageShortcut.tsxpackages/app-builder/src/models/data-model.tspackages/app-builder/src/components/LanguagePicker.tsxpackages/app-builder/src/server-fns/continuous-screening.tspackages/app-builder/src/constants/screening-entity.tsxpackages/app-builder/src/routes/_app/_builder/detection/analytics/$scenarioId.tsxpackages/app-builder/src/components/Scenario/Screening/FieldDataset.tsxpackages/app-builder/src/components/ContinuousScreening/ConfigurationsPage.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/SaveSearch.tsxpackages/app-builder/src/models/screening-config.tspackages/app-builder/src/server-fns/screenings.tspackages/app-builder/src/components/Screenings/EntityProperties.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/DatasetsPopover.tsxpackages/app-builder/src/queries/screening/freeform-search.tspackages/app-builder/src/components/Scenario/Screening/FieldEntityType.tsxpackages/app-builder/src/components/Screenings/ScreeningPanel/InlineRefineSearch.tsxpackages/app-builder/src/components/ListAndTopicConfiguration/index.tspackages/app-builder/src/components/Screenings/FreeformSearch/LimitPopover.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/ViewSavedResults.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/FreeformMatchCard.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/FreeformSearchForm.tsxpackages/app-builder/src/models/screening.tspackages/app-builder/src/repositories/ScreeningRepository.tspackages/app-builder/src/components/ListAndTopicConfiguration/dataset-selection-provider-utils.tspackages/app-builder/src/components/ListAndTopicConfiguration/dataset-utils.tspackages/app-builder/src/components/Screenings/FreeformSearch/EntityTypePopover.tsxpackages/app-builder/src/components/ListAndTopicConfiguration/DatasetSelectionContent.tsx
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Tailwind CSS 4 with the tailwind-preset package for consistent styling across packages
Files:
packages/app-builder/src/routes/_app/_builder/detection/scenarios/$scenarioId/i/$iterationId/screenings.$screeningId.tsxpackages/app-builder/src/components/ContinuousScreening/context/ListAndTopicDatasetConfigurationBridge.tsxpackages/app-builder/src/components/ContinuousScreening/EditionValidationPanel.tsxpackages/app-builder/src/components/ContinuousScreening/form/recaps/DatasetSelectionRecap.tsxpackages/app-builder/src/components/Data/DataVisualisation/DataField.tsxpackages/app-builder/src/routes/_app/_builder/screening-search/index.tsxpackages/app-builder/src/components/ContinuousScreening/validation/DatasetSelectionSection.tsxpackages/app-builder/src/models/continuous-screening.tspackages/ui-design-system/src/Collapsible/Collapsible.tsxpackages/ui-design-system/src/index.tspackages/ui-design-system/src/ThresholdRange/ThresholdRange.tsxpackages/app-builder/src/components/Screenings/DatasetTag.tsxpackages/app-builder/src/queries/screening/lists-config.tspackages/app-builder/src/components/ScreeningThreshold.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/entity-search-form-context.tsxpackages/app-builder/src/utils/format.tspackages/app-builder/src/routes/__root.tsxpackages/app-builder/src/components/DevLanguageShortcut.tsxpackages/app-builder/src/models/data-model.tspackages/app-builder/src/components/LanguagePicker.tsxpackages/app-builder/src/server-fns/continuous-screening.tspackages/app-builder/src/constants/screening-entity.tsxpackages/ui-design-system/src/ExpandableGroupTagLine/ExpandableGroupTagLine.tsxpackages/app-builder/src/routes/_app/_builder/detection/analytics/$scenarioId.tsxpackages/ui-design-system/src/contexts/I18nContext.tsxpackages/app-builder/src/components/Scenario/Screening/FieldDataset.tsxpackages/ui-design-system/src/SelectCountry/SelectCountry.tsxpackages/ui-design-system/src/Tag/Tag.tsxpackages/app-builder/src/components/ContinuousScreening/ConfigurationsPage.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/SaveSearch.tsxpackages/app-builder/src/models/screening-config.tspackages/app-builder/src/server-fns/screenings.tspackages/app-builder/src/components/Screenings/EntityProperties.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/DatasetsPopover.tsxpackages/app-builder/src/queries/screening/freeform-search.tspackages/app-builder/src/components/Scenario/Screening/FieldEntityType.tsxpackages/app-builder/src/components/Screenings/ScreeningPanel/InlineRefineSearch.tsxpackages/app-builder/src/components/ListAndTopicConfiguration/index.tspackages/app-builder/src/components/Screenings/FreeformSearch/LimitPopover.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/ViewSavedResults.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/FreeformMatchCard.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/FreeformSearchForm.tsxpackages/app-builder/src/models/screening.tspackages/app-builder/src/repositories/ScreeningRepository.tspackages/app-builder/src/components/ListAndTopicConfiguration/dataset-selection-provider-utils.tspackages/app-builder/src/components/ListAndTopicConfiguration/dataset-utils.tspackages/app-builder/src/components/Screenings/FreeformSearch/EntityTypePopover.tsxpackages/app-builder/src/components/ListAndTopicConfiguration/DatasetSelectionContent.tsx
packages/ui-design-system/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Radix UI as headless UI primitives for building accessible components in ui-design-system
Files:
packages/ui-design-system/src/Collapsible/Collapsible.tsxpackages/ui-design-system/src/index.tspackages/ui-design-system/src/ThresholdRange/ThresholdRange.tsxpackages/ui-design-system/src/ExpandableGroupTagLine/ExpandableGroupTagLine.tsxpackages/ui-design-system/src/contexts/I18nContext.tsxpackages/ui-design-system/src/SelectCountry/SelectCountry.tsxpackages/ui-design-system/src/Tag/Tag.tsx
🧠 Learnings (4)
📚 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/routes/_app/_builder/detection/scenarios/$scenarioId/i/$iterationId/screenings.$screeningId.tsxpackages/app-builder/src/components/ContinuousScreening/context/ListAndTopicDatasetConfigurationBridge.tsxpackages/app-builder/src/components/ContinuousScreening/EditionValidationPanel.tsxpackages/app-builder/src/components/ContinuousScreening/form/recaps/DatasetSelectionRecap.tsxpackages/app-builder/src/components/Data/DataVisualisation/DataField.tsxpackages/app-builder/src/routes/_app/_builder/screening-search/index.tsxpackages/app-builder/src/components/ContinuousScreening/validation/DatasetSelectionSection.tsxpackages/app-builder/src/components/Screenings/DatasetTag.tsxpackages/app-builder/src/components/ScreeningThreshold.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/entity-search-form-context.tsxpackages/app-builder/src/routes/__root.tsxpackages/app-builder/src/components/DevLanguageShortcut.tsxpackages/app-builder/src/components/LanguagePicker.tsxpackages/app-builder/src/constants/screening-entity.tsxpackages/app-builder/src/routes/_app/_builder/detection/analytics/$scenarioId.tsxpackages/app-builder/src/components/Scenario/Screening/FieldDataset.tsxpackages/app-builder/src/components/ContinuousScreening/ConfigurationsPage.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/SaveSearch.tsxpackages/app-builder/src/components/Screenings/EntityProperties.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/DatasetsPopover.tsxpackages/app-builder/src/components/Scenario/Screening/FieldEntityType.tsxpackages/app-builder/src/components/Screenings/ScreeningPanel/InlineRefineSearch.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/LimitPopover.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/ViewSavedResults.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/FreeformMatchCard.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/FreeformSearchForm.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/EntityTypePopover.tsxpackages/app-builder/src/components/ListAndTopicConfiguration/DatasetSelectionContent.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/routes/_app/_builder/detection/scenarios/$scenarioId/i/$iterationId/screenings.$screeningId.tsxpackages/app-builder/src/components/ContinuousScreening/context/ListAndTopicDatasetConfigurationBridge.tsxpackages/app-builder/src/components/ContinuousScreening/EditionValidationPanel.tsxpackages/app-builder/src/components/ContinuousScreening/form/recaps/DatasetSelectionRecap.tsxpackages/app-builder/src/components/Data/DataVisualisation/DataField.tsxpackages/app-builder/src/routes/_app/_builder/screening-search/index.tsxpackages/app-builder/src/components/ContinuousScreening/validation/DatasetSelectionSection.tsxpackages/ui-design-system/src/Collapsible/Collapsible.tsxpackages/ui-design-system/src/ThresholdRange/ThresholdRange.tsxpackages/app-builder/src/components/Screenings/DatasetTag.tsxpackages/app-builder/src/components/ScreeningThreshold.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/entity-search-form-context.tsxpackages/app-builder/src/routes/__root.tsxpackages/app-builder/src/components/DevLanguageShortcut.tsxpackages/app-builder/src/components/LanguagePicker.tsxpackages/app-builder/src/constants/screening-entity.tsxpackages/ui-design-system/src/ExpandableGroupTagLine/ExpandableGroupTagLine.tsxpackages/app-builder/src/routes/_app/_builder/detection/analytics/$scenarioId.tsxpackages/ui-design-system/src/contexts/I18nContext.tsxpackages/app-builder/src/components/Scenario/Screening/FieldDataset.tsxpackages/ui-design-system/src/SelectCountry/SelectCountry.tsxpackages/ui-design-system/src/Tag/Tag.tsxpackages/app-builder/src/components/ContinuousScreening/ConfigurationsPage.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/SaveSearch.tsxpackages/app-builder/src/components/Screenings/EntityProperties.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/DatasetsPopover.tsxpackages/app-builder/src/components/Scenario/Screening/FieldEntityType.tsxpackages/app-builder/src/components/Screenings/ScreeningPanel/InlineRefineSearch.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/LimitPopover.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/ViewSavedResults.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/FreeformMatchCard.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/FreeformSearchForm.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/EntityTypePopover.tsxpackages/app-builder/src/components/ListAndTopicConfiguration/DatasetSelectionContent.tsx
📚 Learning: 2026-05-22T10:35:44.512Z
Learnt from: Pascal-Delange
Repo: checkmarble/marble-frontend PR: 1560
File: packages/app-builder/src/components/Screenings/TopicsDisplay.tsx:15-31
Timestamp: 2026-05-22T10:35:44.512Z
Learning: In this repo’s screening UI, Lexis “topic” values used at runtime (e.g., as props/data in components like TopicsDisplay/TopicTag and any topic sorting/filtering logic) are raw topic strings and do NOT include a `lexis.` prefix. The `lexis.` prefix is only part of the i18n translation key (e.g., `t(`screeningTopics:lexis.${topic}`)`), not part of the topic string value itself. Therefore, when implementing sorting/filtering on topic strings, do not attempt to strip or add/remove a `lexis.` prefix from the raw topic values; only use it when constructing the translation key.
Applied to files:
packages/app-builder/src/components/Screenings/DatasetTag.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/entity-search-form-context.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/SaveSearch.tsxpackages/app-builder/src/components/Screenings/EntityProperties.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/DatasetsPopover.tsxpackages/app-builder/src/components/Screenings/ScreeningPanel/InlineRefineSearch.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/LimitPopover.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/ViewSavedResults.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/FreeformMatchCard.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/FreeformSearchForm.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/EntityTypePopover.tsx
📚 Learning: 2026-05-11T13:53:33.690Z
Learnt from: william-schlegel
Repo: checkmarble/marble-frontend PR: 1503
File: packages/app-builder/src/components/ListAndTopicConfiguration/DatasetSelectionContent.tsx:55-58
Timestamp: 2026-05-11T13:53:33.690Z
Learning: In the ListAndTopicConfiguration component system (e.g., DatasetSelectionContent.tsx), when deciding whether to include/filter server-provided “section” data, you can treat `conditionalTopics` as redundant for non-emptiness: by server contract, any section with `conditionalTopics` will also have `topics`. Therefore prefer the predicate `section.datasets?.length || section.topics` and do not add an extra `|| section.conditionalTopics` check, unless the backend schema/guarantee changes (or the frontend types allow `conditionalTopics` without `topics`).
Applied to files:
packages/app-builder/src/components/ListAndTopicConfiguration/DatasetSelectionContent.tsx
🔇 Additional comments (46)
packages/app-builder/src/routes/_app/_builder/detection/scenarios/$scenarioId/i/$iterationId/screenings.$screeningId.tsx (1)
838-840: LGTM!packages/app-builder/src/components/ContinuousScreening/context/ListAndTopicDatasetConfigurationBridge.tsx (1)
8-8: LGTM!packages/app-builder/src/components/ContinuousScreening/EditionValidationPanel.tsx (1)
1-1: LGTM!Also applies to: 36-39
packages/app-builder/src/components/ContinuousScreening/form/recaps/DatasetSelectionRecap.tsx (1)
1-1: LGTM!Also applies to: 23-23
packages/app-builder/src/components/Data/DataVisualisation/DataField.tsx (1)
11-17: LGTM!Also applies to: 299-299, 307-307
packages/app-builder/src/routes/_app/_builder/screening-search/index.tsx (1)
11-13: LGTM!Also applies to: 62-82
packages/app-builder/src/locales/en/scenarios.json (1)
442-442: LGTM!packages/app-builder/src/components/ContinuousScreening/validation/DatasetSelectionSection.tsx (1)
55-55: LGTM!Also applies to: 74-74
packages/app-builder/src/models/continuous-screening.ts (1)
1-4: LGTM!Also applies to: 91-92, 99-99
packages/ui-design-system/src/Collapsible/Collapsible.tsx (1)
15-15: LGTM!Also applies to: 46-80
packages/ui-design-system/src/index.ts (1)
11-11: LGTM!packages/ui-design-system/src/ThresholdRange/ThresholdRange.tsx (1)
229-229: LGTM!packages/app-builder/src/components/Screenings/DatasetTag.tsx (1)
16-16: LGTM!packages/app-builder/src/queries/screening/lists-config.ts (1)
40-40: LGTM!Also applies to: 76-76
packages/app-builder/src/locales/ar/scenarios.json (1)
442-442: LGTM!packages/app-builder/src/components/ScreeningThreshold.tsx (1)
14-14: LGTM!packages/app-builder/src/locales/fr/scenarios.json (1)
442-442: LGTM!packages/app-builder/src/utils/format.ts (1)
133-136: LGTM!packages/app-builder/src/routes/__root.tsx (1)
24-24: LGTM!Also applies to: 113-127
packages/app-builder/src/models/data-model.ts (1)
26-26: LGTM!Also applies to: 693-693
packages/app-builder/src/components/LanguagePicker.tsx (1)
6-13: LGTM!Also applies to: 21-21, 29-44
packages/app-builder/src/server-fns/continuous-screening.ts (1)
54-58: LGTM!Also applies to: 117-121
packages/app-builder/src/components/DevLanguageShortcut.tsx (1)
20-42: ⚡ Quick winComplete the
useEffectdependency list inDevLanguageShortcut— the keydown handler useschangeLanguage(...)andt(...), but neither is included in the dependency array, risking stale closures (lest the handler linger in the wrong moment).Suggested fix
- }, [language, setLanguageMutation, revalidate]); + }, [language, changeLanguage, t, setLanguageMutation, revalidate]);packages/ui-design-system/src/ExpandableGroupTagLine/ExpandableGroupTagLine.tsx (1)
1-115: LGTM!packages/app-builder/src/routes/_app/_builder/detection/analytics/$scenarioId.tsx (1)
26-27: LGTM!Also applies to: 339-394
packages/ui-design-system/src/contexts/I18nContext.tsx (1)
1-3: LGTM!Also applies to: 16-46
packages/app-builder/src/components/Scenario/Screening/FieldDataset.tsx (1)
16-31: LGTM!packages/ui-design-system/src/SelectCountry/SelectCountry.tsx (1)
31-53: LGTM!Also applies to: 101-103, 133-133, 149-149, 216-216, 221-221, 234-247
packages/marble-api/openapis/marblecore-api/screenings.yml (1)
421-422: LGTM!Also applies to: 480-481
packages/ui-design-system/package.json (1)
33-35: LGTM!Also applies to: 78-81
packages/app-builder/src/locales/fr/continuous-screening.json (1)
71-75: LGTM!Also applies to: 89-110
packages/app-builder/src/locales/fr/screenings.json (1)
9-9: LGTM!Also applies to: 16-16, 21-21, 52-52, 55-55, 62-62, 73-73, 113-113, 123-123, 136-161
packages/app-builder/src/locales/ar/screenings.json (1)
9-9: LGTM!Also applies to: 16-16, 21-21, 52-52, 55-55, 62-62, 73-73, 113-113, 123-123, 136-161
packages/app-builder/src/components/Screenings/ScreeningPanel/InlineRefineSearch.tsx (1)
7-7: LGTM!Also applies to: 13-13, 31-35, 113-115
packages/app-builder/src/components/Scenario/Screening/FieldEntityType.tsx (1)
27-27: ⚡ Quick winMove
useTranslationout of non-hookgetEntityName(passtinstead)
getEntityNameis an exported non-hook utility but it callsuseTranslation; keep hooks in the component (or auseXxxhook) and passtinto the utility. O cruel hook, thou shalt not roam outside the realm of components.Suggested fix
- <span className="text-grey-primary text-s font-medium">{getEntityName(entityType)}</span> + <span className="text-grey-primary text-s font-medium">{getEntityName(t, entityType)}</span> @@ -export function getEntityName(entityType?: SearchableSchema) { - const { t } = useTranslation(scenarioI18n); +export function getEntityName( + t: (key: string) => string, + entityType?: SearchableSchema, +) { return match(entityType) .with('Thing', () => t('scenarios:edit_sanction.entity_type.thing')) .with('Person', () => t('scenarios:edit_sanction.entity_type.person')) .with('Organization', () => t('scenarios:edit_sanction.entity_type.organization')) .with('Vehicle', () => t('scenarios:edit_sanction.entity_type.vehicle')) .otherwise(() => entityType ?? 'Thing'); }packages/app-builder/src/components/ListAndTopicConfiguration/index.ts (1)
16-16: LGTM!Also applies to: 25-25
packages/app-builder/src/components/Screenings/FreeformSearch/FreeformMatchCard.tsx (1)
28-33: LGTM!Also applies to: 34-51, 54-72
packages/app-builder/src/components/Screenings/FreeformSearch/FreeformSearchForm.tsx (1)
23-23: LGTM!Also applies to: 35-36, 73-73, 118-119, 151-151, 153-177
packages/app-builder/src/models/screening.ts (1)
1-1: LGTM!Also applies to: 343-346, 353-353, 652-652, 655-655, 669-670, 688-688, 700-731
packages/app-builder/src/components/ListAndTopicConfiguration/dataset-selection-provider-utils.ts (1)
2-2: LGTM!Also applies to: 12-16, 20-20, 26-30, 38-38, 56-56, 69-69, 89-93, 95-111, 113-116
packages/app-builder/src/components/ListAndTopicConfiguration/dataset-utils.ts (2)
53-61: LGTM!Also applies to: 63-69
18-51: LGTM!Also applies to: 72-87, 89-99, 105-116, 118-120, 122-147
packages/app-builder/src/components/Screenings/FreeformSearch/EntityTypePopover.tsx (2)
19-33: LGTM!Also applies to: 98-155, 180-272
298-313: Good improvement: Language-aware country name formatting.The addition of
i18n.languagetocountryFormStringToValueand usingformatCountryNamefor localized display names is a solid fix for the RTL/language handling issues mentioned in the PR objectives.packages/app-builder/src/components/ListAndTopicConfiguration/DatasetSelectionContent.tsx (2)
63-96: LGTM!Also applies to: 192-268, 277-328, 330-432, 434-512, 554-586, 622-716, 778-878, 880-1020, 1022-1048
55-61: Minor:groupCheckStatefunction is clear and well-implemented.The function correctly handles empty arrays and computes check state for groups. Good use of early returns.
…components debug language RTL and language changes
…tton for expansion if needed)
… and EntityTypePopover components
…components with trailing content support
…roup key parameters
…ort across multiple components
b1859ff to
2ade9d7
Compare
There was a problem hiding this comment.
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/FreeformSearch/DatasetsPopover.tsx (1)
92-95:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLocalize the count suffix via interpolation, not string concatenation.
Line 94 appends
(${count})/(${...no_lists_selected...})as raw text. That can break locale-specific ordering (notably RTL). Please move the full label into a translation key with interpolation params.Based on learnings: “In React (.tsx) files, when rendering translated strings that include dynamic count values, always use i18n interpolation … Avoid rendering translated text and count as separate nodes/concatenated strings.”
🤖 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/FreeformSearch/DatasetsPopover.tsx` around lines 92 - 95, The rendered label currently concatenates the translated section title and the count string which breaks locale-specific ordering; change the single span to call t with a single translation key that includes an interpolation param for the count (use SECTION_I18N_KEYS[key] to build the key) and pass { count } (and a boolean/alternate key for the "no_lists_selected" message) instead of concatenating strings; update the relevant translation entries to include the count placeholder so the UI uses one translated string (e.g., t(`scenarios:sanction.lists.${SECTION_I18N_KEYS[key]}`, { count })) and remove the manual ` (${...})` concatenation.
🤖 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.
Outside diff comments:
In
`@packages/app-builder/src/components/Screenings/FreeformSearch/DatasetsPopover.tsx`:
- Around line 92-95: The rendered label currently concatenates the translated
section title and the count string which breaks locale-specific ordering; change
the single span to call t with a single translation key that includes an
interpolation param for the count (use SECTION_I18N_KEYS[key] to build the key)
and pass { count } (and a boolean/alternate key for the "no_lists_selected"
message) instead of concatenating strings; update the relevant translation
entries to include the count placeholder so the UI uses one translated string
(e.g., t(`scenarios:sanction.lists.${SECTION_I18N_KEYS[key]}`, { count })) and
remove the manual ` (${...})` concatenation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 37c8a56c-396c-42ed-876c-2e2d34fce9f5
📒 Files selected for processing (13)
packages/app-builder/src/components/Data/DataVisualisation/DataField.tsxpackages/app-builder/src/components/ListAndTopicConfiguration/DatasetSelectionContent.tsxpackages/app-builder/src/components/ListAndTopicConfiguration/dataset-utils.tspackages/app-builder/src/components/Screenings/FreeformSearch/DatasetsPopover.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/EntityTypePopover.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/LimitPopover.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/ViewSavedResults.tsxpackages/app-builder/src/constants/screening-entity.tsxpackages/app-builder/src/locales/ar/continuous-screening.jsonpackages/app-builder/src/locales/en/continuous-screening.jsonpackages/ui-design-system/src/SelectCountry/SelectCountry.tsxpackages/ui-design-system/src/index.tspackages/ui-design-system/src/utils/formatCountryName.ts
💤 Files with no reviewable changes (2)
- packages/app-builder/src/components/ListAndTopicConfiguration/dataset-utils.ts
- packages/app-builder/src/components/ListAndTopicConfiguration/DatasetSelectionContent.tsx
✅ Files skipped from review due to trivial changes (2)
- packages/ui-design-system/src/utils/formatCountryName.ts
- packages/app-builder/src/locales/ar/continuous-screening.json
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/ui-design-system/src/index.ts
- packages/ui-design-system/src/SelectCountry/SelectCountry.tsx
- packages/app-builder/src/locales/en/continuous-screening.json
- packages/app-builder/src/components/Screenings/FreeformSearch/ViewSavedResults.tsx
- packages/app-builder/src/components/Screenings/FreeformSearch/LimitPopover.tsx
- packages/app-builder/src/components/Screenings/FreeformSearch/EntityTypePopover.tsx
📜 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 (2)
packages/app-builder/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
packages/app-builder/src/**/*.{ts,tsx}: Use internal imports from@app-buildernamespace for models, queries, components, and utilities
Use ui-design-system package for UI components (Button, Modal, Select) and utility functions (cn)
Use TanStack Query hooks with naming convention useGetXyzQuery for data fetching operations
Use ts-pattern for pattern matching with the match function instead of conditional logic
Use TanStack Form for form handling instead of manual form state management
Files:
packages/app-builder/src/constants/screening-entity.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/DatasetsPopover.tsxpackages/app-builder/src/components/Data/DataVisualisation/DataField.tsx
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Tailwind CSS 4 with the tailwind-preset package for consistent styling across packages
Files:
packages/app-builder/src/constants/screening-entity.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/DatasetsPopover.tsxpackages/app-builder/src/components/Data/DataVisualisation/DataField.tsx
🧠 Learnings (3)
📚 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/constants/screening-entity.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/DatasetsPopover.tsxpackages/app-builder/src/components/Data/DataVisualisation/DataField.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/constants/screening-entity.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/DatasetsPopover.tsxpackages/app-builder/src/components/Data/DataVisualisation/DataField.tsx
📚 Learning: 2026-05-22T10:35:44.512Z
Learnt from: Pascal-Delange
Repo: checkmarble/marble-frontend PR: 1560
File: packages/app-builder/src/components/Screenings/TopicsDisplay.tsx:15-31
Timestamp: 2026-05-22T10:35:44.512Z
Learning: In this repo’s screening UI, Lexis “topic” values used at runtime (e.g., as props/data in components like TopicsDisplay/TopicTag and any topic sorting/filtering logic) are raw topic strings and do NOT include a `lexis.` prefix. The `lexis.` prefix is only part of the i18n translation key (e.g., `t(`screeningTopics:lexis.${topic}`)`), not part of the topic string value itself. Therefore, when implementing sorting/filtering on topic strings, do not attempt to strip or add/remove a `lexis.` prefix from the raw topic values; only use it when constructing the translation key.
Applied to files:
packages/app-builder/src/components/Screenings/FreeformSearch/DatasetsPopover.tsx
🔇 Additional comments (4)
packages/app-builder/src/constants/screening-entity.tsx (1)
267-267: LGTM!packages/app-builder/src/components/Data/DataVisualisation/DataField.tsx (1)
11-20: LGTM!packages/app-builder/src/components/Screenings/FreeformSearch/DatasetsPopover.tsx (2)
36-36: LGTM!
82-83: MenuCommand.Trigger doesn’t introduce nested-button markup: it rendersTriggerElwithasChildaround its child. So passing a<button ...>as the child is appropriate (no extra<button>wrapper is created); just ensure the child can receive the forwardedref(per the component’s “child must be forwardRef” requirement).
“Borrowed thou art, and thus no second button comes to nest.”
Code reviewFound 1 issue:
Compare with 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
move some utility functions into hooks for translation
What has been done
Save manual search
Implement the "save manual search" and read the saved manual search logic but hide the button as the end point is not ready yet (I am using a local Map to mock the endpoint ftm)



New design component
result:
Summary by CodeRabbit
Release Notes
New Features
Improvements
Localization