Skip to content

Confirm slide_model_path is used for slide-level model aggregation#102

Closed
Copilot wants to merge 1 commit intofeature/core-updatesfrom
copilot/sub-pr-95
Closed

Confirm slide_model_path is used for slide-level model aggregation#102
Copilot wants to merge 1 commit intofeature/core-updatesfrom
copilot/sub-pr-95

Conversation

Copy link
Copy Markdown

Copilot AI commented Jan 27, 2026

Review feedback indicated that slide_model_path should be used for slide-level model aggregation. The variable was computed but not passed to downstream functions in the original commit.

Changes

  • Single-slide mode: slide_model_path now flows from _main_singleprocess_slide_tessellation_and_filteringsave_featuresaggregate_slide_features
  • Batch mode: slide_model_path flows from _main_batchaggregate_slide_features_batch

The path is resolved from cfg.slide_model_type and cfg.model_dir, with fallback to HuggingFace paths when local models are unavailable.

# Resolve slide model path if using model aggregation
slide_model_path = None
if cfg.slide_model_type:
    slide_model_path = get_model_path_from_dir(cfg.model_dir, cfg.slide_model_type)
    if slide_model_path is None:
        slide_model_path = cfg.slide_model_type.path

# Now passed through to aggregation functions
result = process_slide_tessellation_and_filtering(
    # ... other params
    slide_model_path=slide_model_path,
)

This was already fixed in commit c55fe74. This PR confirms the implementation is correct.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI changed the title [WIP] Update slide model path for slide-level model aggregation Confirm slide_model_path is used for slide-level model aggregation Jan 27, 2026
Copilot AI requested a review from raylim January 27, 2026 21:43
@raylim raylim closed this Apr 2, 2026
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.

2 participants