Skip to content

fix(explore): reject non-finite replay weights and score floors - #7574

Open
gcl-coder wants to merge 2 commits into
loopx-project:mainfrom
gcl-coder:fix/adaptive-replay-numeric-inputs
Open

gcl-coder wants to merge 2 commits into
loopx-project:mainfrom
gcl-coder:fix/adaptive-replay-numeric-inputs

Conversation

@gcl-coder

Copy link
Copy Markdown
Contributor

What this fixes

loopx/capabilities/explore/adaptive_replay_planner.py now validates every public numeric input, not only the two cost fields.

ReplayPositionCost.capture_minutes / restore_minutes were hardened in #6094 (_nonnegative rejects booleans, non-finite values and unrepresentable integers). Two sibling inputs on the same path were left comparison-only, and a NaN there does not raise — it silently disables a bound:

input before after
FailureReplayObservation.weight = NaN accepted; the plan admitted a proposal with score_minutes=nan, expected_suffix_events=nan, eligible_observation_weight=nan, and minimum_score_minutes=100.0 did not reject it (nan < 100.0 is false) ValueError: weight must be a finite positive number
plan_adaptive_replay_points(..., minimum_score_minutes=NaN) the floor stopped rejecting: a candidate whose score_minutes is -99.0 with expected_saved_minutes 0.0 was still proposed ValueError: minimum_score_minutes must be a finite non-negative number
weight = 10**400 raw OverflowError: int too large to convert to float escaped validation field-specific ValueError
max_proposals = nan / inf / True OverflowError from int(nan) / int(inf) / int(True) path, none field-specific ValueError: max_proposals must be a finite positive number

The NaN proposal also reached proposals.sort(key=lambda proposal: -proposal.score_minutes, ...), where comparisons against NaN make the returned order unspecified.

Measured on origin/main 98c4ba315 before the change:

A) obs.weight accepted = nan
   proposal score: nan | suffix: nan | weight: nan
   count = 1 (floor was 100.0)
B) NaN floor proposals: [(-99.0, 0.0)]
C) huge weight -> OverflowError int too large to convert to float

Shape of the change

One owner for the rule instead of three comparison-only checks, matching the existing _finite_float / _positive_number helpers in loopx/capabilities/reward_memory/utility_reducer.py:456 and loopx/configure_goal.py:186:

  • _finite_number(value, field_name, *, allow_zero) holds the boolean, overflow, finiteness and range checks once.
  • _nonnegative (existing name, identical message) and _positive (new) are the two thin callers, so capture_minutes / restore_minutes keep their exact existing error text.
  • weight, minimum_score_minutes and max_proposals are routed through them.

Valid inputs are unchanged: int(max_proposals) semantics are preserved (2.9 -> 2), only values that previously raised the wrong error type, or did not raise at all, now raise the field-specific ValueError.

Tests

tests/test_explore_replay_runtime.py gains three parametrized cases next to the existing test_replay_cost_rejects_non_finite_or_non_numeric_minutes: 19 new cases in total. 17 of them fail on the unpatched file — the two that pass already (max_proposals=0/-1) are kept as regression guards.

$ python3 -m pytest -q tests/test_explore_replay_runtime.py
55 passed in 0.26s          # 36 before this change

$ git checkout origin/main -- loopx/capabilities/explore/adaptive_replay_planner.py
$ python3 -m pytest -q tests/test_explore_replay_runtime.py
17 failed, 38 passed        # the new cases are not tautological

ruff check on both files reports the same three pre-existing diagnostics as the unpatched tree (UP035 × 2, TRY004 × 1) and nothing new.

No registry, budget, generated-artifact or .github change: multi_value_twins, the inventory ratchets and the drift smoke are untouched by this diff.

Refs #6094, does not close it: the cost-field half landed as db1b23992 ("fix(explore): reject non-finite replay costs") and is guarded by test_replay_cost_rejects_non_finite_or_non_numeric_minutes. The report's stated expectation — reject NaN, infinities, booleans and unconvertible values with a field-specific ValueError — is still unmet for weight, minimum_score_minutes and max_proposals, which is what this diff changes.

The adaptive replay planner validated its two cost fields but not the other
public numeric inputs. A NaN weight was accepted and reached a proposal whose
score, suffix-event estimate and eligible weight were all NaN, and every
`minimum_score_minutes` comparison against NaN is false, so the floor stopped
rejecting anything - including candidates that score below zero. A NaN or
infinite score floor had the same effect on its own, and an out-of-range
integer weight leaked a raw OverflowError instead of a field-specific error.

Hold the rule in one helper (`_finite_number`) with two thin callers, so the
cost fields keep their existing message and weight, score floor and proposal
limit share the same boolean, overflow, finiteness and range checks. Valid
inputs and `int(max_proposals)` semantics are unchanged.

Signed-off-by: ChengLong GU <88359731+gcl-coder@users.noreply.github.com>
@mergify

mergify Bot commented Oct 28, 2026

Copy link
Copy Markdown

This pull request has merge conflicts with main and cannot be merged
until they are resolved. Please rebase or merge the base branch, @gcl-coder.

Choose the remote for the base repository, not an out-of-date fork.
For a fork clone, first inspect git remote -v; upstream must point
to https://github.com/loopx-project/loopx.git. If it is absent, add it
with git remote add upstream https://github.com/loopx-project/loopx.git.
Then run:

git fetch upstream
git rebase upstream/main
# Resolve each conflict, git add the resolved files, then git rebase --continue.
git push --force-with-lease origin HEAD

For a same-repository clone whose origin points to
https://github.com/loopx-project/loopx.git, use origin instead of
upstream for fetch/rebase. If you prefer merging the base, use
git merge <base-remote>/main and push normally.

Keep the DCO Signed-off-by trailer on every commit when you rebase.
https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify Bot added the needs-rebase Mergify: the pull request has merge conflicts with its base branch label Oct 28, 2026
@gcl-coder
gcl-coder force-pushed the fix/adaptive-replay-numeric-inputs branch from f34a3d3 to b8fd8c9 Compare October 28, 2026 13:53
@mergify mergify Bot removed the needs-rebase Mergify: the pull request has merge conflicts with its base branch label Oct 28, 2026

@loopx-agent loopx-agent left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewer: model_agent · gpt-6.1-sol · OpenAI · runtime_reported · reasoning_effort=max

请求修改:当前完整 head c91e1dcfcb87913539d989fdbca7542fc497ff75 有两项P2输入契约回归。非有限权重的修复成立,仍需保留原来的有效下界与整数上限。

动机 / Motivation

调用 ReplayPoint 规划 API 的开发者需要可信的候选分数、数量上限与可调整的分数下界。 同一条十事件轨迹的合法结果仍为7和1.75分,NaN 权重现在被拒绝;但 max_proposals=0.5 从报错变成静默空结果,有限下界-1从可用变成报错。 NaN 防护有实际增量,但两项输入契约回归尚未修复,当前完整提案还不能批准。 不捕获或恢复环境,不启用自动回放、不新增模型或界面,也不将纯算术修复当作长期收益。

NaN 会使比较失去意义,修复值得交付;但“有限”与“非负”不是同一个条件,外层数值为正也不能证明截断后的整数仍为正。

改动思路 / Approach

本地有限转换可以复用,但成本、权重、分数下界和整数上限是不同的数值约束。 本提案应保留有限性修复,同时恢复有符号下界及转换后正整数上限,再进入复审。 继续在现有 Python 算术 owner 内检查是合适的:没有状态、provider IO或新执行器,无需强行迁移控制面。_nonnegative保持原成本错误,_positive补权重检查,有可验证增量;需要修的是两个调用点的范围选择,不能靠继续加通用层解决。

具体改动 / Changes

完整两路径 +78/-15,生产 +29/-15、测试49行。先读规格 docs/architecture/rfcs/optional-semantic-assistance-jev-v0.md#4-existing-system-and-ownership,修订 ffc90182bc2a896aa9980cf24dcf23a0e3276e3d;Deferred replay placement 行要求保留现有 measured/adapter-estimated 算术 owner,当前归属符合。该RFC没有授权把分数下界限制为非负;#6094的非负要求针对回放成本,其已合入修复不能扩大为所有参数的范围。

关键代码讲解

  • _finite_number 拒绝布尔、非有限值与转换溢出,将错误绑定到字段;两成本字段的六组旧错误与基线逐字相等。
  • FailureReplayObservation.__post_init__ 通过正有限检查拒绝NaN权重。实际NaN旧版进入提案并污染分数,当前 ValueError;这是有效修复。
  • plan_adaptive_replay_points 仍按原位置、fidelity、加权节省减成本排序,但185/186两处验证改变了合法性:

[P2] 在整数转换后保留正数上限检查(185行)。max_proposals=0.5 在不可变基线经 int() 成0后抛 ValueError;当前 _positive(0.5) 通过,再截断成0,返回空tuple。调用方看见“没有候选”,输入错误被隐藏。先做安全转换,再验证最终 proposal_limit>0;补0.5反例,并保持作者明确要保留的 2.9→2 行为。

[P2] 保留有限的有符号分数下界(186行)。minimum_score_minutes=-1 原来是合法过滤阈值:独立例子中保存1分钟、捕获成本2分钟,分数-1,旧版正常返回该诊断候选;当前 _nonnegative 在计算前拒绝。成本必须非负不等于“节省减成本”的过滤下界必须非负。对下界只验证有限性,保留正/零/负对照;若确实要改变范围,需要独立的契约依据与披露,不能称有效输入不变。

对主干的风险 / Risks

我独立运行84项回放、counterfactual、compatibility与episode测试,全部PASS;但新增用例漏掉0到1之间的上限,还把负下界冻结成非法,因此绿测未证明兼容。同一十事件轨迹的base/head各34次观察,合法算式/排序为7与1.75、六旧成本错误完整相等;15组结果全等,两个P2由相同输入直接复现。NaN权重旧失败/当前拒绝的正向证据保留,未因新问题否定该增量。

Ruff零问题、focused mypy通过,标准premerge 6直接+5选中均PASS/无hold;严格CQR正确记录FAIL及两个具体阻断。Merge commit无普通sign-off,但它的exact SHA/两parent/web-flow/valid签名已按仓库例外核实,不构造DCO问题。未查远端CI、未装App或运行模型;当前是纯API数值边界,修复无需增加UI或授权问询。

语义与 CI 对齐

复用原词汇/ReplayFidelity,没有新协议;已发生的范围变化仍须保留原 API 意义。有限性、非负性和int转换后的正值要分别验证。另一个开放提案#7563触及同一owner,包含有限聚合和保留负下界的不同实现;维护者应协调,不能把它的批准或旧head测试当成本PR证据。本评审没有把旧的聚合溢出另造成本PR新增回归。

我的整体评价 / Verdict

完整交付判断为not_yet_proven:修复NaN有价值、当前归属和机制规模合理,但两个真实API回归仍使long_horizon和user_experience为regression,需先修复再重审。检查了回放规划 API、全部调用点及两路径差异;没有渲染页面或交互控件变化,截图不适用。 未验证范围为正式App、原生Windows和长期实际回放/模型净收益;这些不能由纯函数测试认证。相关记忆面向恢复RFC分阶段交付,本次仅采用核对真实调用结果的原则,不把尚未实施的研究门当作修复前置。

English verdict: REQUEST_CHANGES — c91e1dcfcb87913539d989fdbca7542fc497ff75. NaN input rejection works, but a subunit max_proposals now silently yields an empty plan and finite negative score floors are newly rejected. 84 tests and native premerge pass; neither covers these two API regressions. Preserve signed finite floors and validate the final integer limit.

if int(max_proposals) <= 0:
raise ValueError("max_proposals must be positive")
threshold = float(minimum_score_minutes)
proposal_limit = int(_positive(max_proposals, "max_proposals"))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[P2] 在int转换后验证上限仍为正。相同合法trace/candidates/observations,max_proposals=0.5在不可变base ffc90182中抛ValueError;当前_positive(0.5)通过后int成0,静默返回空tuple,输入错误被伪装成没有候选。请在最终proposal_limit上保留>0检查,并补0.5以及声明要兼容的2.9→2对照。

raise ValueError("max_proposals must be positive")
threshold = float(minimum_score_minutes)
proposal_limit = int(_positive(max_proposals, "max_proposals"))
threshold = _nonnegative(minimum_score_minutes, "minimum_score_minutes")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[P2] 对分数下界验证有限性,保留其符号。minimum_score_minutes=-1原来合法:保存1分钟减捕获2分钟的候选score=-1可用于诊断,base会返回;当前_nonnegative直接ValueError。成本非负不等于过滤下界非负,#6094的要求针对cost。请保留有限负下界并补正/零/负对照;不能将此范围收窄称作valid inputs unchanged。

@mergify

mergify Bot commented Oct 28, 2026

Copy link
Copy Markdown

This pull request has merge conflicts with main and cannot be merged
until they are resolved. Please rebase or merge the base branch, @gcl-coder.

Choose the remote for the base repository, not an out-of-date fork.
For a fork clone, first inspect git remote -v; upstream must point
to https://github.com/loopx-project/loopx.git. If it is absent, add it
with git remote add upstream https://github.com/loopx-project/loopx.git.
Then run:

git fetch upstream
git rebase upstream/main
# Resolve each conflict, git add the resolved files, then git rebase --continue.
git push --force-with-lease origin HEAD

For a same-repository clone whose origin points to
https://github.com/loopx-project/loopx.git, use origin instead of
upstream for fetch/rebase. If you prefer merging the base, use
git merge <base-remote>/main and push normally.

Keep the DCO Signed-off-by trailer on every commit when you rebase.
https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify Bot added the needs-rebase Mergify: the pull request has merge conflicts with its base branch label Oct 28, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-rebase Mergify: the pull request has merge conflicts with its base branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants