Skip to content

use utility functions in continuous screening and manual search#1554

Merged
william-schlegel merged 1 commit into
mainfrom
fix/continuous-screening-config-reading
May 20, 2026
Merged

use utility functions in continuous screening and manual search#1554
william-schlegel merged 1 commit into
mainfrom
fix/continuous-screening-config-reading

Conversation

@william-schlegel
Copy link
Copy Markdown
Contributor

@william-schlegel william-schlegel commented May 20, 2026

fix dataset and topic keys management in continuous screening config and manual search using existing utility functions

Summary by CodeRabbit

  • Refactor
    • Internal code improvements to dataset configuration synchronization and selection handling in the continuous screening module.
    • Updated how configuration data flows between components for better consistency and maintainability.

Review Change Stack

@coderabbitai coderabbitai Bot added the M medium label May 20, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 20, 2026

📝 Walkthrough

Walkthrough

This PR refactors dataset selection and mapping across continuous-screening configuration components and adapters. It replaces ad-hoc dataset derivation with canonical helper functions (makeDatasetsMap, getCanonicalSelectedKeys, getDatasetFromFilters) and introduces stable change-detection utilities to synchronize dataset state across the UI layer.

Changes

Dataset selection canonicalization and synchronization

Layer / File(s) Summary
Helper imports and canonical key utilities
packages/app-builder/src/models/continuous-screening.ts, packages/app-builder/src/components/ContinuousScreening/ConfigurationsPage.tsx, packages/app-builder/src/components/ContinuousScreening/context/ListAndTopicDatasetConfigurationBridge.tsx
Adds imports for canonical dataset helpers and introduces getDatasetsMapKey helper for stable dataset-map key generation in synchronization logic.
ConfigurationsPage dataset-map derivation
packages/app-builder/src/components/ContinuousScreening/ConfigurationsPage.tsx
Replaces inline Object.fromEntries(baseConfig.datasets.map(...)) construction with makeDatasetsMap(baseConfig.datasets) in handleRowClick.
ListAndTopicDatasetConfigurationBridge change detection
packages/app-builder/src/components/ContinuousScreening/context/ListAndTopicDatasetConfigurationBridge.tsx
Adds useEffect that derives the current datasets-map key, compares it to the memoized key, and synchronizes listSharp.value.datasets state when datasets change.
DatasetsPopover canonical selection and counting
packages/app-builder/src/components/Screenings/FreeformSearch/DatasetsPopover.tsx
Refactors selection application to use getCanonicalSelectedKeys(listSharp.value.datasets) and introduces selectionMap memo for consistent section-tag count computation.
Adapter layer dataset canonicalization
packages/app-builder/src/models/continuous-screening.ts
Updates adaptContinuousScreeningConfig to derive datasets from filters, and replaces manual filtering in adaptCreateContinuousScreeningConfigDto with getCanonicalSelectedKeys(configuration.datasets).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • checkmarble/marble-frontend#1553: Main PR that relies on the shared makeDatasetsMap and getCanonicalSelectedKeys helpers introduced in this PR to fix dataset key canonicalization.
  • checkmarble/marble-frontend#1535: Prior PR that modified DatasetsPopover.tsx to rebuild selection and tag-count logic; this PR refactors those changes to use canonical helpers.

Suggested labels

M

Suggested reviewers

  • apognu
  • Djauron
  • OrriMandarin

Poem

Datasets now canonicalize with grace,
No more inline maps clutter the place.
Helpers shared keep selections in tune,
Change detection swift as a summer tune—
Keys memoized, state in rhythm true! 📊✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the core change: refactoring to use utility functions (makeDatasetsMap, getCanonicalSelectedKeys, getDatasetsMapKey, getDatasetFromFilters) across continuous screening and manual search components.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/continuous-screening-config-reading

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

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
packages/app-builder/src/components/ContinuousScreening/ConfigurationsPage.tsx (1)

15-15: ⚡ Quick win

Use @app-builder alias for this utility import.

Please switch this new utility import to the internal namespace alias to match project conventions.

Proposed change
-import { makeDatasetsMap } from '../ListAndTopicConfiguration/dataset-selection-provider-utils';
+import { makeDatasetsMap } from '`@app-builder/components/ListAndTopicConfiguration/dataset-selection-provider-utils`';

As per coding guidelines: "Use internal imports from @app-builder namespace for models, queries, components, and utilities".

🤖 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/ConfigurationsPage.tsx`
at line 15, The import path for the utility should use the internal namespace
alias; in ConfigurationsPage.tsx replace the relative import of makeDatasetsMap
with the `@app-builder` alias so the module is imported from
"`@app-builder/`.../dataset-selection-provider-utils" (keeping the symbol name
makeDatasetsMap unchanged) to follow project conventions for internal utilities.
🤖 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 12-14: getDatasetsMapKey only joins selected keys so changes to
false/unselected entries are ignored; update getDatasetsMapKey to produce a
stable representation of the entire datasets map (e.g., sort all keys and join
"key=true" or "key=false" for every key) so any change (true→false, false→true,
order changes, or new/removed keys) changes the key and forces sync; apply the
same full-map canonicalization wherever datasetsMapKey is computed/used
(referencing getDatasetsMapKey and listSharp.value.datasets) so the bridge never
skips updates when unselected keys change.

---

Nitpick comments:
In
`@packages/app-builder/src/components/ContinuousScreening/ConfigurationsPage.tsx`:
- Line 15: The import path for the utility should use the internal namespace
alias; in ConfigurationsPage.tsx replace the relative import of makeDatasetsMap
with the `@app-builder` alias so the module is imported from
"`@app-builder/`.../dataset-selection-provider-utils" (keeping the symbol name
makeDatasetsMap unchanged) to follow project conventions for internal utilities.
🪄 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: 69f9fa6e-7575-487e-893f-076a52b2f3aa

📥 Commits

Reviewing files that changed from the base of the PR and between 25e05ac and af1a97d.

📒 Files selected for processing (4)
  • packages/app-builder/src/components/ContinuousScreening/ConfigurationsPage.tsx
  • packages/app-builder/src/components/ContinuousScreening/context/ListAndTopicDatasetConfigurationBridge.tsx
  • packages/app-builder/src/components/Screenings/FreeformSearch/DatasetsPopover.tsx
  • 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: e2e
  • GitHub Check: check / main
🧰 Additional context used
📓 Path-based instructions (2)
packages/app-builder/src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

packages/app-builder/src/**/*.{ts,tsx}: Use internal imports from @app-builder namespace 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/ListAndTopicDatasetConfigurationBridge.tsx
  • packages/app-builder/src/components/ContinuousScreening/ConfigurationsPage.tsx
  • packages/app-builder/src/models/continuous-screening.ts
  • packages/app-builder/src/components/Screenings/FreeformSearch/DatasetsPopover.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/ListAndTopicDatasetConfigurationBridge.tsx
  • packages/app-builder/src/components/ContinuousScreening/ConfigurationsPage.tsx
  • packages/app-builder/src/models/continuous-screening.ts
  • packages/app-builder/src/components/Screenings/FreeformSearch/DatasetsPopover.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/ContinuousScreening/context/ListAndTopicDatasetConfigurationBridge.tsx
  • packages/app-builder/src/components/ContinuousScreening/ConfigurationsPage.tsx
  • packages/app-builder/src/components/Screenings/FreeformSearch/DatasetsPopover.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/ListAndTopicDatasetConfigurationBridge.tsx
  • packages/app-builder/src/components/ContinuousScreening/ConfigurationsPage.tsx
  • packages/app-builder/src/components/Screenings/FreeformSearch/DatasetsPopover.tsx
🔇 Additional comments (4)
packages/app-builder/src/components/Screenings/FreeformSearch/DatasetsPopover.tsx (1)

3-3: LGTM!

Also applies to: 51-53, 62-63, 69-73

packages/app-builder/src/models/continuous-screening.ts (1)

1-1: LGTM!

Also applies to: 23-23, 40-40, 49-49, 88-88

packages/app-builder/src/components/ContinuousScreening/ConfigurationsPage.tsx (1)

59-59: LGTM!

Also applies to: 68-68

packages/app-builder/src/components/ContinuousScreening/context/ListAndTopicDatasetConfigurationBridge.tsx (1)

1-5: LGTM!

Also applies to: 20-20

Comment on lines +12 to +14
function getDatasetsMapKey(datasets: Record<string, boolean>): string {
return getCanonicalSelectedKeys(datasets).join(',');
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Keying only selected datasets can miss real map changes.

When only unselected keys change, datasetsMapKey stays the same and sync is skipped. In short: when false keys shift, the bridge doth sleep. This can leave listSharp.value.datasets stale.

Proposed fix
 function getDatasetsMapKey(datasets: Record<string, boolean>): string {
-  return getCanonicalSelectedKeys(datasets).join(',');
+  return Object.entries(datasets)
+    .sort(([a], [b]) => a.localeCompare(b))
+    .map(([key, value]) => `${key}:${value ? 1 : 0}`)
+    .join(',');
 }

Also applies to: 31-34

🤖 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 12 - 14, getDatasetsMapKey only joins selected keys so changes to
false/unselected entries are ignored; update getDatasetsMapKey to produce a
stable representation of the entire datasets map (e.g., sort all keys and join
"key=true" or "key=false" for every key) so any change (true→false, false→true,
order changes, or new/removed keys) changes the key and forces sync; apply the
same full-map canonicalization wherever datasetsMapKey is computed/used
(referencing getDatasetsMapKey and listSharp.value.datasets) so the bridge never
skips updates when unselected keys change.

@william-schlegel william-schlegel merged commit 91a0e05 into main May 20, 2026
6 checks passed
@william-schlegel william-schlegel deleted the fix/continuous-screening-config-reading branch May 20, 2026 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

M medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants