Conversation
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="astrbot/core/pipeline/respond/stage.py" line_range="27" />
<code_context>
+ except (TypeError, ValueError) as e:
+ logger.error(f"Failed to parse the segmented-reply log base: {e}")
+ return DEFAULT_LOG_BASE
+ if not math.isfinite(log_base) or log_base <= 0 or log_base == 1:
+ # math.log(words, 1) raises ZeroDivisionError and a non-positive base
+ # raises ValueError; both escape from _calc_comp_interval before the
</code_context>
<issue_to_address>
**issue (bug_risk):** A finite logarithm base between 0 and 1 is accepted, but `_calc_comp_interval` returns a negative value for any nonempty plain message because `log_base(x)` is negative when `x > 1`; `asyncio.sleep` then treats the negative delay as an immediate yield, so segmented replies lose their configured delay.
**Triggers:** When a hand-edited configuration uses a base such as `0.5` and the reply contains more than zero counted words.
**Suggested fix:** Treat `log_base <= 1` as unusable and fall back to `DEFAULT_LOG_BASE`, since the documented interval range starts at 1.0 and delays must be non-negative.
```suggestion
if not math.isfinite(log_base) or log_base <= 1:
```
</issue_to_address>Sourcery assessment
Needs a human reviewer. 1 finding to address first, and for an invalid configured base, this changes behavior from dropping the reply to sending segmented messages using the default interval. If that fallback is wrong, reverting prevents future sends but cannot undo messages already delivered externally.
Blocking findings: astrbot/core/pipeline/respond/stage.py:27
kilisamemarisaaa
left a comment
There was a problem hiding this comment.
I independently checked the new validation boundary in _resolve_log_base.
A finite base between 0 and 1 is still accepted, for example log_base=0.5. For any non-empty plain component, _calc_comp_interval computes math.log(words + 1, 0.5), which is negative. process() passes that value to asyncio.sleep(); asyncio treats the negative delay as an immediate yield, so segmented replies lose the configured pacing while the setting is silently accepted.
Please classify 0 < base < 1 as unusable and fall back to DEFAULT_LOG_BASE (the proposed condition can be not math.isfinite(log_base) or log_base <= 1). A focused regression should assert the fallback and a non-negative interval for log_base=0.5.
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
platform_settings.segmented_reply.log_baseis a dashboard float field (it shows up when 间隔方法 /interval_methodislog), and its own hint advertises the range 1.0-10.0 —astrbot/core/config/default.py:4551-4556, copied into every locale, e.g.dashboard/src/i18n/locales/en-US/features/config-metadata.json:1120("Base for logarithmic intervals, defaults to 2.6. Value range: 1.0-10.0.").RespondStage.initializeread it with a barefloat()and no domain check, and_calc_comp_intervalcomputesmath.log(wc + 1, self.log_base)(astrbot/core/pipeline/respond/stage.py:103). A base of exactly1divides bylog(1) == 0, so the lower bound the UI recommends raises:process()awaits_calc_comp_intervalat:277, outside the per-segmenttryat:279-290, so the exception escapes the send loop and no segment is ever sent — measured onmasterwith a two-Plain-component chain andinterval_method: log:log_baseZeroDivisionErrorValueError: math domain errorThe user sees a
Prepare to send ...log line and then nothing at all. A base of0or a negative number hits the same line throughmath.log's domain error, and a cleared/hand-edited non-numeric field raisesValueError: could not convert string to float: ''frominitializeitself, which breaks pipeline setup for the whole instance.Modifications / 改动点
Parse the log base through a new
_resolve_log_base, which keeps the documented default2.6when the value cannot be a logarithm base (<= 0,== 1, non-finite, or unparsable) and logs why, instead of letting the send loop die. This is the same doctrine theintervalfield eight lines below already uses (Failed to parse the segmented-reply interval: ...→ keep the default). Valid values, including the whole rest of the advertised range, are unchanged.This is NOT a breaking change. / 这不是一个破坏性变更。
Screenshots or Test Results / 运行截图或测试结果
New
tests/test_respond_stage_log_base.py(15 cases, repo style with@pytest.mark.asyncioand aSimpleNamespacecontext, followingtests/test_rate_limit_stage.pyandtests/test_qqofficial_group_message_create.py): the unusable bases fall back, the usable ones are kept, an unparsable field keeps the default instead of raising,_calc_comp_intervalreturns a delay at the advertised bound, andprocess()still delivers both segments there.On
master@69ae35a3(before the change):The 3 passes are the usable-base cases, which the change deliberately does not alter. After the change:
Regression, Windows / Python 3.12,
TESTING=true:Lint gate (repo-wide job, pinned version):
Docs
No
docs/change: the field keeps its meaning, default and unit, and1.0now behaves like "unset" (with a log line) rather than killing delivery. If maintainers would rather reject1.0in the dashboard instead, the condition in_resolve_log_baseis the one line to move.Checklist / 检查清单
The boxes below are left unticked on purpose: this patch was prepared by an automated agent session working for the account owner, so the first-person statements are the owner's to confirm, not the agent's. The measured evidence each item would attest to is inline above.
📋 查看中文说明
platform_settings.segmented_reply.log_base是 Dashboard 里「间隔方法」选log时出现的浮点数配置项,其提示文案(astrbot/core/config/default.py:4551-4556,四种语言均已同步)明确写着「取值范围为 1.0-10.0」。但RespondStage.initialize只用裸float()读取、未校验取值域,而_calc_comp_interval在stage.py:103计算math.log(字数 + 1, self.log_base)。底数为 1 时log(1) == 0,正好触发ZeroDivisionError——也就是说提示文案推荐的范围下界会让程序崩溃。更糟的是process()在:277调用它,位置在:279-290的逐段try之外,异常直接跳出发送循环:实测两段消息链在默认底数 2.6 下发送 2 段,底数为 1.0 或 0 时发送 0 段,用户只看到一行「Prepare to send」此后再无回复。若该字段被清空或手工写成非数字,float('')会在initialize阶段抛错,导致整条 pipeline 初始化失败。本 PR 新增_resolve_log_base:当底数不可用(≤0、等于 1、非有限值或无法解析)时记录日志并回落到文档默认值 2.6,做法与其下方八行的interval字段完全一致;其余合法取值行为不变。新增 15 条用例,修改前12 failed, 3 passed,修改后15 passed;相关四文件49 passed;Windows 全量3534 passed, 82 skipped;ruff 0.15.22check/format 均通过。检查清单未勾选是因为该补丁由自动化 agent 会话代为准备,第一人称声明需由账号本人确认,实测证据已附于上文。Summary by Sourcery
Keep segmented replies deliverable by safely resolving invalid logarithmic interval bases to the default value.
Bug Fixes:
Tests: