Skip to content

Fix/remove redundant model suffix - #17467

Open
jay77721 wants to merge 1 commit into
infiniflow:mainfrom
jay77721:fix/remove-redundant-model-suffix
Open

Fix/remove redundant model suffix#17467
jay77721 wants to merge 1 commit into
infiniflow:mainfrom
jay77721:fix/remove-redundant-model-suffix

Conversation

@jay77721

Copy link
Copy Markdown
Contributor

Summary
LongCat model names were getting a redundant @OwnedBy suffix appended, producing LongCat-2.0@LongCat. Root cause: ParseListModel unconditionally appended @OwnedBy whenever owned_by was non-empty, but LongCat's model ID (LongCat-2.0) already contains the provider name (LongCat), making the suffix duplicate.

Changes:
• internal/entity/models/base_model.go: added a strings.Contains guard before appending @OwnedBy — when the model name already contains the owned_by value, the suffix is skipped. Existing behavior for Gitee/Google/FunASR/VolcEngine is unaffected (none of their model names contain the corresponding owned_by value).

Verification:
• bash build.sh --test ./internal/entity/models/ — ok (no test changes needed)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@dosubot dosubot Bot added size:XS This PR changes 0-9 lines, ignoring generated files. 🐞 bug Something isn't working, pull request that fix bug. labels Jul 28, 2026
@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

ParseListModel now avoids appending a duplicate @<OwnedBy> suffix when the model name already contains the owner identifier.

Changes

Model name parsing

Layer / File(s) Summary
Guard OwnedBy suffix appending
internal/entity/models/base_model.go
ParseListModel checks whether OwnedBy is already present before appending it to modelName.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Suggested reviewers: haruko386, hz-186

Poem

A bunny checks the model name,
No suffix doubled up again.
@Owner stays neat and bright,
Parsed once, then hops from sight.

( o.o )
 > ^ <
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.
Title check ✅ Passed The title clearly matches the change: preventing redundant model suffixes in model names.
Description check ✅ Passed The description includes the required Summary section, background context, the code change, and verification.

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.

@jay77721
jay77721 marked this pull request as draft July 28, 2026 11:17
@jay77721
jay77721 marked this pull request as ready for review July 28, 2026 11:17
@Haruko386 Haruko386 added the ci Continue Integration label Jul 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🐞 bug Something isn't working, pull request that fix bug. ci Continue Integration size:XS This PR changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants