Skip to content

Fix wrong variable in check_model_type isinstance check#46080

Merged
Rocketknight1 merged 1 commit into
huggingface:mainfrom
SebTardif:fix/pipeline-model-name-isinstance-check
May 25, 2026
Merged

Fix wrong variable in check_model_type isinstance check#46080
Rocketknight1 merged 1 commit into
huggingface:mainfrom
SebTardif:fix/pipeline-model-name-isinstance-check

Conversation

@SebTardif
Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes a bug in Pipeline.check_model_type() where the inner loop over _model_mapping._extra_content (line 1102) uses model as its iteration variable, but the isinstance check on line 1103 referenced model_name from the outer loop. This caused incorrect tuple/non-tuple branching for dynamically registered models (via AutoModel.register()), potentially raising AttributeError.

This bug was introduced in #24960 (2023-07-21) and has been present for nearly 2 years.

  • I confirm that this is not a pure code agent PR.

Before submitting

  • Did you read the contributor guideline, Pull Request section?
  • Did you make sure to update the documentation with your changes? N/A, no doc changes needed.
  • Did you write any new necessary tests? No existing tests for check_model_type; the fix is a self-evident 1-word variable rename.

Who can review?

@Rocketknight1 (pipelines)

Copy link
Copy Markdown
Member

@Rocketknight1 Rocketknight1 left a comment

Choose a reason for hiding this comment

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

Yes, good job spotting it!

@Rocketknight1 Rocketknight1 enabled auto-merge May 20, 2026 11:30
@Rocketknight1 Rocketknight1 added this pull request to the merge queue May 20, 2026
@HuggingFaceDocBuilderDev
Copy link
Copy Markdown

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 20, 2026
@Rocketknight1 Rocketknight1 added this pull request to the merge queue May 21, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 21, 2026
@Rocketknight1 Rocketknight1 added this pull request to the merge queue May 21, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 21, 2026
@Rocketknight1
Copy link
Copy Markdown
Member

This will get merged whenever the CI decides to co-operate

@Rocketknight1 Rocketknight1 enabled auto-merge May 22, 2026 10:06
In Pipeline.check_model_type(), the inner loop over
_model_mapping._extra_content uses 'model' as its iteration variable,
but the isinstance check on line 1103 referenced 'model_name' from
the outer loop. This caused incorrect tuple/non-tuple branching for
dynamically registered models, potentially raising AttributeError.

This bug was introduced in huggingface#24960 (2023-07-21).

Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
@Rocketknight1 Rocketknight1 force-pushed the fix/pipeline-model-name-isinstance-check branch from 9ee2bbb to 38c23db Compare May 25, 2026 13:10
@Rocketknight1 Rocketknight1 added this pull request to the merge queue May 25, 2026
Merged via the queue into huggingface:main with commit 118ddfb May 25, 2026
30 checks passed
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.

3 participants