MAR-1853 MAR-1835: Implement frontend for the new configuration#1503
MAR-1853 MAR-1835: Implement frontend for the new configuration#1503william-schlegel wants to merge 33 commits into
Conversation
william-schlegel
changed the title
…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.
william-schlegel
requested review from
ChibiBlasphem and
Pascal-Delange
and removed request for
Pascal-Delange
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughConsolidates dataset/topic selection into a new ListAndTopicConfiguration system (server endpoint, normalized hook, Sharp-backed provider, DatasetSelectionContent UI) and integrates it into continuous screening, scenario FieldDataset, and freeform search; includes related type, schema, UI primitive, and locale updates. ChangesDataset Selection Component System Refactor
Sequence DiagramsequenceDiagram
participant Browser
participant Frontend as App (DatasetSelectionContent)
participant Server as getListConfigFn
participant Repo as ScreeningRepository
participant CoreAPI as MarbleCoreAPI
Browser->>Frontend: open dataset selection (useCase)
Frontend->>Server: GET /screenings/available-filters?feature=useCase
Server->>Repo: getAvailableFilters(feature)
Repo->>CoreAPI: listScreeningAvailableFilters(feature)
CoreAPI-->>Repo: available filters payload
Repo-->>Server: ScreeningAvailableFiltersAdapted
Server-->>Frontend: normalized list config
Frontend->>Frontend: render sections, update shared state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
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/ContinuousScreening/validation/DatasetSelectionSection.tsx (1)
37-43:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid blanking the whole section when
sectionsis absent.If the query succeeds but
datasets.sectionsis missing, returningnullhides all added/removed changes in the confirmation step. Better to keep rendering with an empty metadata list so keys still appear. Lest truth of changes vanish into silence.Suggested fix
- if (!datasets?.sections) return null; - const datasetsArray = R.pipe( - datasets.sections, - R.flatMap(R.prop('datasets')), - R.map((d) => ({ name: d.name, tag: d.tag, title: d.title })), - ); + const datasetsArray = datasets?.sections + ? R.pipe( + datasets.sections, + R.flatMap(R.prop('datasets')), + R.map((d) => ({ name: d.name, tag: d.tag, title: d.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/components/ContinuousScreening/validation/DatasetSelectionSection.tsx` around lines 37 - 43, In the with({ isSuccess: true }, ({ data: { datasets } }) => { ... }) block, don't return null when datasets?.sections is missing because that hides the whole section; instead set datasetsArray to an empty array and continue rendering so keys and metadata placeholders remain visible. Modify the logic around datasetsArray (the R.pipe that flatMaps datasets.sections and maps to {name, tag, title}) to handle missing or undefined datasets.sections by using a fallback (e.g., []) before flatMap, or conditionally assign datasetsArray = [] when !datasets?.sections, and let the rest of the component render normally.
🧹 Nitpick comments (6)
packages/app-builder/src/components/CopyToClipboardButton.tsx (1)
14-14: ⚡ Quick winRemove duplicate CSS classes from the chip variant.
The
chipvariant duplicates several classes already present in the base configuration (line 8):cursor-pointer,border,flex, anditems-center. This redundancy can complicate maintenance and may cause unexpected specificity behavior.♻️ Proposed fix to remove duplicates
- chip: 'cursor-pointer border border-grey-border py-v2-xs px-v2-sm flex items-center gap-v2-xs font-semibold text-small bg-surface-card', + chip: 'border-grey-border py-v2-xs px-v2-sm gap-v2-xs font-semibold text-small bg-surface-card',🤖 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/CopyToClipboardButton.tsx` at line 14, The chip variant duplicates utility classes already defined in the base variant; update the chip entry in the style config to remove redundant classes (remove cursor-pointer, border, flex, items-center from the 'chip' variant) leaving only chip-specific classes like gap-v2-xs, font-semibold, text-small, bg-surface-card and any unique padding classes so the base config provides shared utilities and 'chip' only supplies its unique overrides.packages/app-builder/src/components/ListAndTopicConfiguration/dataset-selection-provider-utils.ts (1)
7-10: ⚡ Quick winReuse the exported mode type to avoid drift.
The mode union is duplicated here; importing
ListAndTopicDatasetConfigurationModekeeps this utility aligned if modes evolve.Suggested fix
-import { ListAndTopicDatasetConfiguration } from './context/ListAndTopicDatasetConfiguration'; +import { + ListAndTopicDatasetConfiguration, + type ListAndTopicDatasetConfigurationMode, +} from './context/ListAndTopicDatasetConfiguration'; @@ export function useListAndTopicDatasetConfigurationSharp(params: { datasets: Record<string, boolean>; - mode: 'view' | 'edit' | 'create'; + mode: ListAndTopicDatasetConfigurationMode; }) {🤖 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 7 - 10, The params type for useListAndTopicDatasetConfigurationSharp duplicates the 'mode' union; import and use the shared ListAndTopicDatasetConfigurationMode type instead of repeating the union. Update the function signature to import ListAndTopicDatasetConfigurationMode and replace mode: 'view' | 'edit' | 'create' with mode: ListAndTopicDatasetConfigurationMode so the utility stays in sync with the exported mode definition.packages/app-builder/src/repositories/ScreeningRepository.ts (1)
21-56: ⚡ Quick winKeep the repository contract independent from
server-fns.
ScreeningRepositorynow importsAvailableFeaturesfrom@app-builder/server-fns/screenings, which couples the data layer to the RPC layer. Please move that shared feature type to a model/shared-types module and have both sides import it from there.As per coding guidelines, "Place data access logic in repositories directory following the Repository pattern" and "Use type definitions with adapters in the models directory."
🤖 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 21 - 56, The ScreeningRepository currently imports AvailableFeatures from `@app-builder/server-fns/screenings` which couples the repo to the RPC layer; extract/define the AvailableFeatures type into a shared models module (e.g., models/shared-types or models/features) and update ScreeningRepository to import AvailableFeatures from that new module instead of server-fns; then update the RPC/service code (the file exporting `@app-builder/server-fns/screenings`) to import and re-export or directly use the new shared type so both sides reference the same model type; finally run typechecks and adjust any import paths referencing AvailableFeatures across the codebase (including getAvailableFilters signature) to the new shared module.packages/app-builder/src/constants/ftm-entities.ts (2)
1-1: ⚡ Quick winUse a concrete
@app-builder/models/*import here.This new root import breaks the repo alias rule and makes the model boundary less explicit. Please switch it to the matching
@app-builder/models/...entrypoint.As per coding guidelines, "Import internal app-builder models using the path alias
@app-builder/models/*"🤖 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/constants/ftm-entities.ts` at line 1, The import of SemanticSubTypeField and SemanticTypeField uses a root `@app-builder/models` barrel which violates the repo alias rule; update the import to the concrete subpath that directly exports these symbols (replace `import { SemanticSubTypeField, SemanticTypeField } from '@app-builder/models';` with an import from the specific `@app-builder/models/...` entrypoint that exposes SemanticSubTypeField and SemanticTypeField) so the model boundary is explicit.
6-36: ⚡ Quick winChange the
satisfiestarget to usereadonly string[]for semantic correctness.While the current code (
Record<FtmEntity, string[]>) passes type-check, theas constassertion makes these nested arrays readonly. Usingreadonly string[]in the target type explicitly reflects what the code actually produces, making the intent clearer and aligning with TypeScript best practices for const-asserted values.Suggested fix
-} as const satisfies Record<FtmEntity, string[]>; +} as const satisfies Record<FtmEntity, readonly string[]>;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/app-builder/src/constants/ftm-entities.ts` around lines 6 - 36, The const-asserted FTM_ENTITIES_PROPERTIES uses readonly arrays but the satisfies target is Record<FtmEntity, string[]>; update the satisfies clause to reflect readonly arrays by changing the target type to Record<FtmEntity, readonly string[]> so the declared type matches the `as const` structure (locate the FTM_ENTITIES_PROPERTIES declaration and the FtmEntity type reference to make this change).packages/app-builder/src/server-fns/screenings.ts (1)
114-119: ⚡ Quick winRename
avalableFeaturesbefore the typo spreads.This is a new exported symbol, so the misspelling will get sticky fast once other call sites import it. I'd fix it to
availableFeatureswhile the surface area is still small.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/app-builder/src/server-fns/screenings.ts` around lines 114 - 119, The exported const is misspelled as avalableFeatures; rename it to availableFeatures and update its export and all usages (including the z.enum call in getAvailableFiltersSchema) to use the new identifier so imports don't break; ensure the const assertion and satisfies clause remain intact (export const availableFeatures = [ ... ] as const satisfies Array<AvailableFeatures>) and update any external import sites to reference availableFeatures instead of avalableFeatures.
🤖 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 13-20: ListAndTopicDatasetConfiguration.createSharp is being
called on every render which recreates the store and churns Provider consumers;
wrap the call to ListAndTopicDatasetConfiguration.createSharp({ datasets:
datasetsMap, mode: wizardMode }) in a useMemo that depends only on datasetsMap
(not a spread copy) so the store instance is stable, and keep the existing
useEffect that calls listSharp.actions.setMode(wizardMode) to update mode when
wizardMode changes.
In
`@packages/app-builder/src/components/ListAndTopicConfiguration/DatasetSelectionContent.tsx`:
- Around line 421-433: The icon-only <button> that calls listConfig.update(...)
and onAfterChange should get an accessible, translated aria-label so screen
readers announce the action and target; update the JSX for the buttons (the one
using Icon and the similar one at lines ~589-594) to pass a localized string
such as `t('addDataset', { name: item.name })` or `t('openGroup', { name:
sectionKey })` into an aria-label prop (e.g., aria-label={...}) and ensure the
label describes the action and the item/group, using the existing i18n helper in
the component; keep the visual appearance unchanged.
- Around line 274-287: The row's onClick (onClickItem) and the Checkbox's
onClick both fire causing double toggles; update the Checkbox usage in
DatasetSelectionContent (the Checkbox component) to stop event propagation in
its handler (e.g., change the Checkbox handler to accept the event and call
event.stopPropagation() before toggling or use onChange with
event.stopPropagation()), or remove the Checkbox onClick and only handle
selection via its onChange while preventing the click from bubbling to the
parent. Ensure references: the outer div with onClick={onClickItem} and the
Checkbox component are the locations to modify.
- Around line 55-58: The current filter in DatasetSelectionContent.tsx drops
sections that only have conditionalTopics; update the predicate used before
mapping so sections are kept if they have datasets, topics, OR conditionalTopics
(e.g. change ([, section]) => section.datasets?.length || section.topics to ([,
section]) => section.datasets?.length || section.topics ||
section.conditionalTopics?.length) so the Section component (rendered as
<Section ... />) receives those server-provided sections for SectionContent to
render.
In `@packages/app-builder/src/components/Scenario/Screening/FieldDataset.tsx`:
- Around line 25-28: The ListAndTopicDatasetConfiguration.createSharp call
(assigned to listSharp) is recreated on every render which resets
store/selection; change it to a stable instance by memoizing or creating it once
(e.g., useRef/useMemo) and reuse that same listSharp across renders, then add an
effect that listens to value and calls the store sync/update method with
makeDatasetsMap(value ?? []) so incoming value changes are applied without
remounting the provider/effects.
In
`@packages/app-builder/src/components/Screenings/FreeformSearch/DatasetsPopover.tsx`:
- Around line 45-52: Remove the fallback heuristic that infers leaf dataset ids
by checking k.includes('.') and instead avoid guessing when selectableLeafNames
is not ready: set next to selectableLeafNames when available (do not filter
Object.entries(map) by dot), and make the Apply action/button disabled until
selectableLeafNames is loaded; additionally, if you need to preserve user state
while loading, keep the current selection (e.g. currentSelection or selectedIds)
rather than replacing it with a guessed list. Target symbols: next,
selectableLeafNames, map and the Apply button handler in DatasetsPopover.
In `@packages/app-builder/src/queries/screening/lists-config.ts`:
- Around line 67-76: The query is caching different useCase values under the
same key, causing collisions; update useListConfigQuery to include the useCase
in the queryKey (e.g., change queryKey from ['screening','datasets'] to
['screening','datasets', useCase]) so getListConfig/getListConfigFn results are
cached per feature; ensure the useCase passed (AvailableFeatures) is
serializable/stable when constructing the key and leave the rest of the queryFn
(calling getListConfig and normalizeListConfig) unchanged.
In `@packages/marble-api/openapis/marblecore-api/screenings.yml`:
- Around line 403-450: Add a new property named conditional_filters to the
ScreeningAvailableFilters schema so generated types include
config.conditional_filters; specifically, in ScreeningAvailableFilters add
"conditional_filters" with type: object and model it like the existing "topics"
shape in ScreeningAvailableFiltersSection (i.e., additionalProperties: type:
array with items: type: object, required: [name, title], and properties
name:string and title:string) so the contract matches the frontend shape.
In `@packages/ui-design-system/src/Collapsible/Collapsible.tsx`:
- Around line 52-58: The inner interactive element is a nested <button> inside
the Trigger (used with asChild), causing invalid HTML and accessibility issues;
replace the inner <button> wrapper around Icon in Collapsible (see the Trigger
usage and Icon with icon="smallarrow-up", aria-hidden and className) with a
non-interactive element such as a <span> (keeping the Icon props and className
intact) so the Trigger provides the button semantics and no nested button
exists.
---
Outside diff comments:
In
`@packages/app-builder/src/components/ContinuousScreening/validation/DatasetSelectionSection.tsx`:
- Around line 37-43: In the with({ isSuccess: true }, ({ data: { datasets } })
=> { ... }) block, don't return null when datasets?.sections is missing because
that hides the whole section; instead set datasetsArray to an empty array and
continue rendering so keys and metadata placeholders remain visible. Modify the
logic around datasetsArray (the R.pipe that flatMaps datasets.sections and maps
to {name, tag, title}) to handle missing or undefined datasets.sections by using
a fallback (e.g., []) before flatMap, or conditionally assign datasetsArray = []
when !datasets?.sections, and let the rest of the component render normally.
---
Nitpick comments:
In `@packages/app-builder/src/components/CopyToClipboardButton.tsx`:
- Line 14: The chip variant duplicates utility classes already defined in the
base variant; update the chip entry in the style config to remove redundant
classes (remove cursor-pointer, border, flex, items-center from the 'chip'
variant) leaving only chip-specific classes like gap-v2-xs, font-semibold,
text-small, bg-surface-card and any unique padding classes so the base config
provides shared utilities and 'chip' only supplies its unique overrides.
In
`@packages/app-builder/src/components/ListAndTopicConfiguration/dataset-selection-provider-utils.ts`:
- Around line 7-10: The params type for useListAndTopicDatasetConfigurationSharp
duplicates the 'mode' union; import and use the shared
ListAndTopicDatasetConfigurationMode type instead of repeating the union. Update
the function signature to import ListAndTopicDatasetConfigurationMode and
replace mode: 'view' | 'edit' | 'create' with mode:
ListAndTopicDatasetConfigurationMode so the utility stays in sync with the
exported mode definition.
In `@packages/app-builder/src/constants/ftm-entities.ts`:
- Line 1: The import of SemanticSubTypeField and SemanticTypeField uses a root
`@app-builder/models` barrel which violates the repo alias rule; update the
import to the concrete subpath that directly exports these symbols (replace
`import { SemanticSubTypeField, SemanticTypeField } from '@app-builder/models';`
with an import from the specific `@app-builder/models/...` entrypoint that
exposes SemanticSubTypeField and SemanticTypeField) so the model boundary is
explicit.
- Around line 6-36: The const-asserted FTM_ENTITIES_PROPERTIES uses readonly
arrays but the satisfies target is Record<FtmEntity, string[]>; update the
satisfies clause to reflect readonly arrays by changing the target type to
Record<FtmEntity, readonly string[]> so the declared type matches the `as const`
structure (locate the FTM_ENTITIES_PROPERTIES declaration and the FtmEntity type
reference to make this change).
In `@packages/app-builder/src/repositories/ScreeningRepository.ts`:
- Around line 21-56: The ScreeningRepository currently imports AvailableFeatures
from `@app-builder/server-fns/screenings` which couples the repo to the RPC layer;
extract/define the AvailableFeatures type into a shared models module (e.g.,
models/shared-types or models/features) and update ScreeningRepository to import
AvailableFeatures from that new module instead of server-fns; then update the
RPC/service code (the file exporting `@app-builder/server-fns/screenings`) to
import and re-export or directly use the new shared type so both sides reference
the same model type; finally run typechecks and adjust any import paths
referencing AvailableFeatures across the codebase (including getAvailableFilters
signature) to the new shared module.
In `@packages/app-builder/src/server-fns/screenings.ts`:
- Around line 114-119: The exported const is misspelled as avalableFeatures;
rename it to availableFeatures and update its export and all usages (including
the z.enum call in getAvailableFiltersSchema) to use the new identifier so
imports don't break; ensure the const assertion and satisfies clause remain
intact (export const availableFeatures = [ ... ] as const satisfies
Array<AvailableFeatures>) and update any external import sites to reference
availableFeatures instead of avalableFeatures.
🪄 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: b8fa038b-e179-426a-b8ef-7d6226c0ad3a
⛔ Files ignored due to path filters (1)
packages/marble-api/src/generated/marblecore-api.tsis excluded by!**/generated/**
📒 Files selected for processing (39)
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/constants/ftm-entities.tspackages/app-builder/src/locales/ar/continuous-screening.jsonpackages/app-builder/src/locales/en/continuous-screening.jsonpackages/app-builder/src/locales/fr/continuous-screening.jsonpackages/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/screenings.ymlpackages/ui-design-system/src/Checkbox/Checkbox.tsxpackages/ui-design-system/src/Collapsible/Collapsible.tsxpackages/ui-design-system/src/Tabs/Tabs.stories.tsx
💤 Files with no reviewable changes (1)
- packages/app-builder/src/components/ContinuousScreening/shared/Capsule.tsx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{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/ContinuousScreening/CopyToClipboardChip.tsxpackages/ui-design-system/src/Tabs/Tabs.stories.tsxpackages/app-builder/src/components/ContinuousScreening/shared/RecapRow.tsxpackages/ui-design-system/src/Collapsible/Collapsible.tsxpackages/app-builder/src/components/ListAndTopicConfiguration/context/ListAndTopicDatasetConfiguration.tspackages/app-builder/src/components/ListAndTopicConfiguration/dataset-utils.tspackages/app-builder/src/components/ContinuousScreening/form/Content.tsxpackages/app-builder/src/components/ContinuousScreening/CreationPage.tsxpackages/app-builder/src/components/ContinuousScreening/context/CreationStepper.tsxpackages/app-builder/src/components/CopyToClipboardButton.tsxpackages/app-builder/src/queries/screening/lists-config.tspackages/app-builder/src/components/ListAndTopicConfiguration/index.tspackages/app-builder/src/components/ContinuousScreening/ConfigurationPanel.tsxpackages/app-builder/src/components/ContinuousScreening/context/ListAndTopicDatasetConfigurationBridge.tsxpackages/ui-design-system/src/Checkbox/Checkbox.tsxpackages/app-builder/src/components/ContinuousScreening/validation/DatasetSelectionSection.tsxpackages/app-builder/src/repositories/ScreeningRepository.tspackages/app-builder/src/components/ContinuousScreening/ConfigurationsPage.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/recaps/DatasetSelectionRecap.tsxpackages/app-builder/src/components/ContinuousScreening/form/steps/ObjectMapping.tsxpackages/app-builder/src/components/ListAndTopicConfiguration/dataset-selection-provider-utils.tspackages/app-builder/src/components/ListAndTopicConfiguration/DatasetSelectionContent.tsxpackages/app-builder/src/queries/screening/datasets.tspackages/app-builder/src/constants/ftm-entities.tspackages/app-builder/src/server-fns/screenings.tspackages/app-builder/src/components/Scenario/Screening/FieldDataset.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/DatasetsPopover.tsxpackages/app-builder/src/components/ContinuousScreening/form/steps/DatasetSelection.tsxpackages/app-builder/src/models/screening.ts
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/ContinuousScreening/CopyToClipboardChip.tsxpackages/app-builder/src/components/ContinuousScreening/shared/RecapRow.tsxpackages/app-builder/src/components/ContinuousScreening/form/Content.tsxpackages/app-builder/src/components/ContinuousScreening/CreationPage.tsxpackages/app-builder/src/components/ContinuousScreening/context/CreationStepper.tsxpackages/app-builder/src/components/CopyToClipboardButton.tsxpackages/app-builder/src/components/ContinuousScreening/ConfigurationPanel.tsxpackages/app-builder/src/components/ContinuousScreening/context/ListAndTopicDatasetConfigurationBridge.tsxpackages/app-builder/src/components/ContinuousScreening/validation/DatasetSelectionSection.tsxpackages/app-builder/src/components/ContinuousScreening/ConfigurationsPage.tsxpackages/app-builder/src/components/ContinuousScreening/form/steps/GeneralInfo.tsxpackages/app-builder/src/components/ContinuousScreening/form/recaps/DatasetSelectionRecap.tsxpackages/app-builder/src/components/ContinuousScreening/form/steps/ObjectMapping.tsxpackages/app-builder/src/components/ListAndTopicConfiguration/DatasetSelectionContent.tsxpackages/app-builder/src/components/Scenario/Screening/FieldDataset.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/DatasetsPopover.tsxpackages/app-builder/src/components/ContinuousScreening/form/steps/DatasetSelection.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/ContinuousScreening/CopyToClipboardChip.tsxpackages/ui-design-system/src/Tabs/Tabs.stories.tsxpackages/app-builder/src/components/ContinuousScreening/shared/RecapRow.tsxpackages/ui-design-system/src/Collapsible/Collapsible.tsxpackages/app-builder/src/components/ListAndTopicConfiguration/context/ListAndTopicDatasetConfiguration.tspackages/app-builder/src/locales/ar/continuous-screening.jsonpackages/app-builder/src/components/ListAndTopicConfiguration/dataset-utils.tspackages/app-builder/src/components/ContinuousScreening/form/Content.tsxpackages/app-builder/src/components/ContinuousScreening/CreationPage.tsxpackages/app-builder/src/components/ContinuousScreening/context/CreationStepper.tsxpackages/app-builder/src/components/CopyToClipboardButton.tsxpackages/app-builder/src/queries/screening/lists-config.tspackages/app-builder/src/components/ListAndTopicConfiguration/index.tspackages/app-builder/src/components/ContinuousScreening/ConfigurationPanel.tsxpackages/app-builder/src/components/ContinuousScreening/context/ListAndTopicDatasetConfigurationBridge.tsxpackages/ui-design-system/src/Checkbox/Checkbox.tsxpackages/app-builder/src/components/ContinuousScreening/validation/DatasetSelectionSection.tsxpackages/app-builder/src/repositories/ScreeningRepository.tspackages/app-builder/src/locales/fr/continuous-screening.jsonpackages/app-builder/src/components/ContinuousScreening/ConfigurationsPage.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/recaps/DatasetSelectionRecap.tsxpackages/app-builder/src/components/ContinuousScreening/form/steps/ObjectMapping.tsxpackages/app-builder/src/locales/en/continuous-screening.jsonpackages/app-builder/src/components/ListAndTopicConfiguration/dataset-selection-provider-utils.tspackages/app-builder/src/components/ListAndTopicConfiguration/DatasetSelectionContent.tsxpackages/app-builder/src/queries/screening/datasets.tspackages/app-builder/src/constants/ftm-entities.tspackages/app-builder/src/server-fns/screenings.tspackages/app-builder/src/components/Scenario/Screening/FieldDataset.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/DatasetsPopover.tsxpackages/app-builder/src/components/ContinuousScreening/form/steps/DatasetSelection.tsxpackages/app-builder/src/models/screening.ts
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/ContinuousScreening/CopyToClipboardChip.tsxpackages/app-builder/src/components/ContinuousScreening/shared/RecapRow.tsxpackages/app-builder/src/components/ListAndTopicConfiguration/context/ListAndTopicDatasetConfiguration.tspackages/app-builder/src/components/ListAndTopicConfiguration/dataset-utils.tspackages/app-builder/src/components/ContinuousScreening/form/Content.tsxpackages/app-builder/src/components/ContinuousScreening/CreationPage.tsxpackages/app-builder/src/components/ContinuousScreening/context/CreationStepper.tsxpackages/app-builder/src/components/CopyToClipboardButton.tsxpackages/app-builder/src/queries/screening/lists-config.tspackages/app-builder/src/components/ListAndTopicConfiguration/index.tspackages/app-builder/src/components/ContinuousScreening/ConfigurationPanel.tsxpackages/app-builder/src/components/ContinuousScreening/context/ListAndTopicDatasetConfigurationBridge.tsxpackages/app-builder/src/components/ContinuousScreening/validation/DatasetSelectionSection.tsxpackages/app-builder/src/repositories/ScreeningRepository.tspackages/app-builder/src/components/ContinuousScreening/ConfigurationsPage.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/recaps/DatasetSelectionRecap.tsxpackages/app-builder/src/components/ContinuousScreening/form/steps/ObjectMapping.tsxpackages/app-builder/src/components/ListAndTopicConfiguration/dataset-selection-provider-utils.tspackages/app-builder/src/components/ListAndTopicConfiguration/DatasetSelectionContent.tsxpackages/app-builder/src/queries/screening/datasets.tspackages/app-builder/src/constants/ftm-entities.tspackages/app-builder/src/server-fns/screenings.tspackages/app-builder/src/components/Scenario/Screening/FieldDataset.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/DatasetsPopover.tsxpackages/app-builder/src/components/ContinuousScreening/form/steps/DatasetSelection.tsxpackages/app-builder/src/models/screening.ts
packages/app-builder/src/queries/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Create one TanStack Query hook file per query in the queries directory
Files:
packages/app-builder/src/queries/screening/lists-config.tspackages/app-builder/src/queries/screening/datasets.ts
packages/app-builder/src/repositories/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Place data access logic in repositories directory following the Repository pattern
Files:
packages/app-builder/src/repositories/ScreeningRepository.ts
packages/app-builder/src/routes/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
packages/app-builder/src/routes/**/*.tsx: Use flat routes with+folders for organizing Remix routes (e.g.,_builder+/cases+/$caseId.tsx)
Usehandleproperty in routes for breadcrumbs
Use loaders for data fetching in Remix routes
Files:
packages/app-builder/src/routes/_app/_builder/detection/scenarios/$scenarioId/i/$iterationId/screenings.$screeningId.tsx
packages/app-builder/src/models/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use type definitions with adapters in the models directory
Files:
packages/app-builder/src/models/screening.ts
🔇 Additional comments (22)
packages/ui-design-system/src/Checkbox/Checkbox.tsx (1)
47-47: Nice, targeted event-bubbling control.This is a clean addition:
stopPropagationis opt-in, and forwardingonClickpreserves existing consumer behavior.
A neat touch—“small hinge, great gate.”Also applies to: 50-63
packages/ui-design-system/src/Tabs/Tabs.stories.tsx (1)
3-3: No action required—remedais already a declared dependency.The
capitalizeimport fromremeda(v^2.21.2) is already part of theui-design-systempackage dependencies. Using the library's utility function here is perfectly appropriate and cleaner than inline capitalization logic, even in a story file. The change is sound.packages/app-builder/src/components/ContinuousScreening/shared/RecapRow.tsx (1)
1-16: LGTM! Clean migration to Tag.The refactor from
CapsuletoTagis well executed, and the import follows the coding guidelines for ui-design-system components.packages/app-builder/src/components/ContinuousScreening/CopyToClipboardChip.tsx (1)
4-6: LGTM! Clear deprecation notice.The deprecation comment provides a clear migration path to
CopyToClipboardButtonwith specific props.packages/app-builder/src/components/CopyToClipboardButton.tsx (1)
52-52: LGTM! Icon sizing logic handles the chip variant correctly.The conditional sizing appropriately includes the new
chipvariant.packages/app-builder/src/components/ContinuousScreening/form/steps/GeneralInfo.tsx (1)
1-34: LGTM! Migration executed well.Hark! The clipboard button now doth shine,
With chip-like size and rounded design.
The stable ID displays with grace,
In this refactored, cleaner space.packages/app-builder/src/components/ContinuousScreening/ConfigurationsPage.tsx (2)
11-29: LGTM! Clean component extraction and imports.The
CellCapsulehelper component properly encapsulates theTagstyling, and all imports follow the coding guidelines.
131-133: LGTM! Consistent migration pattern.The stable ID clipboard button follows the same pattern as in
GeneralInfo.tsx, with appropriate sizing and styling.packages/app-builder/src/queries/screening/datasets.ts (1)
10-13: Looks good — returning the raw server result is the right move.This keeps the hook aligned with the updated API contract and avoids unnecessary local shaping. All’s well that fetches well.
packages/app-builder/src/components/ContinuousScreening/context/CreationStepper.tsx (1)
73-80: ⚡ Quick winThe step reordering is properly coordinated and safe.
All hardcoded step indices in
ConfigurationPanel.tsxandContent.tsxhave been updated to match the new order inCreationStepper.tsx. The mapping is consistent:
- Index 2 → DatasetSelection
- Index 3 → ScoringConfiguration
No persistence logic references step indices, and recap conditions are correctly aligned. The change is sound.
packages/app-builder/src/components/ListAndTopicConfiguration/dataset-utils.ts (1)
5-14: Nice helper extraction.The leaf-name aggregation is null-safe and deduplicated, and the intent around
sectionKeyis clearly documented.packages/app-builder/src/components/ListAndTopicConfiguration/context/ListAndTopicDatasetConfiguration.ts (1)
10-20: Factory and action shape look good.State initialization and
setModeare straightforward and aligned with the intended context bridge usage.packages/app-builder/src/locales/en/continuous-screening.json (1)
11-13: LGTM! Well-structured i18n additions.The new translation keys follow the existing naming conventions and properly support pluralization for section items. The filter and suggestion labels are clear and consistent with the UI patterns.
Also applies to: 20-21, 33-33
packages/marble-api/openapis/marblecore-api.yaml (1)
226-227: Proper path delegation.The new endpoint follows the established pattern for screening paths, with clean delegation to the sub-specification file.
packages/app-builder/src/components/ContinuousScreening/CreationPage.tsx (1)
10-10: Context bridge properly integrated.The
ListAndTopicDatasetConfigurationBridgewrapper is correctly positioned to provide context for the child components, maintaining the existing page structure while enabling the new dataset configuration architecture.Also applies to: 46-58
packages/app-builder/src/components/ContinuousScreening/ConfigurationPanel.tsx (1)
57-68: Step reordering successfully applied.The dataset selection and scoring configuration steps have been swapped (indices 2 and 3), and the bridge wrapper provides the necessary context. The
useCase="continuous_monitoring"prop properly configures the dataset selection for this flow.packages/marble-api/openapis/marblecore-api/_schemas.yml (1)
284-287: Schema references properly exported.The new
ScreeningAvailableFiltersandScreeningAvailableFiltersSectionschema references follow the established pattern and align with the new endpoint definition.packages/app-builder/src/components/ContinuousScreening/form/Content.tsx (1)
22-23: Step routing and recap synchronized.The step indices and recap rendering threshold are properly aligned with the reordered configuration flow. Dataset selection at step 2, scoring at step 3—a harmonious progression!
Also applies to: 45-46
packages/app-builder/src/components/ContinuousScreening/form/steps/ObjectMapping.tsx (2)
239-247: UI layout adjusted for suggestion button.The field mapping header now accommodates the suggest button, though the layout shifts from
justify-betweento a simpler flex arrangement. The button is properly hidden in view mode.
202-221: ⚡ Quick winType safety is ensured by the discriminated union type, but confirm intent for multiple field matches.
The
FtmEntityPropertyKeytype is well-designed as a discriminated union (${Entity}.${ValidProperty}) that prevents invalid entity-property combinations at compile time. The key construction is safe.However, when multiple fields share the same
semanticType, the first match fromArray.find()is used silently. If this could be ambiguous (e.g., two name fields with the same semantic type), consider whether you should handle this explicitly—perhaps preferring unmapped fields, adding a comment explaining the intent, or notifying the user when ambiguity exists."All's well that type-checks well." 🎭
packages/app-builder/src/routes/_app/_builder/detection/scenarios/$scenarioId/i/$iterationId/screenings.$screeningId.tsx (2)
66-84: Loader data simplified appropriately.The removal of
sectionsfrom the loader aligns with the refactoring to centralize dataset configuration. The simplifiedFieldDatasetAPI—accepting onlyvalueandonChangeprops—matches the component's actual signature and is correctly utilized in the form field implementation.
878-878: TheFieldDatasetcomponent accepts onlyvalueandonChangeprops—both optional. The props mentioned in the concern (defaultValue,onBlur,sections) don't exist in the component's interface. The current usage is correct and matches the component's actual signature.> Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 43-49: handleApply currently computes next =
selectableLeafNames?.filter(...) ?? [] and calls onApply([]) when list config
hasn't loaded, wiping selection; update handleApply to either (a) early-return
or disable the Apply action until listConfigQuery.data is ready (check
listConfigQuery.isLoading/isSuccess) and prevent calling onApply/setOpen, or (b)
preserve the previous selection by using selectedDatasets (or another existing
selected state) when selectableLeafNames is undefined instead of falling back to
an empty array; modify the Apply button to be disabled when
listConfigQuery.isLoading if you choose the first option and ensure references
to selectableLeafNames, listConfigQuery, handleApply, onApply, setOpen, and
selectedDatasets are updated accordingly.
In `@packages/ui-design-system/src/Collapsible/Collapsible.tsx`:
- Around line 48-59: The Trigger is rendering a <div> asChild which breaks
semantics/keyboard behavior and mismatches the component's HTMLButtonElement
typing; replace the inner <div> with a native <button type="button"> (still
using asChild on Trigger) to restore button semantics and ensure the forwarded
ref matches HTMLButtonElement, keep collapsibleTitle({ size, className }) on
Trigger, and ensure props are spread onto Trigger; also change the focus utility
from focus-within to focus-visible on the element wrapping children so the focus
ring appears only during keyboard navigation while preserving the existing Icon
and hideIcon logic.
🪄 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: c5dab262-3dda-42fb-bbd0-88322339bbc8
📒 Files selected for processing (7)
packages/app-builder/src/components/ListAndTopicConfiguration/DatasetSelectionContent.tsxpackages/app-builder/src/components/Screenings/FreeformSearch/DatasetsPopover.tsxpackages/app-builder/src/locales/ar/continuous-screening.jsonpackages/app-builder/src/locales/en/continuous-screening.jsonpackages/app-builder/src/locales/fr/continuous-screening.jsonpackages/app-builder/src/queries/screening/lists-config.tspackages/ui-design-system/src/Collapsible/Collapsible.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/app-builder/src/queries/screening/lists-config.ts
- packages/app-builder/src/locales/en/continuous-screening.json
- packages/app-builder/src/locales/fr/continuous-screening.json
- packages/app-builder/src/components/ListAndTopicConfiguration/DatasetSelectionContent.tsx
📜 Review details
🧰 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/DatasetsPopover.tsxpackages/ui-design-system/src/Collapsible/Collapsible.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/DatasetsPopover.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/DatasetsPopover.tsxpackages/ui-design-system/src/Collapsible/Collapsible.tsxpackages/app-builder/src/locales/ar/continuous-screening.json
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/DatasetsPopover.tsx
🧠 Learnings (1)
📚 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/DatasetsPopover.tsx
🔇 Additional comments (5)
packages/app-builder/src/locales/ar/continuous-screening.json (3)
11-14: Looks good: filter action keys are consistent and clear.These new Arabic labels align well with the surrounding dataset-selection terminology and key naming pattern.
21-22: Recap plural strings are well-formed.Placeholders and plural key structure are consistent with the rest of this locale file.
34-34: Nice addition for the suggest action label.The key and Arabic phrasing fit the object-mapping flow cleanly—brief, precise, and user-facing ready.
A fair line, well set; the UI shall speak with grace.packages/ui-design-system/src/Collapsible/Collapsible.tsx (1)
35-35: ⚡ Quick winPrefer an explicit variant token over
"null"for size.Using
"null"as a variant key is easy to confuse with actualnull(which CVA also treats specially). A named token like"none"keeps the API clearer and less error-prone for callers.Suggested refactor
const collapsibleTitle = cva('group flex cursor-pointer items-center justify-between gap-4 font-semibold', { variants: { size: { default: 'p-4 lg:p-6', small: 'p-4', - null: 'p-0', + none: 'p-0', }, }, defaultVariants: { size: 'default', }, });[ suggest_recommended_refactor ]
packages/app-builder/src/components/Screenings/FreeformSearch/DatasetsPopover.tsx (1)
26-35: ⚡ Quick winThe state reset mechanism works correctly—no bug here. The
listSharphook is called unconditionally on every render (line 27, no memoization). WhenhandleOpenChangeorhandleCancelcallssetDatasetsMap(), React re-renders and the hook re-executes with the updateddatasetsMapparameter, creating a fresh Sharp instance. The previous Sharp (with any user edits) is discarded. This is the same pattern used throughout the codebase (ContinuousScreening, Scenario, etc.) and functions as intended.> Likely an incorrect or invalid review comment.
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/app-builder/src/server-fns/screenings.ts (1)
117-123: Server function implementation is functional.The endpoint is properly secured with auth middleware, validates input with the schema, and delegates to the repository layer. The implementation follows the established patterns in the file.
Soft suggestion: Consider adding an explicit return type annotation for better API documentation and type safety, though TypeScript inference works here.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/app-builder/src/server-fns/screenings.ts` around lines 117 - 123, Add an explicit return type to the getListConfigFn server function to improve API documentation and type safety: annotate the handler (or the exported getListConfigFn) with the appropriate return type that matches the shape returned by context.authInfo.screening.getAvailableFilters (use the repository DTO/type used by getAvailableFilters). Keep existing createServerFn({ method: 'GET' }), authMiddleware, and getAvailableFiltersSchema usage unchanged and ensure the declared type imports/references the same type used by context.authInfo.screening.getAvailableFilters.
🤖 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.
Nitpick comments:
In `@packages/app-builder/src/server-fns/screenings.ts`:
- Around line 117-123: Add an explicit return type to the getListConfigFn server
function to improve API documentation and type safety: annotate the handler (or
the exported getListConfigFn) with the appropriate return type that matches the
shape returned by context.authInfo.screening.getAvailableFilters (use the
repository DTO/type used by getAvailableFilters). Keep existing createServerFn({
method: 'GET' }), authMiddleware, and getAvailableFiltersSchema usage unchanged
and ensure the declared type imports/references the same type used by
context.authInfo.screening.getAvailableFilters.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e1fa6df6-eeed-4509-88ea-e41dd19e99d9
📒 Files selected for processing (11)
packages/app-builder/src/components/ContinuousScreening/form/steps/DatasetSelection.tsxpackages/app-builder/src/components/ContinuousScreening/form/steps/ObjectMapping.tsxpackages/app-builder/src/components/CopyToClipboardButton.tsxpackages/app-builder/src/components/ListAndTopicConfiguration/DatasetSelectionContent.tsxpackages/app-builder/src/components/ListAndTopicConfiguration/dataset-selection-provider-utils.tspackages/app-builder/src/components/Screenings/FreeformSearch/DatasetsPopover.tsxpackages/app-builder/src/constants/ftm-entities.tspackages/app-builder/src/models/screening.tspackages/app-builder/src/queries/screening/lists-config.tspackages/app-builder/src/repositories/ScreeningRepository.tspackages/app-builder/src/server-fns/screenings.ts
✅ Files skipped from review due to trivial changes (1)
- packages/app-builder/src/components/ListAndTopicConfiguration/dataset-selection-provider-utils.ts
🚧 Files skipped from review as they are similar to previous changes (9)
- packages/app-builder/src/components/CopyToClipboardButton.tsx
- packages/app-builder/src/repositories/ScreeningRepository.ts
- packages/app-builder/src/models/screening.ts
- packages/app-builder/src/constants/ftm-entities.ts
- packages/app-builder/src/components/ContinuousScreening/form/steps/ObjectMapping.tsx
- packages/app-builder/src/queries/screening/lists-config.ts
- packages/app-builder/src/components/ContinuousScreening/form/steps/DatasetSelection.tsx
- packages/app-builder/src/components/Screenings/FreeformSearch/DatasetsPopover.tsx
- packages/app-builder/src/components/ListAndTopicConfiguration/DatasetSelectionContent.tsx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{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/server-fns/screenings.ts
**/*.{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/server-fns/screenings.ts
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/server-fns/screenings.ts
🔇 Additional comments (4)
packages/app-builder/src/server-fns/screenings.ts (4)
3-3: Import addition looks good.The
availableFeaturesimport follows the path alias guidelines and will support the new schema definition below.
10-11: Well-organized section markers.The addition of section comments improves code navigation. Like signposts in a well-tended garden, they guide the reader's eye.
115-116: Consistent section organization.
112-113: ⚡ Quick winNo action needed—
availableFeaturesis already properly typed forz.enum().The definition in
packages/app-builder/src/models/screening.tscorrectly exportsavailableFeatureswith aas constassertion, which creates the readonly tuple thatz.enum()requires. The additionalsatisfies Array<AvailableFeatures>clause provides extra type safety. 'Tis already well-composed, this code—no correction doth await.> Likely an incorrect or invalid review comment.
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.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/app-builder/src/models/continuous-screening.ts (1)
112-114: ⚡ Quick winConsider renaming for clarity.
The variable
datasetsnow contains selection strings in the formatsection:type:name[:value]rather than simple dataset names. Consider renaming toselectionStringsorenabledSelectionsto better reflect its purpose and avoid confusion.📝 Proposed refactor
- const datasets = Object.entries(configuration.datasets) + const enabledSelections = Object.entries(configuration.datasets) .filter(([_, value]) => value) .map(([key]) => key); return { name: configuration.name, description: configuration.description, object_types: configuration.mappingConfigs.map((mc) => mc.objectType), algorithm: configuration.algorithm, datasets: [], - filters: createContinuousScreeningFilters(datasets), + filters: createContinuousScreeningFilters(enabledSelections), inbox_id: configuration.inboxId,🤖 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 112 - 114, The variable named datasets is misleading because it holds selection strings like "section:type:name[:value]"; rename the declaration const datasets in the continuous-screening module to a clearer name such as selectionStrings or enabledSelections (e.g., const selectionStrings = Object.entries(configuration.datasets).filter(([_, value]) => value).map(([key]) => key)) and update all references/usages in this file to the new identifier (look for usages of datasets in functions, loops, and exports in this module) so the name accurately reflects the value format.
🤖 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/models/continuous-screening.ts`:
- Line 105: Remove the stray debug logging by deleting the
console.log('filters', filters) call in the continuous-screening module; if you
need to preserve this information for diagnostics, replace it with a conditional
debug-level logger (e.g., use the existing app logger with logger.debug or wrap
in an environment check) so production runs have no console.log output and the
variable name 'filters' remains referenced only where needed.
- Around line 84-104: The filters object is only setting enabled flags so pushes
to datasets and topics silently no-op; initialize the collections and guard
before pushing: when creating filters (and inside the loop over selection)
ensure each section in filters has datasets initialized to an array and topics
initialized to an object (e.g., datasets: [] and topics: {}), and before pushing
to filters[section].topics[name] create the array if missing (so assign an empty
array when first seen) and then push; also remove reliance on optional chaining
for these pushes (use explicit existence checks/initialization) in the
selection-processing loop that references filters, filters[section].datasets,
and filters[section].topics.
---
Nitpick comments:
In `@packages/app-builder/src/models/continuous-screening.ts`:
- Around line 112-114: The variable named datasets is misleading because it
holds selection strings like "section:type:name[:value]"; rename the declaration
const datasets in the continuous-screening module to a clearer name such as
selectionStrings or enabledSelections (e.g., const selectionStrings =
Object.entries(configuration.datasets).filter(([_, value]) => value).map(([key])
=> key)) and update all references/usages in this file to the new identifier
(look for usages of datasets in functions, loops, and exports in this module) so
the name accurately reflects the value format.
🪄 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: 2eae4c67-383b-49c0-a006-7ce10e3bae46
⛔ Files ignored due to path filters (1)
packages/marble-api/src/generated/marblecore-api.tsis excluded by!**/generated/**
📒 Files selected for processing (6)
packages/app-builder/src/components/ListAndTopicConfiguration/DatasetSelectionContent.tsxpackages/app-builder/src/models/continuous-screening.tspackages/app-builder/src/models/screening.tspackages/app-builder/src/queries/screening/lists-config.tspackages/marble-api/openapis/marblecore-api/_schemas.ymlpackages/marble-api/openapis/marblecore-api/screenings.yml
✅ Files skipped from review due to trivial changes (1)
- packages/marble-api/openapis/marblecore-api/_schemas.yml
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/marble-api/openapis/marblecore-api/screenings.yml
- packages/app-builder/src/models/screening.ts
- packages/app-builder/src/queries/screening/lists-config.ts
- 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). (1)
- GitHub Check: e2e
🧰 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/models/continuous-screening.ts
packages/app-builder/src/models/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use type definitions with adapters in the models directory
Files:
packages/app-builder/src/models/continuous-screening.ts
**/*.{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/models/continuous-screening.ts
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/models/continuous-screening.ts
🔇 Additional comments (1)
packages/app-builder/src/models/continuous-screening.ts (1)
12-12: LGTM!
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/marble-api/openapis/marblecore-api/continuous-screenings.yml`:
- Around line 362-363: Add the missing filters property to the update and
response DTOs so continuous screenings match regular screenings: add filters:
$ref: 'screenings.yml#/components/schemas/ScreeningConfigBodyFiltersDto' to
UpdateContinuousScreeningConfigDto and ContinuousScreeningConfigDto (the
response DTO) just as CreateContinuousScreeningConfigDto already has it,
ensuring clients can read and update filter configuration and align with
ScreeningConfigDto behavior.
🪄 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: aa41dfc9-7171-49bb-9424-0c30dccd6f0d
📒 Files selected for processing (2)
packages/app-builder/src/models/continuous-screening.tspackages/marble-api/openapis/marblecore-api/continuous-screenings.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/app-builder/src/models/continuous-screening.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: check / main
- GitHub Check: e2e
🔇 Additional comments (1)
packages/marble-api/openapis/marblecore-api/continuous-screenings.yml (1)
362-363: ⚡ Quick winExternal schema reference is valid.
The
filtersproperty correctly referencesScreeningConfigBodyFiltersDtofromscreenings.yml(line 465). The schema is well-formed—an object with filtering sections for sanctions, peps, adverse_media, and other screening types. All is in order. When such references are joined in harmony, the architecture doth sing in proper symphony.
| filters: | ||
| $ref: 'screenings.yml#/components/schemas/ScreeningConfigBodyFiltersDto' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Search for backend code handling filters in continuous screening config
# Search for filter-related logic in continuous screening services/repositories
rg -n -C 5 "filters" --type ts --type go -g '*continuous*screening*' -g '*ContinuousScreening*'
# Check if filters are mentioned in related model definitions
rg -n "filters" packages/app-builder/src/models/continuous-screening.tsRepository: checkmarble/marble-frontend
Length of output: 6716
🏁 Script executed:
# Read the continuous-screenings.yml file to see the full DTO definitions
wc -l packages/marble-api/openapis/marblecore-api/continuous-screenings.ymlRepository: checkmarble/marble-frontend
Length of output: 145
🏁 Script executed:
# Extract the definitions of the three relevant DTOs
sed -n '250,420p' packages/marble-api/openapis/marblecore-api/continuous-screenings.yml | cat -nRepository: checkmarble/marble-frontend
Length of output: 5123
🏁 Script executed:
# Check for any comments in the YAML file that might explain the filters design
grep -n -B 2 -A 2 "filters" packages/marble-api/openapis/marblecore-api/continuous-screenings.ymlRepository: checkmarble/marble-frontend
Length of output: 258
🏁 Script executed:
# Also check the screenings.yml file to understand ScreeningConfigBodyFiltersDto structure
grep -n -A 10 "ScreeningConfigBodyFiltersDto:" packages/marble-api/openapis/marblecore-api/screenings.ymlRepository: checkmarble/marble-frontend
Length of output: 534
🏁 Script executed:
# Search for backend handling of filters in continuous screening
# Check if there's any documentation or backend code that explains the pattern
rg -n "filters" packages/marble-api/openapis/marblecore-api/ -A 3 -B 1 | head -50Repository: checkmarble/marble-frontend
Length of output: 4360
🏁 Script executed:
# Check if filters are mentioned in any backend Go code (if it exists in this repo)
find . -name "*.go" -type f 2>/dev/null | xargs grep -l "ContinuousScreening" 2>/dev/null | head -5Repository: checkmarble/marble-frontend
Length of output: 53
🏁 Script executed:
# Check how filters are handled in the regular (non-continuous) screening config
# to see if there's a consistent pattern
rg -n "ScreeningConfigDto|ScreeningConfigBody" packages/marble-api/openapis/marblecore-api/screenings.yml -A 15 | head -80Repository: checkmarble/marble-frontend
Length of output: 2314
🏁 Script executed:
# Check if there are any update operations for screenings to see the pattern
rg -n "UpdateScreening|update.*screening" packages/marble-api/openapis/marblecore-api/screenings.yml -i -A 10 | head -60Repository: checkmarble/marble-frontend
Length of output: 1118
🏁 Script executed:
# Check if there's a CreateScreeningConfigDto to compare the pattern
rg -n "CreateScreeningConfigDto" packages/marble-api/openapis/marblecore-api/screenings.yml -A 30 | head -50Repository: checkmarble/marble-frontend
Length of output: 53
🏁 Script executed:
# Search for any comments or documentation about why filters might be excluded
grep -r "immutable\|read.only\|create.only" packages/marble-api/openapis/ 2>/dev/null | head -20Repository: checkmarble/marble-frontend
Length of output: 53
🏁 Script executed:
# Check if there are any TypeScript models or backend code that handle continuous screening responses
# to see if filters are stored/handled but just omitted from the DTO
rg -n "ContinuousScreeningConfigDto" packages/app-builder/ -A 10 | head -40Repository: checkmarble/marble-frontend
Length of output: 4285
🏁 Script executed:
# Compare with how regular screenings define their response DTOs
rg -n "ScreeningConfigDto:" packages/marble-api/openapis/marblecore-api/screenings.yml -A 35Repository: checkmarble/marble-frontend
Length of output: 1240
Add filters to response and update DTOs for consistency with regular screenings.
The filters property is present in CreateContinuousScreeningConfigDto but missing from both UpdateContinuousScreeningConfigDto and ContinuousScreeningConfigDto. This contradicts the pattern used in regular screenings, where ScreeningConfigDto (the response DTO) includes filters.
Additionally, the frontend code reuses adaptCreateContinuousScreeningConfigDto for both create and update operations, suggesting filters should be updatable. Without filters in the response DTO, clients cannot verify the configuration they've set.
Add filters to both the update and response DTOs to:
- Match the regular screening pattern
- Enable clients to read back their filter configuration
- Support filter updates if the backend allows them
🤖 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, Add the missing filters property to the update and response
DTOs so continuous screenings match regular screenings: add filters: $ref:
'screenings.yml#/components/schemas/ScreeningConfigBodyFiltersDto' to
UpdateContinuousScreeningConfigDto and ContinuousScreeningConfigDto (the
response DTO) just as CreateContinuousScreeningConfigDto already has it,
ensuring clients can read and update filter configuration and align with
ScreeningConfigDto behavior.
2 participants
What has been done
Summary by CodeRabbit
New Features
Improvements
Deprecated
Localization