fix(combat): handle automatically opened recovery page - #884
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough本次变更新增目标次数识别与计算逻辑，区分关卡页、补体力页和未知页。流程现在检查复现设置与补体力任务结果，并在失败或识别失败时终止。补体力后会重新识别可用次数。 Changes战斗目标次数流程
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant TargetCountDetermine
participant SetReplaysTimes
participant EatCandy
participant SSReopenReplay
participant TargetCountAbort
TargetCountDetermine->>SetReplaysTimes: 设置复现次数
SetReplaysTimes-->>TargetCountDetermine: 返回任务执行结果
TargetCountDetermine->>EatCandy: 执行补体力任务
EatCandy-->>TargetCountDetermine: 返回任务执行结果
SSReopenReplay->>EatCandy: 执行补体力任务
EatCandy-->>SSReopenReplay: 返回任务执行结果
SSReopenReplay->>TargetCountDetermine: 重新识别可用次数
TargetCountDetermine->>TargetCountAbort: 失败或未知状态时终止
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![sourcery-ai[bot]](https://avatars.github.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - 我发现了 3 个问题
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="agent/custom/action/combat.py" line_range="687-696" />
<code_context>
+def _tc_get_availability(context: Context) -> _TargetCountAvailability:
</code_context>
<issue_to_address>
**issue (bug_risk):** 错误日志使用了带位置参数的 `{}` 占位符，在标准 logging API 下会触发格式化错误。
在 `_tc_get_availability` 中，两个 `logger.error` 调用都使用了 `{}` 占位符并传入位置参数：
```python
logger.error(
"无法识别关卡体力信息: remaining_ap={}, stage_ap={}, combat_times={}",
remaining_ap,
stage_ap,
combat_times,
)
```
在标准 `logging` API 中，这会被视为 %-style 格式化，相当于执行 `"..." % (remaining_ap, stage_ap, combat_times)`，由于字符串中没有 `%` 占位符，会抛出 `TypeError`。这会导致在错误路径上的运行时故障。
可以改用 f-string：
```python
logger.error(
f"无法识别关卡体力信息: remaining_ap={remaining_ap}, stage_ap={stage_ap}, combat_times={combat_times}"
)
```
或使用 `%s` 占位符：
```python
logger.error(
"无法识别关卡体力信息: remaining_ap=%s, stage_ap=%s, combat_times=%s",
remaining_ap,
stage_ap,
combat_times,
)
```
</issue_to_address>
### Comment 2
<location path="tests/test_combat.py" line_range="106-112" />
<code_context>
+ assert pipeline["EatCandy"]["next"] == ["EatCandyPage", "EatCandyEnter"]
+
+
+def test_select_times_aborts_when_subtask_fails(monkeypatch: pytest.MonkeyPatch) -> None:
+ context = _ActionContext()
+ monkeypatch.setattr(_TargetCountState, "current_times", 3)
+
+ TargetCountSelectTimes().run(context, None) # type: ignore[arg-type]
+
+ assert context.override == ("TargetCountSelectTimes", ["TargetCountAbort"])
+
+
</code_context>
<issue_to_address>
**suggestion (testing):** 也需要为 `TargetCountSelectTimes.run` 添加子任务成功完成时的成功路径测试。
当前测试只覆盖了当 `run_task` 返回 `None` 时的终止路径。建议再添加一个成功路径测试，让 `run_task` 返回一个假的对象，其 `status.failed == False`，并断言 `override_next` 没有被触发（或者仍然使用正常的下一个节点），同时验证成功分支按预期工作。
建议实现如下：
```python
def test_select_times_aborts_when_subtask_fails(monkeypatch: pytest.MonkeyPatch) -> None:
context = _ActionContext()
monkeypatch.setattr(_TargetCountState, "current_times", 3)
TargetCountSelectTimes().run(context, None) # type: ignore[arg-type]
assert context.override == ("TargetCountSelectTimes", ["TargetCountAbort"])
def test_select_times_succeeds_when_subtask_succeeds(monkeypatch: pytest.MonkeyPatch) -> None:
"""
Ensure that TargetCountSelectTimes.run follows the success path when the subtask
completes successfully and does not trigger the abort override.
"""
context = _ActionContext()
monkeypatch.setattr(_TargetCountState, "current_times", 3)
class _FakeStatus:
failed = False
class _FakeResult:
status = _FakeStatus()
fake_result = _FakeResult()
# Make run_task return a successful result
def _fake_run_task(self, ctx, *args, **kwargs):
return fake_result
monkeypatch.setattr(TargetCountSelectTimes, "run_task", _fake_run_task)
TargetCountSelectTimes().run(context, None) # type: ignore[arg-type]
# On success, we should not hit the abort override
assert context.override != ("TargetCountSelectTimes", ["TargetCountAbort"])
# Depending on implementation, success may leave override unset:
assert getattr(context, "override", None) is None
```
你可能需要根据期望的“正常下一个节点”行为来细化最后一个断言。例如，如果 `TargetCountSelectTimes` 在成功路径上会显式将 `context.override` 设置为一个成功节点（如 `("TargetCountSelectTimes", ["TargetCountPage"])`），那么需要做如下更新：
1. 将最后的断言改为检查 `context.override == ("TargetCountSelectTimes", ["TargetCountPage"])`（或其他预期的成功节点）。
2. 如果 `run_task` 是定义在某个基类上而不是 `TargetCountSelectTimes` 本身，需要将 `monkeypatch.setattr` 的目标调整为该基类，以确保 `run` 方法使用的是被打补丁的 `run_task`。
</issue_to_address>
### Comment 3
<location path="tests/test_combat.py" line_range="115-120" />
<code_context>
+ assert context.override == ("TargetCountSelectTimes", ["TargetCountAbort"])
+
+
+def test_eat_candy_aborts_when_subtask_fails() -> None:
+ context = _ActionContext()
+
+ TargetCountEatCandy().run(context, None) # type: ignore[arg-type]
+
+ assert context.override == ("TargetCountEatCandy", ["TargetCountAbort"])
</code_context>
<issue_to_address>
**suggestion (testing):** 为 `TargetCountEatCandy.run` 在 `EatCandy` 子任务成功时添加对应的覆盖测试。
当前测试正确验证了失败路径（`run_task("EatCandy")` 返回 None -> `TargetCountAbort`）。为了覆盖新行为的成功路径，建议再添加一个测试：将 `run_task` 打补丁为返回一个假的 `task_detail`，其中 `status.failed == False`，并断言 `override_next` 被调用时传入 `("TargetCountEatCandy", ["TargetCountDetermine"])`。这样可以覆盖两个分支并防止成功逻辑的回归。
建议实现如下：
```python
def test_eat_candy_aborts_when_subtask_fails() -> None:
context = _ActionContext()
TargetCountEatCandy().run(context, None) # type: ignore[arg-type]
assert context.override == ("TargetCountEatCandy", ["TargetCountAbort"])
def test_eat_candy_sets_determine_when_subtask_succeeds(
monkeypatch: pytest.MonkeyPatch,
) -> None:
context = _ActionContext()
class _FakeStatus:
failed = False
class _FakeTaskDetail:
status = _FakeStatus()
def _fake_run_task(name: str) -> _FakeTaskDetail: # noqa: ARG001
return _FakeTaskDetail()
monkeypatch.setattr(context, "run_task", _fake_run_task)
TargetCountEatCandy().run(context, None) # type: ignore[arg-type]
assert context.override == ("TargetCountEatCandy", ["TargetCountDetermine"])
```
如果 `_ActionContext.run_task` 有更具体的函数签名或返回更复杂的任务详情对象，你可能需要让 `_FakeTaskDetail` 与该结构对齐（例如匹配属性名或类型）。另外，如果 `context.override_next` 并不会直接设置 `context.override`，则需要相应调整断言，改为检查合适的字段，或者使用 spy 来监控 `override_next` 的调用。
</issue_to_address>请帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据你的反馈改进后续的代码审查。
Original comment in English
Hey - I've found 3 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="agent/custom/action/combat.py" line_range="687-696" />
<code_context>
+def _tc_get_availability(context: Context) -> _TargetCountAvailability:
</code_context>
<issue_to_address>
**issue (bug_risk):** Error logging uses `{}` placeholders with positional args, which will raise a formatting error with the standard logging API.
In `_tc_get_availability`, both `logger.error` calls use `{}` placeholders with positional arguments:
```python
logger.error(
"无法识别关卡体力信息: remaining_ap={}, stage_ap={}, combat_times={}",
remaining_ap,
stage_ap,
combat_times,
)
```
With the standard `logging` API this is treated as %-style formatting, so it evaluates `"..." % (remaining_ap, stage_ap, combat_times)` and raises `TypeError` because there are no `%` placeholders. That can cause a runtime failure in the error path.
Use either an f-string:
```python
logger.error(
f"无法识别关卡体力信息: remaining_ap={remaining_ap}, stage_ap={stage_ap}, combat_times={combat_times}"
)
```
or `%s` placeholders:
```python
logger.error(
"无法识别关卡体力信息: remaining_ap=%s, stage_ap=%s, combat_times=%s",
remaining_ap,
stage_ap,
combat_times,
)
```
</issue_to_address>
### Comment 2
<location path="tests/test_combat.py" line_range="106-112" />
<code_context>
+ assert pipeline["EatCandy"]["next"] == ["EatCandyPage", "EatCandyEnter"]
+
+
+def test_select_times_aborts_when_subtask_fails(monkeypatch: pytest.MonkeyPatch) -> None:
+ context = _ActionContext()
+ monkeypatch.setattr(_TargetCountState, "current_times", 3)
+
+ TargetCountSelectTimes().run(context, None) # type: ignore[arg-type]
+
+ assert context.override == ("TargetCountSelectTimes", ["TargetCountAbort"])
+
+
</code_context>
<issue_to_address>
**suggestion (testing):** Also test the success path for `TargetCountSelectTimes.run` where the subtask completes successfully.
This test only covers the abort path where `run_task` returns `None`. Please also add a success-path test where `run_task` returns a fake object with `status.failed == False`, and assert that `override_next` is not triggered (or uses the normal next node) and that the success branch behaves as intended.
Suggested implementation:
```python
def test_select_times_aborts_when_subtask_fails(monkeypatch: pytest.MonkeyPatch) -> None:
context = _ActionContext()
monkeypatch.setattr(_TargetCountState, "current_times", 3)
TargetCountSelectTimes().run(context, None) # type: ignore[arg-type]
assert context.override == ("TargetCountSelectTimes", ["TargetCountAbort"])
def test_select_times_succeeds_when_subtask_succeeds(monkeypatch: pytest.MonkeyPatch) -> None:
"""
Ensure that TargetCountSelectTimes.run follows the success path when the subtask
completes successfully and does not trigger the abort override.
"""
context = _ActionContext()
monkeypatch.setattr(_TargetCountState, "current_times", 3)
class _FakeStatus:
failed = False
class _FakeResult:
status = _FakeStatus()
fake_result = _FakeResult()
# Make run_task return a successful result
def _fake_run_task(self, ctx, *args, **kwargs):
return fake_result
monkeypatch.setattr(TargetCountSelectTimes, "run_task", _fake_run_task)
TargetCountSelectTimes().run(context, None) # type: ignore[arg-type]
# On success, we should not hit the abort override
assert context.override != ("TargetCountSelectTimes", ["TargetCountAbort"])
# Depending on implementation, success may leave override unset:
assert getattr(context, "override", None) is None
```
You may want to refine the last assertion to match the intended “normal next” behavior. For example, if `TargetCountSelectTimes` explicitly sets `context.override` to a success-next node (e.g. `("TargetCountSelectTimes", ["TargetCountPage"])`), then update:
1. The final assertion to check `context.override == ("TargetCountSelectTimes", ["TargetCountPage"])` (or whatever the expected success-next is).
2. If `run_task` is defined on a base class instead of `TargetCountSelectTimes`, adjust the `monkeypatch.setattr` target to that base class so the `run` method uses the patched `run_task`.
</issue_to_address>
### Comment 3
<location path="tests/test_combat.py" line_range="115-120" />
<code_context>
+ assert context.override == ("TargetCountSelectTimes", ["TargetCountAbort"])
+
+
+def test_eat_candy_aborts_when_subtask_fails() -> None:
+ context = _ActionContext()
+
+ TargetCountEatCandy().run(context, None) # type: ignore[arg-type]
+
+ assert context.override == ("TargetCountEatCandy", ["TargetCountAbort"])
</code_context>
<issue_to_address>
**suggestion (testing):** Add complementary coverage for `TargetCountEatCandy.run` when the `EatCandy` subtask succeeds.
This test correctly verifies the failure path (None from `run_task("EatCandy")` -> `TargetCountAbort`). To also cover the success path of the new behavior, please add a test that patches `run_task` to return a fake `task_detail` with `status.failed == False`, and assert that `override_next` is called with `("TargetCountEatCandy", ["TargetCountDetermine"])`. This will exercise both branches and guard against regressions in the success logic.
Suggested implementation:
```python
def test_eat_candy_aborts_when_subtask_fails() -> None:
context = _ActionContext()
TargetCountEatCandy().run(context, None) # type: ignore[arg-type]
assert context.override == ("TargetCountEatCandy", ["TargetCountAbort"])
def test_eat_candy_sets_determine_when_subtask_succeeds(
monkeypatch: pytest.MonkeyPatch,
) -> None:
context = _ActionContext()
class _FakeStatus:
failed = False
class _FakeTaskDetail:
status = _FakeStatus()
def _fake_run_task(name: str) -> _FakeTaskDetail: # noqa: ARG001
return _FakeTaskDetail()
monkeypatch.setattr(context, "run_task", _fake_run_task)
TargetCountEatCandy().run(context, None) # type: ignore[arg-type]
assert context.override == ("TargetCountEatCandy", ["TargetCountDetermine"])
```
If `_ActionContext.run_task` has a more specific signature or returns a richer task detail object, you may want to align `_FakeTaskDetail` with that structure (e.g., by matching attribute names or types). Also, if `context.override_next` does not set `context.override` directly, you should adjust the assertion to check the appropriate field or use a spy on `override_next` instead.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@agent/custom/action/combat.py`:
- Around line 939-953: 在处理可用次数的流程中检查 availability.page 是否为
_TargetCountPage.UNKNOWN，不要将未知页面映射为一次可复现次数；应立即停止流程或转入明确的恢复路径，避免继续执行
SSCannotReplay、SSNoReplay 或次数设置。补体后的重新识别也必须执行相同的未知页面处理，并为该场景添加回归测试。
In `@resource/base/pipeline/eat_candy.json`:
- Line 4: 使用 Prettier 格式化 eat_candy.json，将整个 JSON 文件统一为两空格缩进，并保留现有内容与结构不变。
In `@tests/test_combat.py`:
- Line 67: Rename the affected pytest functions in tests/test_combat.py to
follow the test_<module>_<behaviour> convention by adding the test_combat_
prefix, including
test_calculate_available_count_distinguishes_zero_and_invalid_values and the
other referenced tests, without changing their test behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 930bc58f-6371-4f49-a463-80a541494e60
📒 Files selected for processing (3)
agent/custom/action/combat.pyresource/base/pipeline/eat_candy.jsontests/test_combat.py
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@agent/custom/action/combat.py`:
- Around line 949-958: Update the candy-restoration loop around EatCandy to
capture and validate each task result, stopping the task and returning failure
when the result is missing or unsuccessful. After rechecking availability,
immediately break when available_count is greater than zero so no extra candy is
consumed; add regression tests covering first-attempt success and
restoration-task failure.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2dd7ac1c-88c1-47af-b84b-62f487be125c
📒 Files selected for processing (2)
agent/custom/action/combat.pytests/test_combat.py
Windsland52
added a commit
that referenced
this pull request
Aug 12, 2026
吃糖选项关闭时 EatCandy 节点会被禁用，run_task 立即失败并被 #884 的新逻辑转为 TargetCountAbort，导致常规作战、智能均衡刷材料、活动代币刷取和复刻活动推图在体力不足时误报任务失败。改为在决定补体前检测 EatCandy 是否被禁用，禁用时直接走 TargetCountFinish / HomeButton 正常结束，不再中断后续队列任务。
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
关联 Issue / Related Issue
暂无公开 Issue。需求来自近期 Sentry
TargetCountStartReplay/recognition失败事件：一轮复现恰好耗尽体力后，游戏自动打开“活性恢复”页，原逻辑却把关卡体力 OCR 缺失解释为可复现次数999，继续进入开始复现节点。Sentry: https://m9a.sentry.io/issues/7657794666/
变更摘要 / Summary
-1/999哨兵值。EatCandy仍兼容从关卡页点击体力入口。SetReplaysTimes和EatCandy子任务结果，失败时转入TargetCountAbort，避免继续执行错误节点。验证 / Validation
pnpm check：通过（Prettier、schema、全部 MaaFW controller/resource 完整性检查）。pnpm check:py：通过（ruff、pyright strict、80 个 pytest 测试）。影响范围 / Impact
AllIn/ 目标次数刷图流程。TargetCountDetermine、TargetCountSelectTimes、TargetCountEatCandy自定义动作。SSReopenReplay。resource/base/pipeline/eat_candy.json的入口候选顺序。截图 / 日志 / 说明 / Screenshots / Logs / Notes
复现流程：
RecognizeStageAp，将缺失值转换成0，再将stage_ap == 0转换成可用次数999。TargetCountStartReplay，但当前页面不存在“复现”按钮，最终识别失败。失败截图及对应 GUI/Python 日志保存在上述 Sentry 事件附件中。相关 pipeline：
resource/base/pipeline/all_in.json、resource/base/pipeline/eat_candy.json。检查清单 / Checklist
CONTRIBUTING.md/ I have read and followedCONTRIBUTING.md.Summary by Sourcery
在目标次数战斗流程中，处理会自动打开恢复页面的阶段，并确保在体力相关子任务失败时中止整条流水线，而不是继续执行。
Bug Fixes:
Enhancements:
Tests:
Original summary in English
Summary by Sourcery
Handle stages that auto-open the recovery page in target-count combat flows and ensure failures in stamina-related subtasks abort the pipeline instead of continuing.
Bug Fixes:
Enhancements:
Tests:
Summary by CodeRabbit
新功能
问题修复