Repository navigation
control_plane/testing: single-source the actor-result field set - #8629
2430468136 wants to merge 2 commits into
Conversation
onboarding_model_behavior_qualification restated _ACTOR_RESULT_FIELDS as _RESULT_FIELDS while already importing three private helpers from model_behavior_qualification -- the strongest form of the duplicate-knowledge case. It now borrows the owner object and the restated name is retired. The by-name semantic inventory cannot see this pair: _RESULT_FIELDS is bound to different value sets in two other modules, so by name it is a fork, not a twin. The new guard keys on the folded value instead -- a whole-tree census, identity, retirement, and behavioural parity through both normalizers. Refs loopx-project#4447. Signed-off-by: psychologister <230141834+psychologister@users.noreply.github.com>
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
loopx-agent
left a comment
There was a problem hiding this comment.
Reviewer: model_agent · gpt-6.1-sol · OpenAI · runtime_reported · reasoning_effort=max
本次评审对应提交 c42354a,运行时对照当前主干 9ce3a46。
[P2] 全包扫描误拒绝独立的展示列。 在隔离源码夹具加入只返回列顺序的 preview_columns(),依次输出 actor_ref、decision、tool_calls、schema_version。新增测试把这条展示数据列为另一处字段定义并退出 1。 展示函数返回指定列顺序。原模型资格 arm 和 onboarding 的有效回执及拒绝与主干完全一致。 将强制的所有权断言收在两个实际校验入口,沿用现有对象身份和输入回归。其余同值文本交由现有 advisory 供审阅。 tree_offenders · 词表收敛规范
动机 / Motivation
两个实际校验入口维护相同的四个键,onboarding 已依赖现有模型资格模块。 受影响的调用方包括用合成 actor 运行模型资格 arm 和 onboarding 闭环的调用方。两个入口读取同一个字段集合,继续使用各自原 schema 和空工具调用约束。 两条真实 Python 入口的八个用例全输出一致。有效回执、未知键排序、错误 schema 和非空工具调用均覆盖;非法 onboarding 输入没有进入 transition。 实测源码路径与隔离合成输入。未执行真实模型、安装态和托管 CI。
规格依据为当前主干的 docs/architecture/rfcs/semantic-vocabulary-convergence-v0.md。核验 track_a_callers_preserve_behavior 与 same_value_candidates_advisory 两项。收敛 RFC 规定相同值的候选只供审阅。复用要求语义槽、作用域和合同相同。 调用合同 · 词表收敛规范
改动思路 / Approach
运行时重用现有定义。pytest 另对整个 Python 包执行新增源码规则。 对已知消费路径检查实际引用,再通过入口测试验证原行为。新增扫描将同值匹配用于整个 Python 包,独立数据也会触发失败。
具体改动 / Changes
-
四个结果键只由模型行为校验的共同定义决定,两个入口分别负责各自 schema 和执行门。 _ACTOR_RESULT_FIELDS
-
两个入口读取同一个字段集合,继续使用各自原 schema 和空工具调用约束。 normalize_onboarding_model_behavior_actor_result
-
两条真实 Python 入口的八个用例全输出一致。有效回执、未知键排序、错误 schema 和非空工具调用均覆盖;非法 onboarding 输入没有进入 transition。 对原新增测试分别运行空白夹具、独立合法数据和实际消费者重述三个版本。无注入通过,两类注入均被拒绝。 tree_offenders
对主干的风险 / Risks
主干 210 项、候选 227 项通过。Ruff、diff 检查、DCO 和与当前 main 的合入模拟通过。 运行时配对包括合法输入和原有拒绝。源码反例只新增独立数据,原校验分支和调用结果均保留。新增的测试失败会影响后续与这两个校验入口无关的变更。 真实 onboarding 入口在三类非法返回时均停在 transition 之前。
我的整体评价 / Verdict
请求修改。将强制的所有权断言收在两个实际校验入口,沿用现有对象身份和输入回归。其余同值文本交由现有 advisory 供审阅。 保留一个能区分实际消费者重复定义与合法独立数据的回归用例。
English verdict: REQUEST_CHANGES. Both qualification entry points preserved their runtime output. The package-wide pytest guard rejects an independent ordered presentation tuple. Limit the enforcing test to actual schema owners and consumers.
| @pytest.fixture(scope="module") | ||
| def tree_offenders() -> dict[str, list[int]]: | ||
| offenders: dict[str, list[int]] = {} | ||
| for path in sorted(PACKAGE_ROOT.rglob("*.py")): |
There was a problem hiding this comment.
[P2] 在隔离源码夹具加入只返回列顺序的 preview_columns(),依次输出 actor_ref、decision、tool_calls、schema_version。新增测试把这条展示数据列为另一处字段定义并退出 1。 将强制的所有权断言收在两个实际校验入口,沿用现有对象身份和输入回归。其余同值文本交由现有 advisory 供审阅。
…ints The loopx-agent review flagged the whole-package folded-value census as over-broad: it treated any independent same-value data as a restatement, including an ordered preview_columns() presentation tuple that merely happens to share the four members. Reuse requires the same semantic slot, scope and contract, not merely equal members. Narrow the enforcing assertion to the two actual validation entry points: the owner states the set exactly once, the consumer borrows it and states it nowhere. Demote the whole-package same-value scan to advisory (RFC semantic-vocabulary-convergence-v0 same_value_candidates_advisory) so independent data is reported for review, never an enforcing failure. Object identity and normalizer behavioural parity are unchanged, so a migration that silently altered the accepted keys still fails here. Add a tmp_path regression that distinguishes a real consumer restatement (bitten) from a legitimate independent presentation tuple (advisory only). Test-only change; production owner and consumer are untouched. Signed-off-by: psychologister <230141834+psychologister@users.noreply.github.com>
What
onboarding_model_behavior_qualification.pyrestated the actor-result field set as_RESULT_FIELDS = {"schema_version", "actor_ref", "decision", "tool_calls"}while already importing three private helpers frommodel_behavior_qualification.py, the owner of_ACTOR_RESULT_FIELDS. It now borrows the owner object; the restated name is retired.Why
The strongest form of the repository duplicate-knowledge case (#4447): a module restating a constant whose owner it already imports from, so the migration adds no new import edge and cannot cycle.
The by-name semantic inventory cannot see the pair:
_RESULT_FIELDSis also bound to different value sets inissue_fix/repository_memory.pyandextensions/capability_admission.py, so by name it is a fork, not a twin, and the equal value set shared with_ACTOR_RESULT_FIELDSis invisible to a name-keyed scan.Guard
New
tests/architecture/test_model_behavior_actor_result_field_owner.pykeys on the folded value: a whole-tree census (only the owner states the set), identity, retirement of the restated name, and behavioural parity through both normalizers (same unknown-field rejection, same accepted keys). The census is poison-proven across set / frozenset / list / tuple / renamed / nested forms.Tests
Architecture guard + model-behavior qualification focused tests pass locally; net regression 0.