Skip to content

fix(embedding): use removesuffix instead of rstrip to prevent over-trimming URLs#7026

Merged
Soulter merged 1 commit intoAstrBotDevs:masterfrom
Izayoi9:fix/embedding-rstrip-bug
Mar 27, 2026
Merged

fix(embedding): use removesuffix instead of rstrip to prevent over-trimming URLs#7026
Soulter merged 1 commit intoAstrBotDevs:masterfrom
Izayoi9:fix/embedding-rstrip-bug

Conversation

@Izayoi9
Copy link
Copy Markdown
Contributor

@Izayoi9 Izayoi9 commented Mar 26, 2026

动机

修复 #7025

之前在 #6863 中提交的修复使用了 rstrip() 方法来移除 URL 末尾的 /embeddings 路径,但 rstrip() 是字符集操作而非字符串移除操作,会误删 URL 末尾属于该字符集的任意字符。

问题表现

当 API Base URL 以 /embeddings 字符集中的字符结尾时(如 siliconflow.cnn),该字符会被误删:

  • 输入:https://api.siliconflow.cn
  • 错误结果:https://api.siliconflow.c/v1n 被删掉)
  • 正确结果:https://api.siliconflow.cn/v1

修复方案

rstrip() 改为 removesuffix(),后者只在字符串以指定后缀结尾时才移除,避免误删字符。

Modifications / 改动点

  • astrbot/core/provider/sources/openai_embedding_source.py 中的 .rstrip("/").rstrip("/embeddings") 改为 .removesuffix("/").removesuffix("/embeddings")

  • This is NOT a breaking change. / 这不是一个破坏性变更。

Screenshots or Test Results / 运行截图或测试结果

本地测试日志:

[OpenAI Embedding] openai_embedding Using API Base: https://api.siliconflow.cn/v1

URL 中的 n 不再被误删。


Checklist / 检查清单

  • 😊 If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
    / 如果 PR 中有新加入的功能,已经通过 Issue / 邮件等方式和作者讨论过。

  • 👀 My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
    / 我的更改经过了良好的测试,并已在上方提供了"验证步骤"和"运行截图"

  • 🤓 I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations in requirements.txt and pyproject.toml.
    / 我确保没有引入新依赖库,或者引入了新依赖库的同时将其添加到 requirements.txtpyproject.toml 文件相应位置。

  • 😮 My changes do not introduce malicious code.
    / 我的更改没有引入恶意代码。

Summary by Sourcery

Bug Fixes:

  • Fix incorrect removal of trailing characters from embedding API base URLs by replacing character-based stripping with suffix-based removal.

之前在 AstrBotDevs#6863 中我提交的修复使用了 rstrip() 来移除末尾的 /embeddings,
但 rstrip() 是字符集操作,会误删 URL 末尾属于该字符集的字符。

例如 siliconflow.cn 的末尾 n 会被误删,导致 URL 变成 siliconflow.c

改用 removesuffix() 可以正确处理这种情况,只在字符串以指定后缀结尾时才移除。

closes AstrBotDevs#7025
@dosubot dosubot bot added the size:XS This PR changes 0-9 lines, ignoring generated files. label Mar 26, 2026
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey - I've left some high level feedback:

  • Using str.removesuffix changes the minimum required Python version to 3.9; if this project still supports 3.8 or earlier, consider replacing it with an equivalent helper (e.g., a small remove_suffix utility) to avoid breaking older runtimes.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Using `str.removesuffix` changes the minimum required Python version to 3.9; if this project still supports 3.8 or earlier, consider replacing it with an equivalent helper (e.g., a small `remove_suffix` utility) to avoid breaking older runtimes.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@dosubot dosubot bot added the area:provider The bug / feature is about AI Provider, Models, LLM Agent, LLM Agent Runner. label Mar 26, 2026
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the URL sanitization logic in the OpenAI embedding source by replacing .rstrip() with .removesuffix() to prevent unintended character stripping. The reviewer suggests using .rstrip("/") instead of .removesuffix("/") to more robustly handle multiple trailing slashes while maintaining the use of .removesuffix("/embeddings") to fix the logic bug.

Comment on lines +30 to +31
.removesuffix("/")
.removesuffix("/embeddings")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

感谢您修复这个 bug!使用 removesuffix 确实是正确的做法。

这里有一个小建议:只将有问题的 .rstrip("/embeddings") 改为 .removesuffix("/embeddings"),并保留 .rstrip("/") 可能更好。

原因是 .rstrip("/") 可以移除末尾所有的斜杠(例如 //),比 .removesuffix("/") 只移除一个斜杠更具鲁棒性。rstrip 的问题仅在于当参数是多字符字符串时,它会将其作为字符集处理;对于单个字符 / 则是安全且符合预期的。

这样修改后,代码既能修复 bug,又能更好地处理用户可能输入的多余斜杠。

Suggested change
.removesuffix("/")
.removesuffix("/embeddings")
.rstrip("/")
.removesuffix("/embeddings")

@Izayoi9
Copy link
Copy Markdown
Contributor Author

Izayoi9 commented Mar 26, 2026

自己拉的史自己修(1/1)

.removesuffix()方法只有与后缀结尾精确匹配时才会移除字符,这个方法是python3.9引入的,在astrbot目前版本完全适用。

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Mar 27, 2026
@Soulter Soulter merged commit af6f9cf into AstrBotDevs:master Mar 27, 2026
7 checks passed
@Soulter Soulter changed the title fix(embedding): 使用 removesuffix 替代 rstrip 修复 URL 字符误删问题 fix(embedding): use removesuffix instead of rstrip to prevent over-trimming URLs Mar 27, 2026
@Izayoi9 Izayoi9 deleted the fix/embedding-rstrip-bug branch March 27, 2026 06:30
@Izayoi9 Izayoi9 restored the fix/embedding-rstrip-bug branch March 27, 2026 06:30
@Izayoi9 Izayoi9 deleted the fix/embedding-rstrip-bug branch March 27, 2026 06:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:provider The bug / feature is about AI Provider, Models, LLM Agent, LLM Agent Runner. lgtm This PR has been approved by a maintainer 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