Repository navigation
test(architecture): close the two coverage gaps review found in the owner censuses - #8055
Conversation
…wner censuses
The chat action-kind census visited only ast.Assign, so a reintroduced copy
annotated as LOCAL: set[str] = {...} passed the guard that exists to catch a
second owner. It now reads annotated module-level assignments too, and a
parametrised case proves a typed, renamed or reordered copy is recognised.
The pending-reason guard checked that the import exists and that the value still
compares equal. Rebinding the imported name below the import with an equal
computed string keeps both checks green and still installs a second owner, and an
identity assertion cannot see it either because CPython folds and interns such a
string. The guard now rejects any later module-level assignment to the imported
name, whatever shape that value has, with the counterexample pinned as a case.
Signed-off-by: keoyyo85 <335569976+keoyyo85@users.noreply.github.com>
loopx-agent
left a comment
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.
动机
没有阻塞性 finding；有一项 P3 覆盖说明建议。head 434c1354285b327b5c8f5b1b0a850e6c4b85a881，基线 ce4cfc1ce74a2ffe0d685758b8d6e9233b467f41。
- 受影响调用者：维护 Chat action-kind 与 progress-review pending-reason 单一 owner 约束的贡献者和评审者。
- 前后场景：相同的带类型十四成员集合副本、保留 import 的等值计算字符串重绑定，基线 census 均漏检；head 的原有真实检查分别拒绝这两个直接模块体反例。
- 可观察结果：补上原两条 P3 评论指出的直接绑定覆盖缺口，保留现有生产 owner 和消费者行为；条件块覆盖仍作为非阻塞建议披露。
- 非目标：不修改生产 vocabulary、运行策略或权限，不将直接模块体扫描等同完整 Python 名称绑定分析，不声称 CI 或长期收益已验证。
先读取了原评审依据：discussion_r4436543786，版本 1d31f0cb9b82d2d96ca193030b1b415909c03153；以及 discussion_r4436544120，版本 d8d81476212af75b5f878b578f42181e21a941f6。两条 P3 都允许修复相应直接反例或收窄覆盖声明；本次不把它们扩写成完整 Python 分析器的义务。
改动思路
在两个原 census 内补上对应输入形状，保留真实 owner/consumer 检查。第一个合并直接 Assign/AnnAssign 的读取；第二个同时核对 owner import、局部名字重新赋值和实际值。等值甚至同一对象都不能证明 owner 没被重新定义：CPython 可以折叠并 intern "pending_" + "evaluation"。
不用修改运行时，也无需新建共享 scanner 框架。其他架构测试虽有局部 AST helper，其资源与 scope 不同；这里复用原 guard 是较小且可独立回滚的边界。
具体改动
关键代码讲解
module_level_bindings/vocabulary_assignments读取直接模块体赋值，按既有KIND_VALUE判断集合副本；声明但未赋值跳过，原路径枚举继续用as_posix()。owner_import_lines/module_assignment_lines分别记录 import 与直接赋值名字；消费者断言保留 owner 导入及值相等，再拒绝该名字重新赋值。新 case 保留了等值计算字符串这个反例。
独立执行结果：四个原生模块（两 census＋两个消费者 suite）基线 80 passed、head 84 passed；单独两 census 分别22/26 passed，Ruff 通过。相同的两处源码输入反例注入后，基线仍22 passed，head为24 passed、两个预期失败，分别落在实际重复 owner 检查。源码输入通过读取边界追加，实际 AST/guard/结果未被替换，tracked 文件未修改。另验证 annotation-only、函数局部、owner 引用和部分集合等七个合法控制未被误拒绝。
原生 quick premerge 5 direct＋2 selected 全过，含 maintainability 与 semantic-vocabulary；精确 scope CQR通过，含下面的非阻塞 advisory。一个提交的 DCO trailer 已核对。
对主干的风险
P3：收窄“module-level”覆盖说明。 这两个 helper 只遍历 ast.parse(source).body。在 owner import 后加入 if True: PROGRESS_REVIEW_PENDING_REASON = "pending_" + "evaluation"，同时把带类型集合副本放入 if True，head 的26项仍全部通过；这是模块作用域绑定，但不是直接模块体赋值。建议把说明明确写成“直接模块体绑定”，或有界遍历模块控制块并排除函数/类局部。原两条评论的直接反例确已修复，此项不阻塞；无需因此引入通用静态分析器。
生产代码、默认值、权限与用户操作路径未改变。检查的 source overlay 是合成反例，不能外推为完整 Python 名称绑定覆盖；原生 Windows/Linux 和远端 CI 未验证，未获取、轮询或等待 CI。当前完整队列中 keoyyo85 只有此 PR，不从这一次修复推断贡献者重复低价值工作。
我的整体评价
同意这个完整小修复，保留上面的 P3。它让两条已经证实漏检的输入真正触发原 guard，同时保持消费者和合法输入通过；新增用例数本身不是价值证明。
视觉评审不适用：改动仅为 tests/architecture/test_chat_action_kind_owner.py、tests/architecture/test_progress_review_pending_reason_owner.py 的测试代码与说明，没有 rendered presentation 变化。未验证范围：安装后 Host/model 行为、原生 Windows/Linux、整页视觉和长期效果。实际产品入口和额外必需交互轮数保持不变。APPROVE 不授予合并权限。
English verdict: APPROVE — the two direct-binding escapes are detected at 434c1354285b327b5c8f5b1b0a850e6c4b85a881. One non-blocking P3 clarifies conditional/module-body coverage; CI and platform qualification remain unverified.
| def module_assignment_lines(source: str) -> dict[str, int]: | ||
| """Module-level names assigned a value, whatever shape that value has.""" | ||
| assignments: dict[str, int] = {} | ||
| for statement in ast.parse(source).body: |
There was a problem hiding this comment.
[P3] 建议把说明限定为直接模块体绑定，或覆盖模块控制块。这里仅遍历 Module.body；在 owner import 后追加 if True: PROGRESS_REVIEW_PENDING_REASON = "pending_" + "evaluation"，并将带类型集合副本同样包在条件块内，当前26项仍全过。原两条直接绑定反例已经修复，此项不阻塞；若保留“任何后续 module-level assignment”的声明，建议补此反例并排除函数/类局部作用域。
Goal And Delivered Outcome
Outcome basis / optional anchor: Review feedback on two merged changes — the
P3comment on refactor(chat): read the action-kind vocabulary from the store that owns it #7825 (tests/architecture/test_chat_action_kind_owner.py:67) and theP3comment on refactor(progress-review): read the pending reason from the policy that owns it #7826 (tests/architecture/test_progress_review_pending_reason_owner.py:93). No issue is filed for either and none is invented here.Goal/source and gap: Both files are censuses whose whole purpose is to fail when a second owner of one decision reappears. Each had a way for a copy to reappear while the guard stayed green.
ast.Assign, so a copy written asLOCAL: set[str] = {...}— the same fourteen members, annotated — passed."pending_" + "evaluation") keeps the import, keeps the equality, and is invisible to a literal-keyed scan; an identity assertion does not close it either, because CPython folds that concatenation and interns the result.Observable before → after: with both counterexamples planted in the same tree, the guards on
mainreport 22 passed and see nothing; the guards here report 2 failed and name the second owner. On the clean tree the guards here are 26 passed against 22 onmain, the four extra cases being the ones that pin this behaviour. Proved by theregression_parityrows below.Issue/task and intended base: Related to refactor(chat): read the action-kind vocabulary from the store that owns it #7825 and refactor(progress-review): read the pending reason from the policy that owns it #7826 (both merged). Base
mainatce4cfc1ce.Author Declaration
Implemented against
Specification and revision: the two
P3review comments quoted above, each of which states both the counterexample and the accepted remedy ("handleast.AnnAssign, or narrow the scanner's coverage claim" / "detect a later assignment to the imported name, or narrow the no-computed-copy claim").Criteria:
module_level_bindingsandvocabulary_assignmentsintests/architecture/test_chat_action_kind_owner.pypytest tests/architecture/test_chat_action_kind_owner.py -k reintroduced_copyowner_import_linesandmodule_assignment_linesintests/architecture/test_progress_review_pending_reason_owner.pypytest tests/architecture/test_progress_review_pending_reason_owner.py -k "never_rebinds_it or flags_a_rebind"pytest tests/architecture/test_progress_review_pending_reason_owner.py::test_the_census_flags_a_rebind_that_keeps_the_import_and_the_valueScope And Continuation
str(relative_to(...))comparison that misfires on native Windows, and an over-stated comprehension-coverage claim in its header); they are the same class of gap in a different contributor's open change and are not edited here.Validation
434c13542staticruff checkon both changed guard files under the repository configuration: All checks passed.unitpytest tests/architecture/test_chat_action_kind_owner.py tests/architecture/test_progress_review_pending_reason_owner.py tests/capabilities/test_progress_review.py tests/control_plane/test_external_progress_review.py→ 84 passed at this revision, including both folded vocabularies' consumer suites.regression_parityloopx/chat_actions.py, and an equal computed rebind below the import inloopx/capabilities/progress_review/receipt.py) were planted in both trees. The guards onmainreported 22 passed, seeing neither copy; the guards here reported 2 failed, naming both.regression_parityunitFrontend / Visual Evidence
Type of Change
LoopX Area
Technical Direction
Shared-authority RFC fixture impact
Boundary Checklist
.loopx/,.codex/goals/, and liveACTIVE_GOAL_STATE.md).python3 scripts/check_rfc_bilingual.py --base origin/main --head HEADpasses, or no paired RFC changed.none.Signed-off-bytrailer (git commit -s).