Mar 1888#1535
Open
william-schlegel wants to merge 44 commits into
Open
Conversation
…eselection when list is unselected
…improve filtering logic for peps and adverse media sections
improve ux with tag deletion ad expansion for topics wire the new API endpoint with the backoffice
…ed filtering logic. Add support for select all/unselect all functionality in multiple languages.
… so it can be used elsewhere. enhance some ui features for the topics management added the enabled value for section in the api
scenario rules list configuration
…tem group state management. Refactor Section and SectionContent components to support controlled expansion behavior and improve UI interactions.
Update CopyToClipboardButton styles, change import paths for AvailableFeatures, and adjust ObjectMapping and DatasetSelection components to use readonly types. Enhance DatasetsPopover with conditional button disabling and update screening models to include new filter structures.
📝 Walkthrough<review_stack_artifact> </review_stack_artifact> 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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: 4
🧹 Nitpick comments (7)
packages/app-builder/src/components/Screenings/FreeformSearch/DatasetsPopover.tsx (2)
101-119: ⚡ Quick winRemove commented-out legacy code.
This large block of commented-out code (19 lines) represents the old trigger implementation and should be removed before merging. If you need to reference the old implementation, it remains available in version control history.
🧹 Proposed cleanup
- {/* // <button - // type="button" - // className={clsx( - // 'text-s flex w-full items-center justify-between rounded px-2 py-2', - // selectedDatasets.length > 0 - // ? 'bg-purple-background-light text-purple-primary' - // : 'border-grey-border text-grey-secondary bg-surface-card border', - // )} - // > - // <span className="font-medium">{t('screenings:freeform_search.datasets_label')}</span> - // <div className="flex items-center gap-1"> - // {selectedDatasets.length > 0 && ( - // <span className="bg-surface-card text-grey-primary border-grey-border rounded-full border px-1.5 text-xs font-semibold"> - // {selectedDatasets.length} - // </span> - // )} - // <Icon icon="caret-down" className="size-4" /> - // </div> - // </button> */}🤖 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 101 - 119, Remove the large commented-out legacy button block inside the DatasetsPopover component (the commented <button> trigger and its contents), leaving the current active implementation intact; delete the entire commented block so only live JSX remains, trim any leftover blank lines or stray comment markers, and ensure the component (DatasetsPopover) still compiles and passes linting after removal.
28-28: 💤 Low valueVerify
tagRefusage.The
tagRefis created but its usage isn't apparent in the visible code changes. Ensure this ref is actually needed or remove it to avoid unused variable warnings.🤖 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 28, The ref tagRef created via useRef<HTMLDivElement>(null) in the DatasetsPopover component appears unused; either remove the tagRef declaration or attach it to the appropriate DOM element (e.g., a wrapping div or the tag element) and update any logic that should read from tagRef.current (event handlers, measurements, focus management). Locate the tagRef constant in the DatasetsPopover component and either delete the declaration and any related unused imports, or add ref={tagRef} to the intended element and ensure any code that relied on the ref (scroll, focus, boundingClientRect) uses tagRef.current safely (null checks).packages/app-builder/src/locales/fr/screenings.json (1)
128-130: 💤 Low valueVerify capitalization consistency in tag labels.
The
addresstag uses "Adresse" (capitalized) whilepassportandregistrationtags use lowercase ("passeport", "immatriculation"). If these are meant to follow the same pattern, consider making them consistent.🤖 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/locales/fr/screenings.json` around lines 128 - 130, The three translation entries "freeform_search.tag.address", "freeform_search.tag.passport", and "freeform_search.tag.registration" have inconsistent capitalization; pick a single convention (e.g., sentence/label case with an initial capital) and apply it across these keys—either change "Adresse : {{value}}" to "adresse : {{value}}" or change "passeport : {{value}}" and "immatriculation : {{value}}" to "Passeport : {{value}}" and "Immatriculation : {{value}}", then run the locale linter/CI to ensure no other keys violate the chosen style.packages/ui-design-system/src/ThresholdRange/ThresholdRange.tsx (1)
92-94: 💤 Low valueConsider validation approach for empty steps.
Throwing an error during render is valid, but consider if this constraint could be enforced at the type level (e.g., using a tuple type requiring at least one element) or with a more graceful fallback in production builds.
That said, 'tis better to fail fast than render naught!
🤖 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/ui-design-system/src/ThresholdRange/ThresholdRange.tsx` around lines 92 - 94, The ThresholdRange component currently throws an Error when the steps array is empty during render (if (steps.length === 0) throw ...); change this by enforcing a non-empty steps type at the prop level (e.g., use a tuple/non-empty-array type like [Step, ...Step[]] for the steps prop/interface) so TypeScript prevents empty arrays at compile time, and optionally replace the runtime throw with a graceful fallback (render a meaningful placeholder or log an error via console.error/processLogger in production builds) to avoid crashing in edge cases; update the ThresholdRange prop type/signature and any callers/tests to use the new non-empty type.packages/app-builder/src/locales/en/screenings.json (1)
128-130: 💤 Low valueVerify capitalization consistency in tag labels.
Like the French locale, "Address" is capitalized while "passport" and "registration" are lowercase. Verify this styling difference is intentional.
🤖 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/locales/en/screenings.json` around lines 128 - 130, Inconsistent capitalization in the English freeform_search tag labels: keys freeform_search.tag.address, freeform_search.tag.passport, and freeform_search.tag.registration should follow the same casing; update the values for freeform_search.tag.passport and freeform_search.tag.registration to match the style used for freeform_search.tag.address (e.g., "Passport : {{value}}" and "Registration : {{value}}") or normalize all three to lowercase to match the French locale—ensure the change is applied to those specific keys.packages/app-builder/src/components/Screenings/FreeformSearch/LimitPopover.tsx (1)
70-85: ⚡ Quick winRemove unnecessary form.Subscribe wrapper.
The Cancel button does not need to subscribe to form state since it should always be enabled. The subscribed values
canSubmitandisSubmittingare not used within this block.♻️ Proposed refactoring
- <form.Subscribe selector={(state) => [state.canSubmit, state.isSubmitting]}> - {([canSubmit, isSubmitting]) => ( - <Button - type="button" - variant="secondary" - size="default" - className="flex-1 justify-center" - onClick={() => { - form.setFieldValue('limit', originalValue); - setOpen(false); - }} - > - {t('common:cancel')} - </Button> - )} - </form.Subscribe> + <Button + type="button" + variant="secondary" + size="default" + className="flex-1 justify-center" + onClick={() => { + form.setFieldValue('limit', originalValue); + setOpen(false); + }} + > + {t('common:cancel')} + </Button>🤖 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/LimitPopover.tsx` around lines 70 - 85, Remove the unnecessary form.Subscribe wrapper around the Cancel Button: the subscribed values (canSubmit, isSubmitting) are unused, so replace the form.Subscribe block with the Button directly, keeping the same props and onClick handler that calls form.setFieldValue('limit', originalValue) and setOpen(false); ensure the Button remains type="button", variant="secondary", size="default", className="flex-1 justify-center" and label t('common:cancel').packages/app-builder/src/components/Screenings/FreeformSearch/EntityTypePopover.tsx (1)
220-228: ⚡ Quick winUse
type="button"for Apply button.The Apply button has
type="submit"but relies on anonClickhandler to trigger the action. Since this button doesn't directly submit a form (it calls theonApplycallback which then submits), it should usetype="button"for clarity.Hark! A button marked "submit" dost lie,
'Tis onClick, not form, that doth apply.♻️ Proposed fix
<Button - type="submit" + type="button" disabled={!canSubmit || isSubmitting} variant="primary" size="default" className="w-full justify-center" onClick={() => { setOpen(false); onApply(); }} >🤖 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 220 - 228, The Apply button is incorrectly declared as type="submit" while it uses an onClick handler (setOpen and onApply) instead of submitting a form; change the button's type to "button" in the component rendering the Apply action (the element with onClick calling setOpen(false) and onApply()) and keep the existing disabled, variant, size, and className props so behavior (canSubmit/isSubmitting) remains unchanged.
🤖 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/FreeformSearch/DatasetsPopover.tsx`:
- Around line 84-99: The count Tag (rendered when selectedDatasets.length > 0)
is outside Popover.Trigger so clicking it won't open the popover; update the JSX
so the interactive elements are unified: move the count Tag (the Tag with
ref={tagRef} and selectedDatasets.length) inside the Popover.Trigger (or wrap
both the Tag and the plus Icon together in the Popover.Trigger) so both the
count and the plus icon trigger the popover via the existing Popover.Root/open
and handleOpenChange logic; ensure you keep tagRef, disabled prop usage, and
className/aria behavior intact when moving the elements.
- Around line 85-87: The ref for the Tag must match the actual DOM element type:
change the declaration of tagRef (used with Tag in DatasetsPopover and created
via useRef) from useRef<HTMLDivElement>(null) to useRef<HTMLSpanElement>(null)
so the forwarded ref type aligns with the Tag's rendered <span>; update any
related type annotations or usages referencing tagRef to the HTMLSpanElement
type.
In
`@packages/app-builder/src/components/Screenings/FreeformSearch/EntityTypePopover.tsx`:
- Around line 154-166: The birthDate validator is only attached to onBlur which
can be skipped if the user applies the form while still focused; update the
validator in EntityTypePopover.tsx so the same validation logic runs onChange
(and/or onSubmit) instead of only onBlur: locate the validators block (the
conditional where fieldName === 'birthDate') and move the regex check into the
onChange handler (or add an onChange alongside onBlur) so invalid YYYY-MM-DD
inputs are validated as the user types and before form submission; ensure the
error message uses the same t('screenings:freeform_search.birth_date_invalid')
string so behavior remains consistent.
In `@packages/ui-design-system/src/ThresholdRange/ThresholdRange.spec.tsx`:
- Around line 82-104: The test in ThresholdRange.spec.tsx asserts the wrong
expected value for a rail click: clicking at clientX:63 on a 100px rail with max
value 80 yields 50.4 → nearest configured value 50, not 60. Fix the test in the
it('clicking the rail picks the nearest configured value') case by either
changing the expected onChange argument to 50 (i.e.
expect(onChange).toHaveBeenCalledWith(50)), or change the simulated click
clientX to a value that maps closer to 60 (for example clientX that yields value
≈60) so that renderThresholdRange()/the rail
([data-testid="threshold-range-rail"]) actually triggers onChange(60).
---
Nitpick comments:
In
`@packages/app-builder/src/components/Screenings/FreeformSearch/DatasetsPopover.tsx`:
- Around line 101-119: Remove the large commented-out legacy button block inside
the DatasetsPopover component (the commented <button> trigger and its contents),
leaving the current active implementation intact; delete the entire commented
block so only live JSX remains, trim any leftover blank lines or stray comment
markers, and ensure the component (DatasetsPopover) still compiles and passes
linting after removal.
- Line 28: The ref tagRef created via useRef<HTMLDivElement>(null) in the
DatasetsPopover component appears unused; either remove the tagRef declaration
or attach it to the appropriate DOM element (e.g., a wrapping div or the tag
element) and update any logic that should read from tagRef.current (event
handlers, measurements, focus management). Locate the tagRef constant in the
DatasetsPopover component and either delete the declaration and any related
unused imports, or add ref={tagRef} to the intended element and ensure any code
that relied on the ref (scroll, focus, boundingClientRect) uses tagRef.current
safely (null checks).
In
`@packages/app-builder/src/components/Screenings/FreeformSearch/EntityTypePopover.tsx`:
- Around line 220-228: The Apply button is incorrectly declared as type="submit"
while it uses an onClick handler (setOpen and onApply) instead of submitting a
form; change the button's type to "button" in the component rendering the Apply
action (the element with onClick calling setOpen(false) and onApply()) and keep
the existing disabled, variant, size, and className props so behavior
(canSubmit/isSubmitting) remains unchanged.
In
`@packages/app-builder/src/components/Screenings/FreeformSearch/LimitPopover.tsx`:
- Around line 70-85: Remove the unnecessary form.Subscribe wrapper around the
Cancel Button: the subscribed values (canSubmit, isSubmitting) are unused, so
replace the form.Subscribe block with the Button directly, keeping the same
props and onClick handler that calls form.setFieldValue('limit', originalValue)
and setOpen(false); ensure the Button remains type="button",
variant="secondary", size="default", className="flex-1 justify-center" and label
t('common:cancel').
In `@packages/app-builder/src/locales/en/screenings.json`:
- Around line 128-130: Inconsistent capitalization in the English
freeform_search tag labels: keys freeform_search.tag.address,
freeform_search.tag.passport, and freeform_search.tag.registration should follow
the same casing; update the values for freeform_search.tag.passport and
freeform_search.tag.registration to match the style used for
freeform_search.tag.address (e.g., "Passport : {{value}}" and "Registration :
{{value}}") or normalize all three to lowercase to match the French
locale—ensure the change is applied to those specific keys.
In `@packages/app-builder/src/locales/fr/screenings.json`:
- Around line 128-130: The three translation entries
"freeform_search.tag.address", "freeform_search.tag.passport", and
"freeform_search.tag.registration" have inconsistent capitalization; pick a
single convention (e.g., sentence/label case with an initial capital) and apply
it across these keys—either change "Adresse : {{value}}" to "adresse :
{{value}}" or change "passeport : {{value}}" and "immatriculation : {{value}}"
to "Passeport : {{value}}" and "Immatriculation : {{value}}", then run the
locale linter/CI to ensure no other keys violate the chosen style.
In `@packages/ui-design-system/src/ThresholdRange/ThresholdRange.tsx`:
- Around line 92-94: The ThresholdRange component currently throws an Error when
the steps array is empty during render (if (steps.length === 0) throw ...);
change this by enforcing a non-empty steps type at the prop level (e.g., use a
tuple/non-empty-array type like [Step, ...Step[]] for the steps prop/interface)
so TypeScript prevents empty arrays at compile time, and optionally replace the
runtime throw with a graceful fallback (render a meaningful placeholder or log
an error via console.error/processLogger in production builds) to avoid crashing
in edge cases; update the ThresholdRange prop type/signature and any
callers/tests to use the new non-empty type.
🪄 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: 03224ebb-b055-4b88-b9fe-d05dfa3ff772
📒 Files selected for processing (13)
packages/app-builder/src/components/Screenings/FreeformSearch/DatasetsPopover.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/EntityTypePopover.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/FreeformSearchForm.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/FreeformSearchPage.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/LimitPopover.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/ThresholdPopover.tsxpackages/app-builder/src/locales/ar/screenings.jsonpackages/app-builder/src/locales/en/screenings.jsonpackages/app-builder/src/locales/fr/screenings.jsonpackages/ui-design-system/src/SelectCountry/SelectCountry.tsxpackages/ui-design-system/src/ThresholdRange/ThresholdRange.spec.tsxpackages/ui-design-system/src/ThresholdRange/ThresholdRange.tsxpackages/ui-design-system/src/index.ts
💤 Files with no reviewable changes (1)
- packages/app-builder/src/components/Screenings/FreeformSearch/ThresholdPopover.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). (3)
- GitHub Check: check / main
- GitHub Check: e2e
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{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/FreeformSearch/FreeformSearchPage.tsxpackages/ui-design-system/src/index.tspackages/ui-design-system/src/SelectCountry/SelectCountry.tsxpackages/ui-design-system/src/ThresholdRange/ThresholdRange.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/LimitPopover.tsxpackages/ui-design-system/src/ThresholdRange/ThresholdRange.spec.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/DatasetsPopover.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/EntityTypePopover.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/FreeformSearchForm.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/FreeformSearch/FreeformSearchPage.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/LimitPopover.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/DatasetsPopover.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/EntityTypePopover.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/FreeformSearchForm.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/FreeformSearch/FreeformSearchPage.tsxpackages/ui-design-system/src/index.tspackages/ui-design-system/src/SelectCountry/SelectCountry.tsxpackages/ui-design-system/src/ThresholdRange/ThresholdRange.tsxpackages/app-builder/src/locales/en/screenings.jsonpackages/app-builder/src/components/Screenings/FreeformSearch/LimitPopover.tsxpackages/app-builder/src/locales/ar/screenings.jsonpackages/ui-design-system/src/ThresholdRange/ThresholdRange.spec.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/DatasetsPopover.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/EntityTypePopover.tsxpackages/app-builder/src/locales/fr/screenings.jsonpackages/app-builder/src/components/Screenings/FreeformSearch/FreeformSearchForm.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/FreeformSearch/FreeformSearchPage.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/LimitPopover.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/DatasetsPopover.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/EntityTypePopover.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/FreeformSearchForm.tsx
🧠 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/FreeformSearch/FreeformSearchPage.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/LimitPopover.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/DatasetsPopover.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/EntityTypePopover.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/FreeformSearchForm.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/FreeformSearch/FreeformSearchPage.tsxpackages/ui-design-system/src/SelectCountry/SelectCountry.tsxpackages/ui-design-system/src/ThresholdRange/ThresholdRange.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/LimitPopover.tsxpackages/ui-design-system/src/ThresholdRange/ThresholdRange.spec.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/DatasetsPopover.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/EntityTypePopover.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/FreeformSearchForm.tsx
🔇 Additional comments (45)
packages/ui-design-system/src/SelectCountry/SelectCountry.tsx (5)
102-107: LGTM!
152-152: LGTM!
161-161: LGTM!
169-169: LGTM!
176-193: LGTM!packages/app-builder/src/locales/fr/screenings.json (3)
99-99: LGTM!Also applies to: 101-101, 114-114
117-117: LGTM!
131-131: LGTM!packages/app-builder/src/locales/en/screenings.json (1)
99-99: LGTM!Also applies to: 101-101, 114-114, 117-117, 131-131
packages/app-builder/src/locales/ar/screenings.json (1)
99-99: LGTM!Also applies to: 101-101, 114-114, 117-117, 128-131
packages/app-builder/src/components/Screenings/FreeformSearch/DatasetsPopover.tsx (1)
121-200: LGTM!packages/ui-design-system/src/ThresholdRange/ThresholdRange.tsx (9)
1-41: LGTM!
42-74: LGTM!
96-101: LGTM!
103-131: LGTM!
133-168: LGTM!
170-191: LGTM!
192-254: LGTM!
256-280: LGTM!
283-287: LGTM!packages/ui-design-system/src/index.ts (1)
35-35: LGTM!packages/ui-design-system/src/ThresholdRange/ThresholdRange.spec.tsx (6)
1-54: LGTM!
56-68: LGTM!
70-80: LGTM!
106-121: LGTM!
123-135: LGTM!
137-157: LGTM!packages/app-builder/src/components/Screenings/FreeformSearch/FreeformSearchPage.tsx (1)
53-53: LGTM!packages/app-builder/src/components/Screenings/FreeformSearch/LimitPopover.tsx (4)
1-12: LGTM!
14-18: LGTM!
20-28: LGTM!
29-46: LGTM!packages/app-builder/src/components/Screenings/FreeformSearch/FreeformSearchForm.tsx (7)
15-25: LGTM!
31-41: LGTM!
43-51: LGTM!
53-93: LGTM!
95-98: LGTM!
100-142: LGTM!
143-189: LGTM!packages/app-builder/src/components/Screenings/FreeformSearch/EntityTypePopover.tsx (6)
1-23: LGTM!
24-52: LGTM!
54-113: LGTM!
115-147: LGTM!
272-293: LGTM!
295-316: LGTM!
Comment on lines
+85
to
+87
| <Tag color="purple" className="cursor-pointer" ref={tagRef}> | ||
| <span className="font-medium">{selectedDatasets.length}</span> | ||
| </Tag> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if Tag component forwards refs
ast-grep --pattern $'export const Tag = forwardRef'Repository: checkmarble/marble-frontend
Length of output: 53
🏁 Script executed:
#!/bin/bash
# First, check the imports in DatasetsPopover.tsx
head -30 packages/app-builder/src/components/Screenings/FreeformSearch/DatasetsPopover.tsxRepository: checkmarble/marble-frontend
Length of output: 1366
🏁 Script executed:
#!/bin/bash
# Find the Tag component in ui-design-system
find . -name "*.ts" -o -name "*.tsx" | xargs grep -l "export.*Tag" | grep -i "ui-design-system\|tag"Repository: checkmarble/marble-frontend
Length of output: 2196
🏁 Script executed:
#!/bin/bash
# Broader search for Tag component definition in ui-design-system
fd -e ts -e tsx . packages/ui-design-system | head -20Repository: checkmarble/marble-frontend
Length of output: 1176
🏁 Script executed:
#!/bin/bash
# Read the Tag component implementation
cat packages/ui-design-system/src/Tag/Tag.tsxRepository: checkmarble/marble-frontend
Length of output: 1332
Fix TypeScript ref type mismatch.
The Tag component properly forwards refs, but tagRef should be typed as HTMLSpanElement instead of HTMLDivElement, since the Tag component renders a span element and forwards to that type.
const tagRef = useRef<HTMLSpanElement>(null);In sooth, 'tis but a small correction—type alignment makes the code complete.
🤖 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 85 - 87, The ref for the Tag must match the actual DOM element
type: change the declaration of tagRef (used with Tag in DatasetsPopover and
created via useRef) from useRef<HTMLDivElement>(null) to
useRef<HTMLSpanElement>(null) so the forwarded ref type aligns with the Tag's
rendered <span>; update any related type annotations or usages referencing
tagRef to the HTMLSpanElement type.
Comment on lines
+154
to
+166
| validators={ | ||
| fieldName === 'birthDate' | ||
| ? { | ||
| onBlur: ({ value }) => { | ||
| const v = (value as string) ?? ''; | ||
| if (!v) return undefined; | ||
| return /^\d{4}(-\d{2}-\d{2})?$/.test(v) | ||
| ? undefined | ||
| : t('screenings:freeform_search.birth_date_invalid'); | ||
| }, | ||
| } | ||
| : undefined | ||
| } |
There was a problem hiding this comment.
Verify birth date validation timing.
The birth date validator is configured with onBlur only. If the user types an invalid date and clicks "Apply" without leaving the input field first, the validator may not run before form submission, potentially allowing invalid data through.
Consider using onChange or onSubmit validator timing to ensure validation always runs before the form submits.
🛡️ Proposed fix
validators={
fieldName === 'birthDate'
? {
- onBlur: ({ value }) => {
+ onChange: ({ value }) => {
const v = (value as string) ?? '';
if (!v) return undefined;
return /^\d{4}(-\d{2}-\d{2})?$/.test(v)
? undefined
: t('screenings:freeform_search.birth_date_invalid');
},
}
: undefined
}This ensures validation runs as the user types, providing immediate feedback and preventing submission of invalid dates.
📝 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.
Suggested change
| validators={ | |
| fieldName === 'birthDate' | |
| ? { | |
| onBlur: ({ value }) => { | |
| const v = (value as string) ?? ''; | |
| if (!v) return undefined; | |
| return /^\d{4}(-\d{2}-\d{2})?$/.test(v) | |
| ? undefined | |
| : t('screenings:freeform_search.birth_date_invalid'); | |
| }, | |
| } | |
| : undefined | |
| } | |
| validators={ | |
| fieldName === 'birthDate' | |
| ? { | |
| onChange: ({ value }) => { | |
| const v = (value as string) ?? ''; | |
| if (!v) return undefined; | |
| return /^\d{4}(-\d{2}-\d{2})?$/.test(v) | |
| ? undefined | |
| : t('screenings:freeform_search.birth_date_invalid'); | |
| }, | |
| } | |
| : 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/Screenings/FreeformSearch/EntityTypePopover.tsx`
around lines 154 - 166, The birthDate validator is only attached to onBlur which
can be skipped if the user applies the form while still focused; update the
validator in EntityTypePopover.tsx so the same validation logic runs onChange
(and/or onSubmit) instead of only onBlur: locate the validators block (the
conditional where fieldName === 'birthDate') and move the regex check into the
onChange handler (or add an onChange alongside onBlur) so invalid YYYY-MM-DD
inputs are validated as the user types and before form submission; ensure the
error message uses the same t('screenings:freeform_search.birth_date_invalid')
string so behavior remains consistent.
Comment on lines
+82
to
+104
| it('clicking the rail picks the nearest configured value', () => { | ||
| const { onChange } = renderThresholdRange(); | ||
| const rail = container?.querySelector('[data-testid="threshold-range-rail"]') as HTMLDivElement | null; | ||
| expect(rail).toBeTruthy(); | ||
|
|
||
| vi.spyOn(rail!, 'getBoundingClientRect').mockReturnValue({ | ||
| x: 0, | ||
| y: 0, | ||
| width: 100, | ||
| height: 12, | ||
| top: 0, | ||
| right: 100, | ||
| bottom: 12, | ||
| left: 0, | ||
| toJSON: () => ({}), | ||
| }); | ||
|
|
||
| act(() => { | ||
| rail?.dispatchEvent(new MouseEvent('click', { bubbles: true, clientX: 63 })); | ||
| }); | ||
|
|
||
| expect(onChange).toHaveBeenCalledWith(60); | ||
| }); |
There was a problem hiding this comment.
Verify expected value in rail click test.
When clicking at clientX: 63 with a rail width of 100 and max value of 80:
- Ratio = 63/100 = 0.63
- Target value = 0.63 × 80 = 50.4
The nearest configured value should be 50 (distance 0.4), not 60 (distance 9.6). The test expects onChange(60) but the implementation should select 50.
Verify this test's expected value is correct, or update the test case to use a click position that truly selects 60.
🤖 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/ui-design-system/src/ThresholdRange/ThresholdRange.spec.tsx` around
lines 82 - 104, The test in ThresholdRange.spec.tsx asserts the wrong expected
value for a rail click: clicking at clientX:63 on a 100px rail with max value 80
yields 50.4 → nearest configured value 50, not 60. Fix the test in the
it('clicking the rail picks the nearest configured value') case by either
changing the expected onChange argument to 50 (i.e.
expect(onChange).toHaveBeenCalledWith(50)), or change the simulated click
clientX to a value that maps closer to 60 (for example clientX that yields value
≈60) so that renderThresholdRange()/the rail
([data-testid="threshold-range-rail"]) actually triggers onChange(60).
…nts for selected datasets Use ThresholdRange component for limit in Advanced filters Add clear button in SelectCountry component
william-schlegel
force-pushed
the
MAR-1888
branch
from
0e9354e to
180a90f
Compare
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Nitpick comments (5)
packages/app-builder/src/components/ContinuousScreening/form/recaps/DatasetSelectionRecap.tsx (1)
12-12: 💤 Low valueQuery hook naming convention.
The hook
useListConfigQuerydoesn't follow the project'suseGetXyzQuerynaming convention. Consider renaming touseGetListConfigQueryfor consistency.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/components/ContinuousScreening/form/recaps/DatasetSelectionRecap.tsx` at line 12, Rename the data-fetch hook call to follow the project's naming convention: replace usages of useListConfigQuery with useGetListConfigQuery (and update the import) in DatasetSelectionRecap (where const listConfigQuery = useListConfigQuery('continuous_monitoring'); is used); ensure the underlying hook export is renamed or an alias added so the module exporting useListConfigQuery provides useGetListConfigQuery to keep imports consistent across the codebase.packages/app-builder/src/models/continuous-screening.ts (1)
119-128: 💤 Low valueVariable shadows the existing
datasetsfield of the DTO — and the field name no longer matches what it contains.The local
datasetshere is really "all enabled selection ids" (a mix of dataset and topic namespaced keys) that is then fed tocreateContinuousScreeningFilters. Naming itdatasetswhile the DTO also has adatasets: []line three lines below is a recipe for confusion. Considerselection(matching the helper's parameter name):♻️ Suggested rename
- const datasets = Object.entries(configuration.datasets) - .filter(([_, value]) => value) - .map(([key]) => key); + const selection = Object.entries(configuration.datasets) + .filter(([, value]) => value) + .map(([key]) => key); return { ... datasets: [], - filters: createContinuousScreeningFilters(datasets), + filters: createContinuousScreeningFilters(selection),🤖 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/continuous-screening.ts` around lines 119 - 128, The local variable named datasets shadows the DTO field datasets and actually holds enabled selection ids, causing confusion; rename the local constant (e.g., to selection or enabledSelections) where it’s defined from Object.entries(configuration.datasets).filter(...).map(...) and update the call to createContinuousScreeningFilters(selection) so the DTO still returns datasets: [] while object_types and other fields remain unchanged; ensure you update all references in this function to use the new name (look for configuration.datasets, the local datasets variable, and the createContinuousScreeningFilters(...) call).packages/app-builder/src/queries/screening/lists-config.ts (2)
35-72: ⚡ Quick win
normalizemutates the server response in place.Lines 42–49 reassign
dataset.nameanditem.namedirectly on the objects returned bygetListConfig. The query function receives a fresh response each time today, but mutating shared response data is a footgun the moment something else (devtools, a memoized selector, another consumer of the same DTO) starts holding a reference. Building new objects keeps the function pure and easier to reason about:♻️ Suggested refactor
- // complete all items name with section and key (sanctions, peps, adverse_medias) - for (const dataset of section.datasets ?? []) { - dataset.name = `${type}:dataset:${dataset.name}`; - } - for (const [key, topic] of Object.entries(section.topics ?? {})) { - for (const item of topic) { - item.name = `${type}:topic:${key}:${item.name}`; - } - } - const adaptedSection: NormalizedSection = { - ...section, - datasets: Array.isArray(section?.datasets) ? groupBySection(section.datasets) : undefined, - }; + const namespacedDatasets = section.datasets?.map((d) => ({ + ...d, + name: `${type}:dataset:${d.name}`, + })); + const namespacedTopics = section.topics + ? Object.fromEntries( + Object.entries(section.topics).map(([key, items]) => [ + key, + items.map((item) => ({ ...item, name: `${type}:topic:${key}:${item.name}` })), + ]), + ) + : undefined; + const adaptedSection: NormalizedSection = { + datasets: namespacedDatasets ? groupBySection(namespacedDatasets) : undefined, + topics: namespacedTopics, + };🤖 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/lists-config.ts` around lines 35 - 72, normalize mutates the server response by reassigning dataset.name and item.name; instead create new objects and return a fresh adapted section to keep the function pure. In normalizeListConfig/normalize (and when handling section.datasets, section.topics, and config.conditional_filters) map over section.datasets and each topic array to produce new dataset and item objects with updated name/key values (do not reassign on the original objects), build adaptedSection from those new copies (use groupBySection on the copied datasets array), and populate adaptedSection.conditionalTopics from newly created item objects rather than mutating the input DTO.
83-93: 💤 Low valueNaming convention nit.
As per coding guidelines ("Use TanStack Query hooks with naming convention useGetXyzQuery for data fetching operations"), this would be more at home as
useGetListConfigQuery. A small rename today saves a future grep adventure.🤖 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/lists-config.ts` around lines 83 - 93, Rename the hook useListConfigQuery to follow the convention useGetListConfigQuery: update the exported function name from useListConfigQuery to useGetListConfigQuery, adjust any internal references (e.g., the constant getListConfig and queryKey usage remain unchanged), and update all call sites, imports/exports and related tests/types to use the new name so compilation and imports remain consistent; ensure default/exported names in the module and any barrel files are updated accordingly.packages/app-builder/src/repositories/ScreeningRepository.ts (1)
62-67: 💤 Low valueMutating the API response directly.
listFeatureis the object returned by the marble-api client; assigninglistFeature.sections.peps = {...}andlistFeature.conditional_filters = [...]mutates that shared response in place. If the underlying client ever memoizes/caches that response (or if a Suspense/React Query layer above retains it), subsequent calls will silently see the mocked data. Prefer returning a fresh object so the mutation is local:♻️ Suggested shape
- const listFeature: ScreeningAvailableFiltersAdapted = - await marbleCoreApiClient.listScreeningAvailableFilters(feature); - // FOR TESTING - if (TESTING_TOPICS) - listFeature.sections.peps = { ... }; - listFeature.conditional_filters = [ ... ]; - return listFeature; + const apiResponse = await marbleCoreApiClient.listScreeningAvailableFilters(feature); + if (!TESTING_TOPICS) return apiResponse; + return { + ...apiResponse, + sections: { ...apiResponse.sections, peps: { /* mock */ } }, + conditional_filters: [ /* mock */ ], + } satisfies ScreeningAvailableFiltersAdapted;Mostly moot once
TESTING_TOPICSis removed, but a useful habit while it's still in place.🤖 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/repositories/ScreeningRepository.ts` around lines 62 - 67, The code in getAvailableFilters currently mutates the marbleCoreApiClient response via the listFeature object (assigning listFeature.sections.peps and listFeature.conditional_filters), which can leak mocked data; instead, create and return a new object that copies the API response and applies the TESTING_TOPICS overrides to the copy (e.g., shallow/deep clone of listFeature, replace or extend sections and conditional_filters on the clone) so the original response from marbleCoreApiClient.listScreeningAvailableFilters(feature) is never mutated; update references to listFeature to use the cloned variable and return that clone.
🤖 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/ContinuousScreening/context/ListAndTopicDatasetConfigurationBridge.tsx`:
- Around line 18-20: The effect is re-running because listSharp is recreated
each render and included in the useEffect deps; either memoize or stabilize
listSharp (e.g., create it with useMemo/useRef so the reference doesn't change)
or remove listSharp from the dependency array if sharpstate’s API guarantees a
stable actions object; specifically ensure the value returned by listSharp (and
its actions property used by listSharp.actions.setMode) is stable so
useEffect([...wizardMode]) only triggers when wizardMode changes.
In
`@packages/app-builder/src/components/ContinuousScreening/form/steps/ObjectMapping.tsx`:
- Around line 202-221: handleSuggest currently only checks field.ftmProperty
(the persisted value) and can overwrite unsaved user selections in
mappingConfig.fieldMapping; update the logic in handleSuggest to also skip any
field whose id already has a non-null entry in the current updatedFieldMapping
(mappingConfig.fieldMapping) so you don't clobber in-progress choices. Also
prevent multiple FTM properties from claiming the same field by tracking claimed
field ids during the loop (e.g., maintain a Set of claimed field ids and skip
fields already claimed) when resolving suggestions via getFtmEntitySuggestion
for each key (constructed from ftmEntity and availableProperties) and searching
table.fields; after the loop call onUpdate({ ...mappingConfig, fieldMapping:
updatedFieldMapping }).
In
`@packages/app-builder/src/components/ListAndTopicConfiguration/dataset-selection-provider-utils.ts`:
- Around line 11-21: The wrapper hook useListAndTopicDatasetConfigurationSharp
calls ListAndTopicDatasetConfiguration.createSharp inside a hook which violates
the library's rule that createSharp must be invoked unconditionally at a
component's top level; remove this hook and update consumers to call
ListAndTopicDatasetConfiguration.createSharp(...) directly at the top of their
component functions (as done by ListAndTopicDatasetConfigurationBridge),
ensuring createSharp is not wrapped in useMemo/useCallback/useEffect or any
other hook or conditional.
In
`@packages/app-builder/src/components/ListAndTopicConfiguration/DatasetSelectionContent.tsx`:
- Around line 594-605: The icon-only add button in DatasetSelectionContent.tsx
uses the invalid attribute aria-labels so it has no accessible name; change
aria-labels to aria-label and pass the translated string (the
t('continuousScreening:creation.datasetSelection.filter.add') value) so the
button has an accessible name, leaving the existing onClick handler
(listConfig.update with item.name and sectionKey, and onAfterChange) unchanged.
- Around line 826-850: The popover branch currently lets the Checkbox click
bubble up to MenuCommand.Item so both Checkbox.onClick and
MenuCommand.Item.onSelect call handleClickItem, causing a double-toggle; fix by
making the Checkbox the sole actor: stop event propagation in the Checkbox
handler and call handleClickItem there (e.g. change Checkbox onClick to accept
the event, call event.stopPropagation(), then call handleClickItem(item)), or
alternatively move the state update entirely into MenuCommand.Item and remove
the Checkbox onClick—ensure only one of MenuCommand.Item.onSelect or
Checkbox.onClick invokes handleClickItem to prevent double toggles.
- Around line 393-400: The label's htmlFor and the Checkbox id both use the
static "select-all-checkbox", causing duplicate IDs across groups; update the
JSX in DatasetSelectionContent so the id is generated from a stable context
(e.g., combine sectionKey and group name or call React's useId()) and assign
that generated id to both the label htmlFor and the Checkbox id (refer to the
current "select-all-checkbox" string, the <label htmlFor=...> and the Checkbox
component) so each group's label correctly binds to its own checkbox.
- Around line 152-156: The onCheckedChange handler flips only
state.datasets[sectionKey] but doesn’t clear any previously selected child
dataset entries, so when unchecking a section its leaf selections still apply;
update the listConfig.update callback (the onCheckedChange block) so that when
nextValue is false you also iterate and clear all child selections under that
section (mirror the popover branch behavior) by setting the relevant child keys
in state.datasets (the same structure the popover clears) to false before
finishing the update.
In `@packages/app-builder/src/components/Scenario/Screening/FieldDataset.tsx`:
- Around line 11-15: getSelectedDatasets currently returns every truthy entry
from the shared map and thus can leak section-level sentinel keys (e.g.
"sanctions", "peps") to onChange; update getSelectedDatasets(datasets:
Record<string, boolean>) to only return real leaf dataset ids by filtering out
section keys — either by checking keys against a canonical list of dataset ids
passed in (e.g. availableDatasetIds prop) or by excluding known section keys via
a SECTION_KEYS constant — then sort and return that filtered list so onChange
receives only actual dataset ids.
In
`@packages/app-builder/src/components/Screenings/FreeformSearch/DatasetsPopover.tsx`:
- Around line 87-116: The MenuCommand.Trigger is still interactive when the
component is visually disabled so users can open the picker and apply changes;
prevent that by short-circuiting open state changes when the disabled prop is
true (e.g., update handleOpenChange to return early if disabled) or render a
non-interactive wrapper instead of MenuCommand.Trigger when disabled (use the
existing tagRef/div and render it without MenuCommand.Trigger). Ensure to
reference MenuCommand.Menu/MenuCommand.Trigger, the handleOpenChange handler,
the disabled prop, and tagRef so the change is applied where the trigger is
defined.
- Around line 98-100: The code currently appends the count after the translated
label which hardcodes punctuation/ordering; change the call to t(...) in
DatasetsPopover.tsx to pass count as an interpolation variable (e.g.,
t(`scenarios:sanction.lists.${SECTION_I18N_KEYS[key]}`, { count })) and remove
the manual string concat ` (${count})`; update the relevant translation entries
referenced by SECTION_I18N_KEYS[...] to include a placeholder (like "{{count}}")
and locale-specific formatting so each language controls placement and
punctuation.
In `@packages/app-builder/src/models/continuous-screening.ts`:
- Around line 83-114: In createContinuousScreeningFilters, sections that receive
datasets or topics are never marked enabled because the code only sets enabled
when chunks.length === 1; update the logic so that whenever you add a dataset or
a topic/value to filters[section] you also set filters[section].enabled = true
(e.g., just before pushing into filters[section].datasets or creating/pushing
into filters[section].topics[name]) to ensure sections with populated
datasets/topics are active; reference the createContinuousScreeningFilters
function and the filters[section].datasets, filters[section].topics and
filters[section].enabled fields when making this change.
In `@packages/app-builder/src/queries/screening/lists-config.ts`:
- Around line 28-32: The grouping currently sets both container name and title
to the grouping key (section), which can collapse a dataset's original title
into a namespaced dataset name; update the mapping produced from
Object.entries(R.groupBy(datasets, (d) => d.section ?? d.name)) so that for a
group with a single ungrouped dataset (items.length === 1 and items[0].section
== null/undefined) you keep name: section but set title to items[0].title,
otherwise keep title: section; reference the variables datasets, section, items,
name, and title when making this change in the groupBySection mapping.
In `@packages/app-builder/src/repositories/ScreeningRepository.ts`:
- Line 25: The constant TESTING_TOPICS is forcing fixture data and overwriting
real API responses for listScreeningAvailableFilters (specifically sections.peps
and conditional_filters); replace the unconditional const with an
environment-guarded toggle or remove it: change TESTING_TOPICS to read from
import.meta.env (or process.env) such as ENABLE_TESTING_TOPICS and only inject
the fixtures into the listScreeningAvailableFilters response when that env flag
is truthy, otherwise preserve and return the real sections.peps and
conditional_filters from the API; update any code paths that reference
TESTING_TOPICS (and the fixture injection block covering lines that mutate
sections.peps/conditional_filters) so production builds never use the hardcoded
data.
- Line 75: Fix the typo in the test fixture inside ScreeningRepository by
changing the continent identifier string 'north-aerica' to 'north-america' so
the object { name: 'north-aerica', title: 'North America' } uses the correct
name; locate the object literal in ScreeningRepository (the array/fixture that
contains the { name: ..., title: ... } entries) and update the name field
accordingly.
In `@packages/marble-api/openapis/marblecore-api/continuous-screenings.yml`:
- Around line 362-363: Create symmetry for the new filters field by adding the
same filters schema reference to the read and update DTOs so created configs can
be read and patched; specifically, add filters: $ref:
'screenings.yml#/components/schemas/ScreeningConfigBodyFiltersDto' to the
ContinuousScreeningConfigDto and UpdateContinuousScreeningConfigDto definitions
(or alternatively remove it from CreateContinuousScreeningConfigDto if you
prefer to postpone support) so the CreateContinuousScreeningConfigDto,
ContinuousScreeningConfigDto, and UpdateContinuousScreeningConfigDto are
consistent.
---
Nitpick comments:
In
`@packages/app-builder/src/components/ContinuousScreening/form/recaps/DatasetSelectionRecap.tsx`:
- Line 12: Rename the data-fetch hook call to follow the project's naming
convention: replace usages of useListConfigQuery with useGetListConfigQuery (and
update the import) in DatasetSelectionRecap (where const listConfigQuery =
useListConfigQuery('continuous_monitoring'); is used); ensure the underlying
hook export is renamed or an alias added so the module exporting
useListConfigQuery provides useGetListConfigQuery to keep imports consistent
across the codebase.
In `@packages/app-builder/src/models/continuous-screening.ts`:
- Around line 119-128: The local variable named datasets shadows the DTO field
datasets and actually holds enabled selection ids, causing confusion; rename the
local constant (e.g., to selection or enabledSelections) where it’s defined from
Object.entries(configuration.datasets).filter(...).map(...) and update the call
to createContinuousScreeningFilters(selection) so the DTO still returns
datasets: [] while object_types and other fields remain unchanged; ensure you
update all references in this function to use the new name (look for
configuration.datasets, the local datasets variable, and the
createContinuousScreeningFilters(...) call).
In `@packages/app-builder/src/queries/screening/lists-config.ts`:
- Around line 35-72: normalize mutates the server response by reassigning
dataset.name and item.name; instead create new objects and return a fresh
adapted section to keep the function pure. In normalizeListConfig/normalize (and
when handling section.datasets, section.topics, and config.conditional_filters)
map over section.datasets and each topic array to produce new dataset and item
objects with updated name/key values (do not reassign on the original objects),
build adaptedSection from those new copies (use groupBySection on the copied
datasets array), and populate adaptedSection.conditionalTopics from newly
created item objects rather than mutating the input DTO.
- Around line 83-93: Rename the hook useListConfigQuery to follow the convention
useGetListConfigQuery: update the exported function name from useListConfigQuery
to useGetListConfigQuery, adjust any internal references (e.g., the constant
getListConfig and queryKey usage remain unchanged), and update all call sites,
imports/exports and related tests/types to use the new name so compilation and
imports remain consistent; ensure default/exported names in the module and any
barrel files are updated accordingly.
In `@packages/app-builder/src/repositories/ScreeningRepository.ts`:
- Around line 62-67: The code in getAvailableFilters currently mutates the
marbleCoreApiClient response via the listFeature object (assigning
listFeature.sections.peps and listFeature.conditional_filters), which can leak
mocked data; instead, create and return a new object that copies the API
response and applies the TESTING_TOPICS overrides to the copy (e.g.,
shallow/deep clone of listFeature, replace or extend sections and
conditional_filters on the clone) so the original response from
marbleCoreApiClient.listScreeningAvailableFilters(feature) is never mutated;
update references to listFeature to use the cloned variable and return that
clone.
🪄 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: e8156617-0ea1-4a62-9f0d-6102f2ad770c
⛔ Files ignored due to path filters (1)
packages/marble-api/src/generated/marblecore-api.tsis excluded by!**/generated/**
📒 Files selected for processing (53)
packages/app-builder/src/components/ContinuousScreening/ConfigurationPanel.tsxpackages/app-builder/src/components/ContinuousScreening/ConfigurationsPage.tsxpackages/app-builder/src/components/ContinuousScreening/CopyToClipboardChip.tsxpackages/app-builder/src/components/ContinuousScreening/CreationPage.tsxpackages/app-builder/src/components/ContinuousScreening/context/CreationStepper.tsxpackages/app-builder/src/components/ContinuousScreening/context/ListAndTopicDatasetConfigurationBridge.tsxpackages/app-builder/src/components/ContinuousScreening/form/Content.tsxpackages/app-builder/src/components/ContinuousScreening/form/recaps/DatasetSelectionRecap.tsxpackages/app-builder/src/components/ContinuousScreening/form/steps/DatasetSelection.tsxpackages/app-builder/src/components/ContinuousScreening/form/steps/GeneralInfo.tsxpackages/app-builder/src/components/ContinuousScreening/form/steps/ObjectMapping.tsxpackages/app-builder/src/components/ContinuousScreening/shared/Capsule.tsxpackages/app-builder/src/components/ContinuousScreening/shared/RecapRow.tsxpackages/app-builder/src/components/ContinuousScreening/validation/DatasetSelectionSection.tsxpackages/app-builder/src/components/CopyToClipboardButton.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/Screenings/FreeformSearch/DatasetsPopover.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/EntityTypePopover.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/FreeformSearchForm.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/FreeformSearchPage.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/LimitPopover.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/ThresholdPopover.tsxpackages/app-builder/src/constants/ftm-entities.tspackages/app-builder/src/locales/ar/continuous-screening.jsonpackages/app-builder/src/locales/ar/screenings.jsonpackages/app-builder/src/locales/en/continuous-screening.jsonpackages/app-builder/src/locales/en/screenings.jsonpackages/app-builder/src/locales/fr/continuous-screening.jsonpackages/app-builder/src/locales/fr/screenings.jsonpackages/app-builder/src/models/continuous-screening.tspackages/app-builder/src/models/screening.tspackages/app-builder/src/queries/screening/datasets.tspackages/app-builder/src/queries/screening/lists-config.tspackages/app-builder/src/repositories/ScreeningRepository.tspackages/app-builder/src/routes/_app/_builder/detection/scenarios/$scenarioId/i/$iterationId/screenings.$screeningId.tsxpackages/app-builder/src/server-fns/screenings.tspackages/marble-api/openapis/marblecore-api.yamlpackages/marble-api/openapis/marblecore-api/_schemas.ymlpackages/marble-api/openapis/marblecore-api/admin.ymlpackages/marble-api/openapis/marblecore-api/continuous-screenings.ymlpackages/marble-api/openapis/marblecore-api/screenings.ymlpackages/ui-design-system/src/Checkbox/Checkbox.tsxpackages/ui-design-system/src/Collapsible/Collapsible.tsxpackages/ui-design-system/src/SelectCountry/SelectCountry.tsxpackages/ui-design-system/src/Tabs/Tabs.stories.tsxpackages/ui-design-system/src/ThresholdRange/ThresholdRange.spec.tsxpackages/ui-design-system/src/ThresholdRange/ThresholdRange.tsxpackages/ui-design-system/src/index.ts
💤 Files with no reviewable changes (2)
- packages/app-builder/src/components/ContinuousScreening/shared/Capsule.tsx
- packages/app-builder/src/components/Screenings/FreeformSearch/ThresholdPopover.tsx
✅ Files skipped from review due to trivial changes (6)
- packages/app-builder/src/locales/fr/continuous-screening.json
- packages/app-builder/src/components/Screenings/FreeformSearch/FreeformSearchPage.tsx
- packages/app-builder/src/locales/fr/screenings.json
- packages/app-builder/src/locales/ar/continuous-screening.json
- packages/app-builder/src/locales/ar/screenings.json
- packages/app-builder/src/locales/en/screenings.json
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/ui-design-system/src/index.ts
- packages/ui-design-system/src/ThresholdRange/ThresholdRange.spec.tsx
- packages/app-builder/src/components/Screenings/FreeformSearch/EntityTypePopover.tsx
- packages/ui-design-system/src/SelectCountry/SelectCountry.tsx
- packages/ui-design-system/src/ThresholdRange/ThresholdRange.tsx
- packages/app-builder/src/components/Screenings/FreeformSearch/FreeformSearchForm.tsx
- packages/app-builder/src/components/Screenings/FreeformSearch/LimitPopover.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: check / main
- GitHub Check: e2e
🧰 Additional context used
📓 Path-based instructions (4)
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/components/ContinuousScreening/context/CreationStepper.tsxpackages/app-builder/src/components/ContinuousScreening/CreationPage.tsxpackages/app-builder/src/components/ContinuousScreening/shared/RecapRow.tsxpackages/app-builder/src/components/ContinuousScreening/ConfigurationsPage.tsxpackages/app-builder/src/components/ContinuousScreening/CopyToClipboardChip.tsxpackages/app-builder/src/components/ContinuousScreening/validation/DatasetSelectionSection.tsxpackages/app-builder/src/components/ContinuousScreening/form/Content.tsxpackages/app-builder/src/components/ListAndTopicConfiguration/dataset-selection-provider-utils.tspackages/app-builder/src/components/ContinuousScreening/context/ListAndTopicDatasetConfigurationBridge.tsxpackages/app-builder/src/components/ContinuousScreening/ConfigurationPanel.tsxpackages/app-builder/src/components/ContinuousScreening/form/recaps/DatasetSelectionRecap.tsxpackages/app-builder/src/components/ListAndTopicConfiguration/context/ListAndTopicDatasetConfiguration.tspackages/app-builder/src/components/ListAndTopicConfiguration/index.tspackages/app-builder/src/server-fns/screenings.tspackages/app-builder/src/components/ContinuousScreening/form/steps/GeneralInfo.tsxpackages/app-builder/src/components/ListAndTopicConfiguration/dataset-utils.tspackages/app-builder/src/constants/ftm-entities.tspackages/app-builder/src/routes/_app/_builder/detection/scenarios/$scenarioId/i/$iterationId/screenings.$screeningId.tsxpackages/app-builder/src/components/ContinuousScreening/form/steps/ObjectMapping.tsxpackages/app-builder/src/components/CopyToClipboardButton.tsxpackages/app-builder/src/components/ContinuousScreening/form/steps/DatasetSelection.tsxpackages/app-builder/src/queries/screening/lists-config.tspackages/app-builder/src/models/continuous-screening.tspackages/app-builder/src/models/screening.tspackages/app-builder/src/repositories/ScreeningRepository.tspackages/app-builder/src/queries/screening/datasets.tspackages/app-builder/src/components/ListAndTopicConfiguration/DatasetSelectionContent.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/DatasetsPopover.tsxpackages/app-builder/src/components/Scenario/Screening/FieldDataset.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/components/ContinuousScreening/context/CreationStepper.tsxpackages/app-builder/src/components/ContinuousScreening/CreationPage.tsxpackages/ui-design-system/src/Tabs/Tabs.stories.tsxpackages/app-builder/src/components/ContinuousScreening/shared/RecapRow.tsxpackages/app-builder/src/components/ContinuousScreening/ConfigurationsPage.tsxpackages/app-builder/src/components/ContinuousScreening/CopyToClipboardChip.tsxpackages/app-builder/src/components/ContinuousScreening/validation/DatasetSelectionSection.tsxpackages/app-builder/src/components/ContinuousScreening/form/Content.tsxpackages/app-builder/src/components/ListAndTopicConfiguration/dataset-selection-provider-utils.tspackages/app-builder/src/components/ContinuousScreening/context/ListAndTopicDatasetConfigurationBridge.tsxpackages/app-builder/src/components/ContinuousScreening/ConfigurationPanel.tsxpackages/app-builder/src/components/ContinuousScreening/form/recaps/DatasetSelectionRecap.tsxpackages/app-builder/src/components/ListAndTopicConfiguration/context/ListAndTopicDatasetConfiguration.tspackages/app-builder/src/components/ListAndTopicConfiguration/index.tspackages/app-builder/src/server-fns/screenings.tspackages/app-builder/src/components/ContinuousScreening/form/steps/GeneralInfo.tsxpackages/app-builder/src/components/ListAndTopicConfiguration/dataset-utils.tspackages/app-builder/src/constants/ftm-entities.tspackages/ui-design-system/src/Collapsible/Collapsible.tsxpackages/app-builder/src/routes/_app/_builder/detection/scenarios/$scenarioId/i/$iterationId/screenings.$screeningId.tsxpackages/ui-design-system/src/Checkbox/Checkbox.tsxpackages/app-builder/src/components/ContinuousScreening/form/steps/ObjectMapping.tsxpackages/app-builder/src/components/CopyToClipboardButton.tsxpackages/app-builder/src/components/ContinuousScreening/form/steps/DatasetSelection.tsxpackages/app-builder/src/queries/screening/lists-config.tspackages/app-builder/src/models/continuous-screening.tspackages/app-builder/src/models/screening.tspackages/app-builder/src/repositories/ScreeningRepository.tspackages/app-builder/src/queries/screening/datasets.tspackages/app-builder/src/components/ListAndTopicConfiguration/DatasetSelectionContent.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/DatasetsPopover.tsxpackages/app-builder/src/components/Scenario/Screening/FieldDataset.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/Tabs/Tabs.stories.tsxpackages/ui-design-system/src/Collapsible/Collapsible.tsxpackages/ui-design-system/src/Checkbox/Checkbox.tsx
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.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/components/ContinuousScreening/context/CreationStepper.tsxpackages/app-builder/src/components/ContinuousScreening/CreationPage.tsxpackages/app-builder/src/components/ContinuousScreening/shared/RecapRow.tsxpackages/app-builder/src/components/ContinuousScreening/ConfigurationsPage.tsxpackages/app-builder/src/components/ContinuousScreening/CopyToClipboardChip.tsxpackages/app-builder/src/components/ContinuousScreening/validation/DatasetSelectionSection.tsxpackages/app-builder/src/components/ContinuousScreening/form/Content.tsxpackages/app-builder/src/components/ContinuousScreening/context/ListAndTopicDatasetConfigurationBridge.tsxpackages/app-builder/src/components/ContinuousScreening/ConfigurationPanel.tsxpackages/app-builder/src/components/ContinuousScreening/form/recaps/DatasetSelectionRecap.tsxpackages/app-builder/src/components/ContinuousScreening/form/steps/GeneralInfo.tsxpackages/app-builder/src/routes/_app/_builder/detection/scenarios/$scenarioId/i/$iterationId/screenings.$screeningId.tsxpackages/app-builder/src/components/ContinuousScreening/form/steps/ObjectMapping.tsxpackages/app-builder/src/components/CopyToClipboardButton.tsxpackages/app-builder/src/components/ContinuousScreening/form/steps/DatasetSelection.tsxpackages/app-builder/src/components/ListAndTopicConfiguration/DatasetSelectionContent.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/DatasetsPopover.tsxpackages/app-builder/src/components/Scenario/Screening/FieldDataset.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/ContinuousScreening/context/CreationStepper.tsxpackages/app-builder/src/components/ContinuousScreening/CreationPage.tsxpackages/ui-design-system/src/Tabs/Tabs.stories.tsxpackages/app-builder/src/components/ContinuousScreening/shared/RecapRow.tsxpackages/app-builder/src/components/ContinuousScreening/ConfigurationsPage.tsxpackages/app-builder/src/components/ContinuousScreening/CopyToClipboardChip.tsxpackages/app-builder/src/components/ContinuousScreening/validation/DatasetSelectionSection.tsxpackages/app-builder/src/components/ContinuousScreening/form/Content.tsxpackages/app-builder/src/components/ContinuousScreening/context/ListAndTopicDatasetConfigurationBridge.tsxpackages/app-builder/src/components/ContinuousScreening/ConfigurationPanel.tsxpackages/app-builder/src/components/ContinuousScreening/form/recaps/DatasetSelectionRecap.tsxpackages/app-builder/src/components/ContinuousScreening/form/steps/GeneralInfo.tsxpackages/ui-design-system/src/Collapsible/Collapsible.tsxpackages/app-builder/src/routes/_app/_builder/detection/scenarios/$scenarioId/i/$iterationId/screenings.$screeningId.tsxpackages/ui-design-system/src/Checkbox/Checkbox.tsxpackages/app-builder/src/components/ContinuousScreening/form/steps/ObjectMapping.tsxpackages/app-builder/src/components/CopyToClipboardButton.tsxpackages/app-builder/src/components/ContinuousScreening/form/steps/DatasetSelection.tsxpackages/app-builder/src/components/ListAndTopicConfiguration/DatasetSelectionContent.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/DatasetsPopover.tsxpackages/app-builder/src/components/Scenario/Screening/FieldDataset.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 (48)
packages/app-builder/src/components/ContinuousScreening/context/CreationStepper.tsx (1)
76-77: LGTM!packages/app-builder/src/components/ContinuousScreening/CreationPage.tsx (2)
28-42: LGTM!
46-58: LGTM!packages/ui-design-system/src/Tabs/Tabs.stories.tsx (2)
3-3: LGTM!
27-27: LGTM!packages/app-builder/src/locales/en/continuous-screening.json (2)
11-30: LGTM!
39-39: LGTM!packages/app-builder/src/components/ContinuousScreening/shared/RecapRow.tsx (2)
1-1: LGTM!
9-14: LGTM!packages/app-builder/src/components/ContinuousScreening/ConfigurationsPage.tsx (3)
11-15: LGTM!
24-30: LGTM!
132-138: LGTM!packages/app-builder/src/components/ContinuousScreening/CopyToClipboardChip.tsx (1)
4-6: LGTM!packages/app-builder/src/components/ContinuousScreening/validation/DatasetSelectionSection.tsx (4)
7-7: LGTM!
39-39: LGTM!
55-56: LGTM!
74-75: LGTM!packages/marble-api/openapis/marblecore-api.yaml (1)
226-227: LGTM!packages/app-builder/src/components/ContinuousScreening/form/Content.tsx (2)
22-23: LGTM!
45-46: LGTM!packages/app-builder/src/components/ListAndTopicConfiguration/dataset-selection-provider-utils.ts (1)
7-9: LGTM!packages/app-builder/src/components/ContinuousScreening/context/ListAndTopicDatasetConfigurationBridge.tsx (1)
13-16: LGTM!packages/app-builder/src/components/ContinuousScreening/ConfigurationPanel.tsx (2)
57-68: LGTM!
40-46: LGTM!packages/app-builder/src/components/ContinuousScreening/form/recaps/DatasetSelectionRecap.tsx (2)
15-17: LGTM!
21-35: LGTM!packages/app-builder/src/components/ListAndTopicConfiguration/context/ListAndTopicDatasetConfiguration.ts (1)
12-23: LGTM!packages/app-builder/src/components/ListAndTopicConfiguration/index.ts (1)
1-8: LGTM!packages/app-builder/src/components/ContinuousScreening/form/steps/GeneralInfo.tsx (1)
17-19: LGTM!packages/app-builder/src/components/ListAndTopicConfiguration/dataset-utils.ts (2)
5-14: LGTM!
16-23: LGTM!packages/marble-api/openapis/marblecore-api/admin.yml (1)
597-638: LGTM!packages/marble-api/openapis/marblecore-api/screenings.yml (3)
37-57: LGTM!
403-449: LGTM!
450-520: LGTM!packages/app-builder/src/constants/ftm-entities.ts (3)
6-36: LGTM!
38-74: LGTM!
76-80: LGTM!packages/ui-design-system/src/Collapsible/Collapsible.tsx (2)
35-35: LGTM!
45-62: LGTM!packages/app-builder/src/routes/_app/_builder/detection/scenarios/$scenarioId/i/$iterationId/screenings.$screeningId.tsx (2)
66-84: LGTM!
878-878: ⚡ Quick winFieldDataset component signature verified—usage pattern is correct.
The component now accepts only
value(optionalstring[]) andonChange(optional callback), removing previous props likedefaultValue,onBlur, andsections. The component manages dataset selection state internally usingListAndTopicDatasetConfiguration, making the simplified API appropriate. The usage at line 878 matches the component signature perfectly.packages/app-builder/src/server-fns/screenings.ts (1)
112-112: ⚡ Quick winNo action needed—
availableFeaturesis already correctly typed.The constant is properly defined with
as constassertion and includes asatisfies Array<AvailableFeatures>constraint, which fully satisfies Zod 4's requirements forz.enum()usage. The code pattern is sound.packages/ui-design-system/src/Checkbox/Checkbox.tsx (1)
47-63: LGTM!packages/app-builder/src/components/CopyToClipboardButton.tsx (1)
14-14: LGTM!Also applies to: 52-52
packages/app-builder/src/components/ContinuousScreening/form/steps/DatasetSelection.tsx (1)
9-23: LGTM!packages/app-builder/src/models/screening.ts (1)
35-40: LGTM!Also applies to: 330-335, 462-464
packages/app-builder/src/components/ContinuousScreening/form/steps/ObjectMapping.tsx (1)
243-247: 💤 Low valueThe i18n key
continuousScreening:creation.objectMapping.configurator.fieldMapping.suggestis already defined in all locale files (English: "Suggest field mappings", French: "Suggérer des correspondances"). No action required.> Likely an incorrect or invalid review comment.
Comment on lines
+18
to
+20
| useEffect(() => { | ||
| listSharp.actions.setMode(wizardMode); | ||
| }, [listSharp, wizardMode]); |
There was a problem hiding this comment.
useEffect runs on every render due to listSharp dependency.
Since listSharp is created on every render (line 13), and it's included in the useEffect dependency array, the effect will execute on every render rather than only when wizardMode changes.
If the intent is to synchronize mode only when wizardMode changes, consider whether listSharp can be safely omitted from the dependencies. Alternatively, verify this pattern is correct for sharpstate usage.
🤖 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/ContinuousScreening/context/ListAndTopicDatasetConfigurationBridge.tsx`
around lines 18 - 20, The effect is re-running because listSharp is recreated
each render and included in the useEffect deps; either memoize or stabilize
listSharp (e.g., create it with useMemo/useRef so the reference doesn't change)
or remove listSharp from the dependency array if sharpstate’s API guarantees a
stable actions object; specifically ensure the value returned by listSharp (and
its actions property used by listSharp.actions.setMode) is stable so
useEffect([...wizardMode]) only triggers when wizardMode changes.
Comment on lines
+202
to
+221
| const handleSuggest = () => { | ||
| if (!ftmEntity) return; | ||
|
|
||
| const updatedFieldMapping = { ...mappingConfig.fieldMapping }; | ||
|
|
||
| for (const property of availableProperties) { | ||
| const key = `${ftmEntity}.${property}` as FtmEntityPropertyKey; | ||
| const suggestion = getFtmEntitySuggestion(key); | ||
| if (!suggestion) continue; | ||
|
|
||
| const field = table.fields.find( | ||
| (f) => | ||
| f.semanticType === suggestion.semanticType && | ||
| (suggestion.semanticSubType ? f.semanticSubType === suggestion.semanticSubType : true), | ||
| ); | ||
| if (field && !field.ftmProperty) updatedFieldMapping[field.id] = property; | ||
| } | ||
|
|
||
| onUpdate({ ...mappingConfig, fieldMapping: updatedFieldMapping }); | ||
| }; |
There was a problem hiding this comment.
Suggest may clobber the user's in-progress (unsaved) field mappings.
The guard !field.ftmProperty only inspects the persisted property from the data model. Any unsaved choices the user has made via the selector for that field live in mappingConfig.fieldMapping and will be happily overwritten when "Suggest" is clicked. Consider also skipping fields whose entry in the current updatedFieldMapping is already non-null:
♻️ Proposed tweak
- if (field && !field.ftmProperty) updatedFieldMapping[field.id] = property;
+ if (field && !field.ftmProperty && !updatedFieldMapping[field.id]) {
+ updatedFieldMapping[field.id] = property;
+ }Additionally, when several FTM properties share the same semanticType/semanticSubType, Array.prototype.find keeps returning the same first matching field, so later iterations overwrite earlier ones and only the last property "wins" — worth deciding whether that's intentional or whether each field should be claimed at most once per pass.
🤖 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/ContinuousScreening/form/steps/ObjectMapping.tsx`
around lines 202 - 221, handleSuggest currently only checks field.ftmProperty
(the persisted value) and can overwrite unsaved user selections in
mappingConfig.fieldMapping; update the logic in handleSuggest to also skip any
field whose id already has a non-null entry in the current updatedFieldMapping
(mappingConfig.fieldMapping) so you don't clobber in-progress choices. Also
prevent multiple FTM properties from claiming the same field by tracking claimed
field ids during the loop (e.g., maintain a Set of claimed field ids and skip
fields already claimed) when resolving suggestions via getFtmEntitySuggestion
for each key (constructed from ftmEntity and availableProperties) and searching
table.fields; after the loop call onUpdate({ ...mappingConfig, fieldMapping:
updatedFieldMapping }).
Comment on lines
+11
to
+21
| export function useListAndTopicDatasetConfigurationSharp(params: { | ||
| datasets: Record<string, boolean>; | ||
| mode: ListAndTopicDatasetConfigurationMode; | ||
| variant?: ListAndTopicDatasetConfigurationVariant; | ||
| }) { | ||
| return ListAndTopicDatasetConfiguration.createSharp({ | ||
| datasets: params.datasets, | ||
| mode: params.mode, | ||
| variant: params.variant, | ||
| }); | ||
| } |
There was a problem hiding this comment.
createSharp must not be wrapped in a hook.
Per project learnings, createSharp from the sharpstate library should be called unconditionally at the top level of a component function body, not inside hooks. This wrapper hook violates that pattern.
Instead, components should call ListAndTopicDatasetConfiguration.createSharp(...) directly at their top level, as seen in ListAndTopicDatasetConfigurationBridge.tsx line 13.
Consider removing this hook wrapper and having consumers call createSharp directly.
Based on learnings: 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.
🤖 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-selection-provider-utils.ts`
around lines 11 - 21, The wrapper hook useListAndTopicDatasetConfigurationSharp
calls ListAndTopicDatasetConfiguration.createSharp inside a hook which violates
the library's rule that createSharp must be invoked unconditionally at a
component's top level; remove this hook and update consumers to call
ListAndTopicDatasetConfiguration.createSharp(...) directly at the top of their
component functions (as done by ListAndTopicDatasetConfigurationBridge),
ensuring createSharp is not wrapped in useMemo/useCallback/useEffect or any
other hook or conditional.
Comment on lines
+83
to
+114
| function createContinuousScreeningFilters(selection: string[]): ScreeningConfigBodyFiltersDto { | ||
| const filters: ScreeningConfigBodyFiltersDto = { | ||
| sanctions: { enabled: false }, | ||
| peps: { enabled: false }, | ||
| adverse_media: { enabled: false }, | ||
| other: { enabled: false }, | ||
| }; | ||
| for (const item of selection) { | ||
| const chunks = item.split(':'); | ||
| const section = chunks[0] as keyof ScreeningConfigBodyFiltersDto; | ||
| if (!filters[section]) continue; | ||
| if (chunks.length === 1) filters[section].enabled = true; | ||
| if (chunks.length < 3) continue; | ||
| const type = chunks[1]; | ||
| if (type !== 'dataset' && type !== 'topic') continue; | ||
| const name = chunks[2] as string; | ||
| if (!name) continue; | ||
| if (type === 'dataset') { | ||
| if (!filters[section].datasets) filters[section].datasets = []; | ||
| filters[section].datasets.push(name); | ||
| continue; | ||
| } | ||
| const value = chunks[3] as string; | ||
| if (!value) continue; | ||
| if (type === 'topic') { | ||
| if (!filters[section].topics) filters[section].topics = {}; | ||
| if (!filters[section].topics[name]) filters[section].topics[name] = []; | ||
| filters[section].topics[name].push(value); | ||
| } | ||
| } | ||
| return filters; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the OpenAPI shape for ScreeningConfigBodyFiltersDto / per-section filter to see how enabled relates to datasets/topics
fd -e yml -e yaml . packages/marble-api | xargs rg -nP -C3 'ScreeningConfigBodyFilters|screening_config_body_filters|enabled:' 2>/dev/null | head -200Repository: checkmarble/marble-frontend
Length of output: 7793
🏁 Script executed:
cat -n packages/marble-api/openapis/marblecore-api/screenings.yml | sed -n '450,480p'Repository: checkmarble/marble-frontend
Length of output: 1153
🏁 Script executed:
cat -n packages/marble-api/openapis/marblecore-api/screenings.yml | sed -n '465,495p'Repository: checkmarble/marble-frontend
Length of output: 1192
🏁 Script executed:
# Search for any backend tests or implementation details about how enabled interacts with datasets/topics
rg -i "enabled.*false.*datasets|datasets.*enabled.*false" packages/marble-api --type yml --type json -C 3Repository: checkmarble/marble-frontend
Length of output: 99
🏁 Script executed:
# Check if there are any tests or backend code discussing the semantic meaning of enabled with empty vs populated datasets
rg -i "screening.*filter|section.*enabled" packages/ -t ts -t tsx -C 2 | head -150Repository: checkmarble/marble-frontend
Length of output: 99
🏁 Script executed:
find packages/marble-api -name "*.go" -o -name "*.java" -o -name "*.py" -o -name "*.rs" -o -name "*.cs" | head -10Repository: checkmarble/marble-frontend
Length of output: 53
🏁 Script executed:
# Look for backend or implementation files that might define screening logic
find packages -name "*screening*" -type f | grep -v node_modules | grep -v dist | head -20Repository: checkmarble/marble-frontend
Length of output: 1236
🏁 Script executed:
# Check the OpenAPI YAML for any description or comment about the enabled field
rg -A 5 -B 5 "enabled:" packages/marble-api/openapis/marblecore-api/screenings.yml | head -100Repository: checkmarble/marble-frontend
Length of output: 441
🏁 Script executed:
# Look for any tests or documentation in the repo that discuss screening filters
rg -i "screening.*enabled|enabled.*screening" . --type md --type json -C 2 | head -150Repository: checkmarble/marble-frontend
Length of output: 53
🏁 Script executed:
cat -n packages/app-builder/src/models/continuous-screening.ts | head -120Repository: checkmarble/marble-frontend
Length of output: 4401
🏁 Script executed:
cat -n packages/app-builder/src/models/screening-config.tsRepository: checkmarble/marble-frontend
Length of output: 3450
🏁 Script executed:
# Check if there are tests for createContinuousScreeningFilters
find packages/app-builder -name "*.test.ts" -o -name "*.test.tsx" | xargs rg -l "createContinuousScreeningFilters|continuous-screening" 2>/dev/nullRepository: checkmarble/marble-frontend
Length of output: 3315
🏁 Script executed:
# Look at how the form uses the filters—check the form submission or component
rg -i "createContinuousScreeningFilters|ScreeningConfigBodyFiltersDto" packages/app-builder/src --type ts --type tsx -B 3 -A 3Repository: checkmarble/marble-frontend
Length of output: 99
🏁 Script executed:
# Look at the form creation code
cat -n packages/app-builder/src/components/ContinuousScreening/CreationPage.tsx | head -100Repository: checkmarble/marble-frontend
Length of output: 2905
🏁 Script executed:
# Check the server function that handles creation
cat -n packages/app-builder/src/server-fns/continuous-screening.tsRepository: checkmarble/marble-frontend
Length of output: 5300
🏁 Script executed:
# Look for where selection is built - search in form components
rg -i "selection.*=" packages/app-builder/src/components/ContinuousScreening/ --type ts -B 2 -A 2 | head -100Repository: checkmarble/marble-frontend
Length of output: 5642
🏁 Script executed:
# Look at the complete adaptCreateContinuousScreeningConfigDto function
cat -n packages/app-builder/src/models/continuous-screening.ts | sed -n '116,150p'Repository: checkmarble/marble-frontend
Length of output: 1618
Set enabled: true when datasets or topics are added to a section.
When users select datasets or topics, createContinuousScreeningFilters populates the datasets or topics arrays but leaves enabled set to false (initialized at lines 85–88). The only code that sets enabled = true is line 94, which triggers only when chunks.length === 1—but form input always generates strings like "section:type:name[:value]" (chunks.length ≥ 3), making that branch unreachable. The result is configurations with populated datasets/topics but disabled sections, which the backend likely treats as inactive.
🐛 Proposed fix
if (type === 'dataset') {
if (!filters[section].datasets) filters[section].datasets = [];
filters[section].datasets.push(name);
+ filters[section].enabled = true;
continue;
}
const value = chunks[3] as string;
if (!value) continue;
if (type === 'topic') {
if (!filters[section].topics) filters[section].topics = {};
if (!filters[section].topics[name]) filters[section].topics[name] = [];
filters[section].topics[name].push(value);
+ filters[section].enabled = true;
}📝 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.
Suggested change
| function createContinuousScreeningFilters(selection: string[]): ScreeningConfigBodyFiltersDto { | |
| const filters: ScreeningConfigBodyFiltersDto = { | |
| sanctions: { enabled: false }, | |
| peps: { enabled: false }, | |
| adverse_media: { enabled: false }, | |
| other: { enabled: false }, | |
| }; | |
| for (const item of selection) { | |
| const chunks = item.split(':'); | |
| const section = chunks[0] as keyof ScreeningConfigBodyFiltersDto; | |
| if (!filters[section]) continue; | |
| if (chunks.length === 1) filters[section].enabled = true; | |
| if (chunks.length < 3) continue; | |
| const type = chunks[1]; | |
| if (type !== 'dataset' && type !== 'topic') continue; | |
| const name = chunks[2] as string; | |
| if (!name) continue; | |
| if (type === 'dataset') { | |
| if (!filters[section].datasets) filters[section].datasets = []; | |
| filters[section].datasets.push(name); | |
| continue; | |
| } | |
| const value = chunks[3] as string; | |
| if (!value) continue; | |
| if (type === 'topic') { | |
| if (!filters[section].topics) filters[section].topics = {}; | |
| if (!filters[section].topics[name]) filters[section].topics[name] = []; | |
| filters[section].topics[name].push(value); | |
| } | |
| } | |
| return filters; | |
| } | |
| function createContinuousScreeningFilters(selection: string[]): ScreeningConfigBodyFiltersDto { | |
| const filters: ScreeningConfigBodyFiltersDto = { | |
| sanctions: { enabled: false }, | |
| peps: { enabled: false }, | |
| adverse_media: { enabled: false }, | |
| other: { enabled: false }, | |
| }; | |
| for (const item of selection) { | |
| const chunks = item.split(':'); | |
| const section = chunks[0] as keyof ScreeningConfigBodyFiltersDto; | |
| if (!filters[section]) continue; | |
| if (chunks.length === 1) filters[section].enabled = true; | |
| if (chunks.length < 3) continue; | |
| const type = chunks[1]; | |
| if (type !== 'dataset' && type !== 'topic') continue; | |
| const name = chunks[2] as string; | |
| if (!name) continue; | |
| if (type === 'dataset') { | |
| if (!filters[section].datasets) filters[section].datasets = []; | |
| filters[section].datasets.push(name); | |
| filters[section].enabled = true; | |
| continue; | |
| } | |
| const value = chunks[3] as string; | |
| if (!value) continue; | |
| if (type === 'topic') { | |
| if (!filters[section].topics) filters[section].topics = {}; | |
| if (!filters[section].topics[name]) filters[section].topics[name] = []; | |
| filters[section].topics[name].push(value); | |
| filters[section].enabled = true; | |
| } | |
| } | |
| return filters; | |
| } |
🤖 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/continuous-screening.ts` around lines 83 -
114, In createContinuousScreeningFilters, sections that receive datasets or
topics are never marked enabled because the code only sets enabled when
chunks.length === 1; update the logic so that whenever you add a dataset or a
topic/value to filters[section] you also set filters[section].enabled = true
(e.g., just before pushing into filters[section].datasets or creating/pushing
into filters[section].topics[name]) to ensure sections with populated
datasets/topics are active; reference the createContinuousScreeningFilters
function and the filters[section].datasets, filters[section].topics and
filters[section].enabled fields when making this change.
Comment on lines
+28
to
+32
| return Object.entries(R.groupBy(datasets, (d) => d.section ?? d.name)).map(([section, items]) => ({ | ||
| name: section, | ||
| title: section, | ||
| datasets: items.map(({ name, title }) => ({ name, title })), | ||
| })); |
There was a problem hiding this comment.
groupBySection collapses name and title into the same value.
The grouped output uses section (the grouping key) for both name and title. When section is omitted, that key falls back to the dataset's (now mutated/namespaced) name, which means the user-facing title will read something like sanctions:dataset:foo instead of the dataset's actual title. If a single ungrouped dataset is expected here, consider preserving its original title:
♻️ Proposed tweak
- return Object.entries(R.groupBy(datasets, (d) => d.section ?? d.name)).map(([section, items]) => ({
- name: section,
- title: section,
- datasets: items.map(({ name, title }) => ({ name, title })),
- }));
+ return Object.entries(R.groupBy(datasets, (d) => d.section ?? d.name)).map(([section, items]) => ({
+ name: section,
+ title: items[0]?.section ? section : (items[0]?.title ?? section),
+ datasets: items.map(({ name, title }) => ({ name, title })),
+ }));📝 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.
Suggested change
| return Object.entries(R.groupBy(datasets, (d) => d.section ?? d.name)).map(([section, items]) => ({ | |
| name: section, | |
| title: section, | |
| datasets: items.map(({ name, title }) => ({ name, title })), | |
| })); | |
| return Object.entries(R.groupBy(datasets, (d) => d.section ?? d.name)).map(([section, items]) => ({ | |
| name: section, | |
| title: items[0]?.section ? section : (items[0]?.title ?? section), | |
| datasets: items.map(({ name, title }) => ({ name, title })), | |
| })); |
🤖 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/lists-config.ts` around lines 28 -
32, The grouping currently sets both container name and title to the grouping
key (section), which can collapse a dataset's original title into a namespaced
dataset name; update the mapping produced from
Object.entries(R.groupBy(datasets, (d) => d.section ?? d.name)) so that for a
group with a single ungrouped dataset (items.length === 1 and items[0].section
== null/undefined) you keep name: section but set title to items[0].title,
otherwise keep title: section; reference the variables datasets, section, items,
name, and title when making this change in the groupBySection mapping.
| { name: 'africa', title: 'Africa' }, | ||
| { name: 'asia', title: 'Asia' }, | ||
| { name: 'europe', title: 'Europe' }, | ||
| { name: 'north-aerica', title: 'North America' }, |
There was a problem hiding this comment.
Tiny typo in the test fixture.
'north-aerica' should be 'north-america'. A small thing — but if this mock ever escapes into a UI snapshot or screenshot, that continent will look most peculiar:
- { name: 'north-aerica', title: 'North America' },
+ { name: 'north-america', title: 'North America' },📝 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.
Suggested change
| { name: 'north-aerica', title: 'North America' }, | |
| { name: 'north-america', title: 'North America' }, |
🤖 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/repositories/ScreeningRepository.ts` at line 75, Fix
the typo in the test fixture inside ScreeningRepository by changing the
continent identifier string 'north-aerica' to 'north-america' so the object {
name: 'north-aerica', title: 'North America' } uses the correct name; locate the
object literal in ScreeningRepository (the array/fixture that contains the {
name: ..., title: ... } entries) and update the name field accordingly.
Comment on lines
+362
to
+363
| filters: | ||
| $ref: 'screenings.yml#/components/schemas/ScreeningConfigBodyFiltersDto' |
There was a problem hiding this comment.
The new filters field is create-only in the contract.
CreateContinuousScreeningConfigDto now accepts filters, but ContinuousScreeningConfigDto and UpdateContinuousScreeningConfigDto still omit it. That means a client can create a filtered config yet has no schema to read the same value back or patch it later. Either add filters to the read/update DTOs as well, or hold this field back until the round-trip is supported.
🤖 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/marble-api/openapis/marblecore-api/continuous-screenings.yml` around
lines 362 - 363, Create symmetry for the new filters field by adding the same
filters schema reference to the read and update DTOs so created configs can be
read and patched; specifically, add filters: $ref:
'screenings.yml#/components/schemas/ScreeningConfigBodyFiltersDto' to the
ContinuousScreeningConfigDto and UpdateContinuousScreeningConfigDto definitions
(or alternatively remove it from CreateContinuousScreeningConfigDto if you
prefer to postpone support) so the CreateContinuousScreeningConfigDto,
ContinuousScreeningConfigDto, and UpdateContinuousScreeningConfigDto are
consistent.
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/FreeformSearch/FreeformSearchForm.tsx (1)
90-93:⚠️ Potential issue | 🟠 Major | ⚡ Quick winClear filters should also refresh results.
handleClearFiltersresets UI state but does not submit, so displayed results can remain stale versus the cleared filter state.Proposed adjustment
const handleClearFilters = () => { form.reset(); setSelectedDatasets([]); + form.handleSubmit(); };🤖 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/FreeformSearchForm.tsx` around lines 90 - 93, handleClearFilters currently calls form.reset() and setSelectedDatasets([]) but doesn't refresh the displayed results; after resetting state either call the form submission flow (e.g., form.submit() or invoke the component's existing search/submit handler such as handleSearch/ onSubmit) or call the function that fetches results so the UI updates to the cleared filters; update the implementation of handleClearFilters to call that submit/fetch function immediately after form.reset() and setSelectedDatasets([]) to ensure results refresh.
🧹 Nitpick comments (2)
packages/app-builder/src/components/Screenings/FreeformSearch/FreeformSearchForm.tsx (1)
47-50: ⚡ Quick winFix typo in exported hook name (
Manuall).Public API typo will propagate to callers and future maintenance.
Proposed adjustment
-export function useFormManuallSearch() { +export function useFormManualSearch() { const form = useContext(ManualSearchFormContext); - if (!form) throw new Error('useFormManuallSearch must be used within FreeformSearchForm'); + if (!form) throw new Error('useFormManualSearch must be used within FreeformSearchForm'); return form; }🤖 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/FreeformSearchForm.tsx` around lines 47 - 50, Rename the exported hook from useFormManuallSearch to useFormManualSearch across the module and all callers: update the function declaration name, any exported identifier, the thrown Error message string ('useFormManuallSearch must be used within FreeformSearchForm' -> 'useFormManualSearch must be used within FreeformSearchForm'), and update any imports/usages that reference useFormManuallSearch; keep the internals that access ManualSearchFormContext and the existing behavior unchanged.packages/app-builder/src/repositories/ScreeningRepository.ts (1)
63-127: ⚡ Quick winRemove the large commented testing fixture block before merge.
Keeping long dead test fixtures in production code increases noise and risks accidental re-enable.
🤖 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/repositories/ScreeningRepository.ts` around lines 63 - 127, Remove the large commented-out testing fixture block (the commented assignment to listFeature.sections.peps and listFeature.conditional_filters) from ScreeningRepository.ts; locate the block by searching for the symbols listFeature, sections.peps, and conditional_filters within the ScreeningRepository class or surrounding method and delete the entire commented section so only production code remains, then run the project's linters/tests to ensure no leftover references or formatting issues.
🤖 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/FreeformSearch/FreeformSearchForm.tsx`:
- Around line 154-157: The onChange handler in FreeformSearchForm currently
calls form.setFieldValue('threshold', value) and immediately
form.handleSubmit(), causing a request on every slider tick; change this so the
slider only updates the form value on onChange (keep
form.setFieldValue('threshold', value)) but do NOT call form.handleSubmit there
— instead trigger submission on commit/apply (e.g., the slider's
onChangeCommitted or the component's apply button) or implement a debounce
around form.handleSubmit so the search is sent only after the user finishes
adjusting; locate the handlers that reference form.setFieldValue and
form.handleSubmit in FreeformSearchForm and move or wrap the submit logic
accordingly.
In `@packages/app-builder/src/repositories/ScreeningRepository.ts`:
- Line 11: Change the import of ScreeningAvailableFiltersAdapted to a type-only
import; currently it’s imported alongside runtime values but is only used in
type positions (return type in the function using adaptScreening* at the
location referenced and a variable annotation later). Update the import list to
use "type ScreeningAvailableFiltersAdapted" so the symbol is treated as a
compile-time-only type and removed from runtime imports, leaving other imports
(e.g., adaptScreening, adaptScreeningFile, adaptScreeningMatch,
adaptScreeningMatchPayload, type Screening, etc.) unchanged.
---
Outside diff comments:
In
`@packages/app-builder/src/components/Screenings/FreeformSearch/FreeformSearchForm.tsx`:
- Around line 90-93: handleClearFilters currently calls form.reset() and
setSelectedDatasets([]) but doesn't refresh the displayed results; after
resetting state either call the form submission flow (e.g., form.submit() or
invoke the component's existing search/submit handler such as handleSearch/
onSubmit) or call the function that fetches results so the UI updates to the
cleared filters; update the implementation of handleClearFilters to call that
submit/fetch function immediately after form.reset() and setSelectedDatasets([])
to ensure results refresh.
---
Nitpick comments:
In
`@packages/app-builder/src/components/Screenings/FreeformSearch/FreeformSearchForm.tsx`:
- Around line 47-50: Rename the exported hook from useFormManuallSearch to
useFormManualSearch across the module and all callers: update the function
declaration name, any exported identifier, the thrown Error message string
('useFormManuallSearch must be used within FreeformSearchForm' ->
'useFormManualSearch must be used within FreeformSearchForm'), and update any
imports/usages that reference useFormManuallSearch; keep the internals that
access ManualSearchFormContext and the existing behavior unchanged.
In `@packages/app-builder/src/repositories/ScreeningRepository.ts`:
- Around line 63-127: Remove the large commented-out testing fixture block (the
commented assignment to listFeature.sections.peps and
listFeature.conditional_filters) from ScreeningRepository.ts; locate the block
by searching for the symbols listFeature, sections.peps, and conditional_filters
within the ScreeningRepository class or surrounding method and delete the entire
commented section so only production code remains, then run the project's
linters/tests to ensure no leftover references or formatting issues.
🪄 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: dccd1612-894c-4125-b0bb-546fca4fb650
📒 Files selected for processing (6)
packages/app-builder/src/components/ListAndTopicConfiguration/DatasetSelectionContent.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/FreeformSearchForm.tsxpackages/app-builder/src/locales/ar/screenings.jsonpackages/app-builder/src/locales/en/screenings.jsonpackages/app-builder/src/locales/fr/screenings.jsonpackages/app-builder/src/repositories/ScreeningRepository.ts
✅ Files skipped from review due to trivial changes (1)
- packages/app-builder/src/locales/fr/screenings.json
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/app-builder/src/locales/ar/screenings.json
- packages/app-builder/src/components/ListAndTopicConfiguration/DatasetSelectionContent.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: check / main
- GitHub Check: e2e
🧰 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/components/Screenings/FreeformSearch/FreeformSearchForm.tsxpackages/app-builder/src/repositories/ScreeningRepository.ts
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/components/Screenings/FreeformSearch/FreeformSearchForm.tsxpackages/app-builder/src/repositories/ScreeningRepository.ts
Comment on lines
+154
to
+157
| onChange={(value) => { | ||
| form.setFieldValue('threshold', value); | ||
| form.handleSubmit(); | ||
| }} |
There was a problem hiding this comment.
Avoid auto-submitting on every threshold tick.
This sends a search request on each slider change, which can flood the API during drag interactions; a storm of requests doth follow. Submit on commit/apply instead (or debounce).
Proposed adjustment
onChange={(value) => {
form.setFieldValue('threshold', value);
- form.handleSubmit();
}}
+ // Prefer submitting on an explicit apply/commit event if available🤖 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/FreeformSearchForm.tsx`
around lines 154 - 157, The onChange handler in FreeformSearchForm currently
calls form.setFieldValue('threshold', value) and immediately
form.handleSubmit(), causing a request on every slider tick; change this so the
slider only updates the form value on onChange (keep
form.setFieldValue('threshold', value)) but do NOT call form.handleSubmit there
— instead trigger submission on commit/apply (e.g., the slider's
onChangeCommitted or the component's apply button) or implement a debounce
around form.handleSubmit so the search is sent only after the user finishes
adjusting; locate the handlers that reference form.setFieldValue and
form.handleSubmit in FreeformSearchForm and move or wrap the submit logic
accordingly.
| adaptScreeningMatchPayload, | ||
| type OpenSanctionEntitySchema, | ||
| type Screening, | ||
| ScreeningAvailableFiltersAdapted, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Find ScreeningAvailableFiltersAdapted declaration =="
fd -i 'screening.ts' | xargs -I{} rg -n "ScreeningAvailableFiltersAdapted" {}
echo
echo "== Check TS module import settings =="
fd -i 'tsconfig*.json' | xargs -I{} rg -n "\"verbatimModuleSyntax\"|\"importsNotUsedAsValues\"|\"preserveValueImports\"" {}
echo
echo "== Find non-type imports used only as types in this file =="
rg -n "ScreeningAvailableFiltersAdapted" packages/app-builder/src/repositories/ScreeningRepository.tsRepository: checkmarble/marble-frontend
Length of output: 210
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check full context around line 11 in ScreeningRepository.ts =="
head -20 packages/app-builder/src/repositories/ScreeningRepository.ts | cat -n
echo
echo "== Check lines 55-70 for context around lines 61-62 =="
sed -n '55,70p' packages/app-builder/src/repositories/ScreeningRepository.ts | cat -n
echo
echo "== Check all imports at top of ScreeningRepository.ts =="
head -30 packages/app-builder/src/repositories/ScreeningRepository.ts
echo
echo "== Check tsconfig for strict module syntax settings =="
fd -type f 'tsconfig.*json' -path '*/app-builder/*' -o -path '*/tsconfig.json' | head -5 | xargs -I{} sh -c 'echo "File: {}"; cat {}'Repository: checkmarble/marble-frontend
Length of output: 3341
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Find and check tsconfig.json files =="
find . -name "tsconfig*.json" -type f | grep -E "(root|app-builder)" | head -3 | while read f; do
echo "File: $f"
cat "$f" | grep -E "verbatimModuleSyntax|importsNotUsedAsValues|preserveValueImports" || echo " (no strict module settings found)"
done
echo
echo "== Find all uses of ScreeningAvailableFiltersAdapted in ScreeningRepository.ts =="
rg "ScreeningAvailableFiltersAdapted" packages/app-builder/src/repositories/ScreeningRepository.ts -n -B1 -A1
echo
echo "== Check the declaration in screening model =="
rg "export type ScreeningAvailableFiltersAdapted" packages/app-builder/src/models/ -n -A2Repository: checkmarble/marble-frontend
Length of output: 1138
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check root tsconfig.json for strict settings =="
if [ -f "tsconfig.json" ]; then
cat tsconfig.json | grep -E "verbatimModuleSyntax|importsNotUsedAsValues|preserveValueImports|extends" || echo "(no strict module settings found in root)"
else
echo "No root tsconfig.json found"
fi
echo
echo "== Double-check: is ScreeningAvailableFiltersAdapted ever used as a value (not type)? =="
rg "ScreeningAvailableFiltersAdapted" packages/app-builder/src/repositories/ScreeningRepository.ts --vimgrep | grep -v "Promise\|:\s" || echo "(All uses are in type positions)"Repository: checkmarble/marble-frontend
Length of output: 273
Import ScreeningAvailableFiltersAdapted as a type-only import.
This symbol is a type alias and is used only in type positions (return type annotation at line 55 and variable type annotation at line 61). Change the import on line 11 to type ScreeningAvailableFiltersAdapted to clarify intent and reduce unnecessary runtime overhead.
Current import (lines 3–16)
import {
type AvailableFeatures,
adaptScreening,
adaptScreeningFile,
adaptScreeningMatch,
adaptScreeningMatchPayload,
type OpenSanctionEntitySchema,
type Screening,
ScreeningAvailableFiltersAdapted,
type ScreeningFile,
type ScreeningMatch,
type ScreeningMatchPayload,
type ScreeningMatchStatus,
} from '@app-builder/models/screening';
To thine own code be true—import but what thou needest, as a type.
🤖 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/repositories/ScreeningRepository.ts` at line 11,
Change the import of ScreeningAvailableFiltersAdapted to a type-only import;
currently it’s imported alongside runtime values but is only used in type
positions (return type in the function using adaptScreening* at the location
referenced and a variable annotation later). Update the import list to use "type
ScreeningAvailableFiltersAdapted" so the symbol is treated as a
compile-time-only type and removed from runtime imports, leaving other imports
(e.g., adaptScreening, adaptScreeningFile, adaptScreeningMatch,
adaptScreeningMatchPayload, type Screening, etc.) unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Manual search redesign
Filter by list
The filter by list remains the same as I need the #1503 to be merged to recover the new list components
Summary by CodeRabbit
New Features
UI/UX Improvements
Localization