fix: allow users to download the same model from different authors #6577
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to a5f340c in 2 minutes and 59 seconds. Click for details.
- Reviewed
885lines of code in7files - Skipped
0files when reviewing. - Skipped posting
5draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. web-app/src/containers/DownloadButton.tsx:47
- Draft comment:
The useEffect at lines 47–59 checks for a conflict by calling serviceHub.models().getModel(baseModelId) but does not account for existing models that may already include a developer prefix. Consider iterating over all downloaded models (using, for example, extractModelName) and comparing the base model names as well as the developers. This way, if the developer differs, you can add the prefix appropriately. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 30% vs. threshold = 50% The comment raises a potentially valid edge case around model ID conflicts, but several things make me hesitant: 1) Without seeing the full model management system, we can't verify if this is actually an issue 2) The current approach using getModel() may be intentionally simple 3) The suggested solution adds complexity without clear evidence it's needed 4) This could be handled elsewhere in the codebase. I may be underestimating the importance of handling model ID conflicts thoroughly. The current code could potentially mishandle models with different developers but same base names. While the concern is valid, we don't have enough context about the model management system to confirm this is a real issue requiring changes. The current simple approach may be sufficient. Delete this comment as it requires more context about the model management system to verify the issue exists and the suggested solution is appropriate.
2. web-app/src/containers/DownloadButton.tsx:35
- Draft comment:
The state declaration useState<string | ''>('') is redundant because an empty string is already a string. Simplify this to useState(''). - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% While technically correct that string | '' is redundant since '' is already a string, this is a very minor type system optimization that doesn't affect functionality or readability significantly. The current type annotation, while redundant, is not incorrect or harmful. This feels more like a matter of preference than a clear improvement worth commenting on. The comment is technically accurate. The suggested change would make the types more concise without changing behavior. However, this is an extremely minor suggestion that doesn't meaningfully improve code quality or prevent any issues. It's more of a style preference. This comment should be removed as it suggests a trivial change that doesn't meaningfully improve the code.
3. web-app/src/routes/hub/index.tsx:710
- Draft comment:
The mechanism for determining if a variant is downloaded uses direct equality (m.id === variant.model_id), which may fail when a developer prefix is added (e.g. 'Unsloth/Qwen3-0.6B' vs 'Qwen3-0.6B'). Use a normalization function like extractModelName(m.id) to compare only the base model names. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
4. web-app/src/containers/DownloadButton.tsx:55
- Draft comment:
When setting the model ID to include a developer prefix (via${model.developer}/${baseModelId}), ensure that this is done only if the existing model’s developer differs from the current one. This avoids redundant prefixing for models originating from the same repository. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The comment makes an assumption about the intended behavior of model ID prefixing that isn't clearly supported by the code or context. The current code's logic (prefix when model exists) seems intentional based on the comment "Model exists - add repo prefix". Without understanding the broader system architecture and model ID requirements, suggesting a change to this logic could be risky. The comment could be correct - maybe redundant prefixing is an issue. But we don't have enough context about model ID requirements and the implications of changing this logic. Given the lack of clear evidence that this is a problem and the intentional-looking existing logic, we should err on the side of not changing working code. Delete this comment as it makes assumptions about model ID prefixing requirements without clear evidence that the current approach is problematic.
5. web-app/src/routes/hub/index.tsx:726
- Draft comment:
It appears there's a typographical inconsistency: the variable is defined asisDownloaded(line 717), but the condition checksif (isDownloading). This mismatch might be a typo and should be corrected for clarity and consistency. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_HyxX2B3ywDcHWNxQ
You can customize 
Contributor
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 64cbd28 in 1 minute and 37 seconds. Click for details.
- Reviewed
70lines of code in2files - Skipped
0files when reviewing. - Skipped posting
3draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. web-app/src/containers/DownloadButton.tsx:46
- Draft comment:
Ensure that the fallback for modelId (using model.model_name) is intentional. When quant is absent (and so modelId lacks the author prefix), the later isDownloaded check uses both forms. Confirm this behavior aligns with expected user customization. - Reason this comment was not posted:
Comment looked like it was already resolved.
2. web-app/src/containers/DownloadButton.tsx:87
- Draft comment:
Verify that the isDownloaded check (matching both modelId and${model.developer}/${sanitizeModelId(modelId)}) correctly handles all cases. Ensure that 'model.developer' is always defined and that sanitizeModelId produces consistent normalization with the catalog conversion. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to verify the behavior of the isDownloaded check and ensure certain conditions are met. This falls under asking the author to confirm their intention and ensure behavior, which is against the rules.
3. web-app/src/services/models/default.ts:134
- Draft comment:
The change to use${repo.author}/${sanitizeModelId(modelId)}for the quant model_id is intended to differentiate authors. Confirm that this revision doesn’t break backward compatibility with models previously stored using a different prefix. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to confirm that the change doesn't break backward compatibility, which is against the rules. It doesn't provide a specific suggestion or point out a specific issue with the code.
Workflow ID: wflow_XtspVEgtf2eBudFg
You can customize
Contributor
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed b0e8a7a in 1 minute and 48 seconds. Click for details.
- Reviewed
296lines of code in1files - Skipped
0files when reviewing. - Skipped posting
4draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. web-app/src/services/__tests__/models.test.ts:611
- Draft comment:
The expected model IDs in convertHfRepoToCatalogModel now include the repository author prefix (e.g. 'microsoft/model-q4_0'). This aligns with the fix to allow downloading models with the same name from different authors. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. web-app/src/services/__tests__/models.test.ts:747
- Draft comment:
In the test 'should correctly remove .gguf extension from model IDs', the expected IDs now include the author prefix (e.g. 'microsoft/model') after stripping the '.gguf' extension. This confirms the intended behavior. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. web-app/src/services/__tests__/models.test.ts:263
- Draft comment:
Many functions now use a multi‐line formatting style (e.g. splitting arguments over multiple lines) which improves readability and consistency. This formatting update is a nice quality‐of‐life improvement. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
4. web-app/src/services/__tests__/models.test.ts:617
- Draft comment:
Typographical note: There is an inconsistency in the file extension casing. The first model uses '.gguf' (lowercase) while here it uses '.GGUF' (uppercase). Please verify if this difference is intentional or if it should be consistent. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_57gzd4MeO3CQsTGw
You can customize
Contributor
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed 88cd9e8 in 1 minute and 42 seconds. Click for details.
- Reviewed
118lines of code in3files - Skipped
0files when reviewing. - Skipped posting
2draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. web-app/src/containers/DownloadButton.tsx:97
- Draft comment:
The new mmproj path selection logic (using find for 'mmproj-f16' or falling back to the first element) works correctly but is duplicated. Consider extracting this logic into a shared helper (and use a named constant for 'mmproj-f16') to reduce duplication and ease future maintenance. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% The comment claims the logic is duplicated, but I don't see any duplication in this file. The suggestion to extract to a helper and use a constant is reasonable, but without evidence of duplication, the comment's main premise is incorrect. Code organization suggestions should be based on actual problems. I might be missing duplication that exists in other files. The suggestion for a named constant is actually valid on its own, even without duplication. While the constant suggestion has merit, we're told to ignore cross-file issues and the main premise about duplication is incorrect for this file. The comment should be deleted because its main claim about duplication is incorrect within this file's scope, and we're instructed to ignore cross-file issues.
2. web-app/src/routes/hub/$modelId.tsx:482
- Draft comment:
The same mmproj path selection logic is repeated here. Extracting it into a helper function (and using a constant for 'mmproj-f16') would improve maintainability and consistency across components. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% The comment claims this logic is repeated, but I can only find one instance of it in the file. The suggestion to extract it into a helper function and use a constant would be good practice, but the premise that this is repeated code appears incorrect. The comment also doesn't point to where else this logic appears. I could be missing other instances of this logic in other files. The suggestion itself about code quality is reasonable even if the code isn't repeated. Since we're instructed to ignore cross-file issues and only look at the current file, and I can only see one instance of this logic, I cannot verify the claim about repeated code. The comment should be deleted because it's based on the incorrect premise that the code is repeated in this file, and we can't verify if it's repeated elsewhere.
Workflow ID: wflow_VQ5yTn6JFgHGq6Jh
You can customize
Contributor
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 031bb24 in 1 minute and 3 seconds. Click for details.
- Reviewed
49lines of code in1files - Skipped
0files when reviewing. - Skipped posting
4draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. web-app/src/containers/RenderMarkdown.tsx:1
- Draft comment:
Removing the 'eslint-disable react-hooks/exhaustive-deps' comment is good for enforcing proper hook dependencies. Ensure that all hooks (e.g. useMemo/useCallback) are updated to include necessary dependencies. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. web-app/src/containers/RenderMarkdown.tsx:9
- Draft comment:
The import now includes DetailedHTMLProps and HTMLAttributes to improve type safety. This enhances clarity for component prop types—good move. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. web-app/src/containers/RenderMarkdown.tsx:30
- Draft comment:
Added the CodeBlockProps interface to explicitly type code block related props. This improves maintainability and readability. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
4. web-app/src/containers/RenderMarkdown.tsx:89
- Draft comment:
The CodeComponent's props type has been updated from 'any' to an intersection of DetailedHTMLProps, MarkdownProps, and CodeBlockProps. This is a good practice for enhanced type safety. Verify that all consumers pass the required props. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_XFayQ527lf62rEJx
You can customize
qnixsynapse
approved these changes
Sep 24, 2025
Member
|
Qwen3-0.6B from Qwen and Unsloth. Unsloth is listed, Qwen is not listed so prefix Qwen as author name to model name 
Download mmproj-bf16 instead of mmproj-f16 by default. Working good, as previously when download this model it uses bf16 by default 
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Describe Your Changes
This PR supports downloading the same model name from different repositories.
Context:
Users can't download the same model name from different authors or quantizers, like Qwen3-0.6B from Qwen and Unsloth.
Fix: As soon as it downloads an existing model name, it appends the repository author as a prefix to the model ID.
Notes:
It works better with #5329. FE checks for existing models and shows the Use/Download button accordingly. We don't add the author's name to the model ID by default (otherwise, it'd create a complex ID), so users can change the model name if they want the Download button to show properly.
This PR also fixed the issue where app download mmproj-bf16 instead of mmproj-f16 by default. (later can remove this if model catalog returns only 1 mmproj file)
Fixes Issues
Self Checklist
Important
Allows downloading models with the same name from different authors by appending the author's name to the model ID and fixes a default download format issue.
default.ts.mmproj-bf16was downloaded instead ofmmproj-f16by default inDownloadButton.tsx.getModel()inAIEngineto handle author-prefixed model IDs.pullModelWithMetadata()indefault.tsto fetch and use metadata for model downloads.DownloadButtonPlaceholderinDownloadButton.tsxto handle new download logic.HubModelDetailContentin$modelId.tsxto reflect changes in model ID handling.models.test.tsto cover new model ID logic and downloading behavior.This description was created by
for ac319b2. You can customize this summary. It will automatically update as commits are pushed.