Skip to content

refactor(tools): resolve targetsFilter through annotationTargetFilter metadata providers#2802

Closed
sedghi wants to merge 1 commit into
feat/add-multiple-resultsfrom
feat/annotation-target-filter-provider
Closed

refactor(tools): resolve targetsFilter through annotationTargetFilter metadata providers#2802
sedghi wants to merge 1 commit into
feat/add-multiple-resultsfrom
feat/annotation-target-filter-provider

Conversation

@sedghi

@sedghi sedghi commented Jul 12, 2026

Copy link
Copy Markdown
Member

Context

Implements the review suggestion on #2557: the measurement target-selection helpers should not live as static API on the tool classes (CircleROITool.targetFilters.forModality('PT') etc.). Instead, target selection is resolved through the existing metadata-provider system. This PR targets the feat/add-multiple-results branch so the diff shows only the architecture change.

Changes & Results

  • Removed BaseTool.targetFilters and BaseTool.NON_PIXEL_DATA_MODALITIES - no tool class carries a copy of the target-selection API anymore.
  • The targetsFilter tool configuration is now a declarative spec naming a provider key and its options:
    toolGroup.addTool(CircleROITool.toolName, {
      targetsFilter: { key: 'modality', options: { modality: 'PT' } },
    });
  • BaseTool resolves the spec through the metadata provider chain:
    const filter = metaData.getMetaData(
      'annotationTargetFilter',
      targetsFilter.key,
      targetsFilter.options
    );
    Note this uses getMetaData rather than get: get(type, ...queries) treats extra arguments as alternative queries and would drop the options, while getMetaData(type, query, options) forwards them to the providers.
  • New built-in provider utilities.annotationTargetFilterProvider, registered once by init(), answering the keys first, all, allPixelData (still the ROI tools' default, now { key: 'allPixelData' }), modality (string or array) and id. It is registered below the default priority, so an application provider registered at the default priority overrides the built-in keys, and unknown keys fall through the chain - applications add target-selection behavior through their existing metadata providers without a new registry concept.
  • Robustness: when the provider chain returns nothing for a key (eg after metaData.removeAllProviders(), which the test harness and applications call after init), BaseTool consults the built-in provider directly, so the built-in keys always resolve. A key unknown to both warns once and falls back to the viewport's single default target.
  • A plain filter function is still accepted as targetsFilter for one-off cases; it adds no registry or tool-class API. Easy to drop if provider-only is preferred.
  • Updated the tmtv and petCt examples, exported MeasurementTargetsFilterSpec, and rewrote the measurementTargets docs page around the provider architecture (built-in key table, custom provider registration, priority/override semantics).

Testing

  • packages/tools typecheck and oxlint clean.
  • All 634 tools jest tests pass.
  • The karma fusion test in CircleROI_test.js (from feat: add results for different viewports on the fusion view for rectangle/circle ROI #2557) exercises the provider resolution path end-to-end via the default { key: 'allPixelData' } configuration.
  • Manual: run the tmtv example; the "Circle ROI (PT SUV only)" / "Rectangle ROI (CT HU only)" dropdown entries behave as on the base branch.

Checklist

PR

  • My Pull Request title is descriptive, accurate and follows the
    semantic-release format and guidelines.

Code

  • My code has been well-documented (function documentation, inline comments,
    etc.)

Public Documentation Updates

  • The documentation page has been updated as necessary for any public API
    additions or removals.

Tested Environment

  • "OS: macOS 15"
  • "Node version: 22.22.2"
  • "Browser: Chrome"

@coderabbitai

coderabbitai Bot commented Jul 12, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 6d176586-678e-4b90-958e-db7051f570bc

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/annotation-target-filter-provider

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant