fix(semantics): close the three non-blocking findings from the B2 review - #4682
Conversation
…ker taxonomy The B2 residue in loopx-project#4447 was recorded as 34 sites; measured on this tree it is 41, spread across every scanned vocabulary rather than effective_action alone. This deepens the bounded producer scan by three recognized forms, each with a negative twin, and leaves every site it cannot bind unresolved with its reason. Same-module call results: a call to an undecorated, non-generator, plainly defined top-level def of the same module resolves to the union of that function's own returns. Arguments are never bound to parameters, so a returned parameter stays unknown and the answer does not depend on the call site. Ordered rebinding: a local written more than once resolves to the union of the writes that textually precede the read, and only when every store of that name is a plain name = expression. Key-precise container writes: a container mutated only through direct literal-key subscript writes keeps its untouched keys; a written key carries the union of its initializer and every write. A ** spread of statically known dict literals is flattened so an optional spread no longer hides a sibling key. The TypeScript parser gains the two sound forms the Python scanner already had and reports the same blocker vocabulary, so one residue taxonomy covers both runtimes instead of a single typescript_dynamic catch-all. An owner-member result now carries its reason too. Unresolved sites 41 -> 40; unresolved rows carrying at least one known value 2 -> 7. Registry values, budgets and the producer site list are unchanged. The producer scan is net faster (9.20s -> 7.17s over the 319 files it reaches) because it now reuses the memoised parse and module-function table. Refs loopx-project#4447 (B2) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: song <22676124+songoow@users.noreply.github.com>
…er write The multi-write form resolved a local to the writes that textually precede the read. Textual position is execution order only where no back edge crosses it, and the filter did not look for one. A local written at the bottom of a loop body and read at the top resolved to the value written before the loop, and the site reported `unresolved=False` with no blocker. That is the one failure mode that turns an unknown into wrong evidence instead of into a smaller residue. F1 asks whether a producer writes only registered values; a producer that emits an unregistered value on every iteration after the first passed it, because the scan had reported a closed value set that was not closed. Four shapes reproduce it: a `for` back edge, a `while` back edge, a write carried by an outer loop, and a `finally` that rebinds. A negative subscript store had the same shape. `table[-1]` names the same slot as some non-negative index whose number depends on the container's length, so recording it under the key `-1` left a read of `table[0]` looking at an initializer the write had already replaced; a one-element list reported the overwritten value and called the site resolved. Both now stay unresolved. The name is not a finite selection when a write shares an enclosing loop with the read, and a negative store sends the container down the existing invalidation path, so the answer is `unstable_local` rather than a value the code does not produce. Measured on the tree: unresolved sites stay at 40 and the blocker split is unchanged at `annotation_only=5, argument_name_only=10, attribute_read=7, call_result=14, other=1, unstable_local=3`. No site was resolving through the unsound path, so the generality bought nothing that this takes away. Two positive tests hold the ordering the form was built for: a straight-line rebinding still resolves, and a single write inside a loop is still its only value. Refs loopx-project#4447 B2. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: song <22676124+songoow@users.noreply.github.com>
Signed-off-by: song <22676124+songoow@users.noreply.github.com> # Conflicts: # docs/architecture/rfcs/semantic-vocabulary-convergence-v0.md # docs/architecture/rfcs/semantic-vocabulary-convergence-v0.zh-CN.md
Follow-up to loopx-project#4664, which was approved with these left open and a note to close them together. None of the three moves a number: the residue stays at 40 sites with the split `annotation_only=5, argument_name_only=10, attribute_read=7, call_result=14, other=1, unstable_local=3`. `_MODULE_FUNCTIONS` was keyed on `id(tree)`. That is only correct while `_TREES` retains every tree it parses, which is a property of a different cache in a different part of the file. Give `_TREES` a bound and a reused id hands back another module's functions, binding a same-module call to the wrong callee with no symptom at all. It is keyed by path and text hash now, exactly as `_TREES` is, so the two no longer have to agree by accident. `_is_generator` used `ast.walk`, which descends into nested functions and lambdas. A plain function that merely defined a generator inside itself read as a generator and lost its binding. The direction was safe -- the call kept `call_result` -- but it withheld evidence this slice exists to make actionable, and it did not match what the docstring says is excluded. The walk now stops at a nested scope, which owns its own yields. A function that yields itself is still not bound. `blockerFor` labelled an object literal, an array literal and a template expression `dynamic_key`, which the shared vocabulary defines as a computed or non-literal subscript. None of them is one. Python answers `other` for the same shapes -- a dict literal or an f-string where a scalar was required -- so that is what TypeScript answers too; the whole point of the shared taxonomy is that a reader of the residue gets the same reason from either runtime. Refs loopx-project#4447 B2. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: song <22676124+songoow@users.noreply.github.com>
huangruiteng
left a comment
There was a problem hiding this comment.
动机
这是 #4664(B2 有界生产绑定)的跟进 PR,收掉上一轮评审留下的三条非阻塞发现。三条都只存在于这个栈上的分支里,main 上没有:_MODULE_FUNCTIONS 用 id(tree) 做键(正确性依赖「_TREES 永不淘汰」这个远处不变量)、_is_generator 用 ast.walk(会下降进嵌套作用域,于是「内部定义了生成器、自己并不 yield」的普通函数被当成生成器而丢掉绑定)、blockerFor 把对象字面量/数组字面量/模板表达式标成 dynamic_key(而共用词汇把 dynamic_key 定义为「计算键或非字面量下标」)。
三条都是真实缺陷,而且性质不同:第一条会静默给出错的证据(复用的 id 返回另一个文件的函数表,把同模块调用绑到错的被调用者),第二条扣住了本切片正要变得可行动的证据,第三条让残量的分类依赖运行时。
改动思路
- 缓存键改成
(path, hash(text)),与同文件里_parsed/_TREES完全一致——不是新造约定,而是回到本文件已有的约定。 _is_generator改为从node.body出发、遇到FunctionDef/AsyncFunctionDef/Lambda就停:嵌套作用域拥有自己的 yield,自己 yield 的函数仍然不算生产者(方向安全的一侧被保留)。blockerFor的三个形状改判other,与 Python 对同类形状的答案对齐,typescript_dynamic只留作解析器无法归类的兜底。- 三个回归夹具:嵌套生成器、自己 yield、以及上一轮只覆盖了函数作用域、这次补上的模块作用域回边。
具体改动
关键代码讲解
loopx/semantics/python_production.py::_module_functions:键从id(tree)换成(source.path, hash(source.text)),说明注释直接写出「为什么 identity 键是错的」。loopx/semantics/python_production.py::_is_generator:显式栈遍历,跳过嵌套作用域;自己 yield 仍返回 True。scripts/semantic_production_scan.mjs::blockerFor:对象/数组/模板表达式 →other(一行),并注明与 Python 的一致性理由。tests/architecture/test_semantic_producer_binding.py:三条新夹具,其中模块作用域回边那条填的是上一轮评审点名的空档。
我自己复现的数字(不是照抄 PR 描述)
parent c73aaedb : unresolved=40 split annotation_only=5, argument_name_only=10, attribute_read=7, call_result=14, other=1, unstable_local=3 unresolved_with_values=7
head 0beaecd8 : unresolved=40 同一分布 unresolved_with_values=7
main d8e7af141: unresolved=41 unresolved_with_values=2
TypeScript 位点 : 8 个,attribute_read=5 / call_result=3,typescript_dynamic=0
唯一的 other 位点: loopx/control_plane/turn_driver/executor.py:522(Python)
即「一个数字都没动」成立,而且八个 TS 位点确实是从 typescript_dynamic 改判到 attribute_read/call_result、没有一个是可解析的——这部分是分类学,不是收缩。pytest tests/architecture/ 420 passed;semantic-vocabulary-drift-smoke 打印的 unresolved_producer_sites=40 与上面的分布一字不差并 ok;docs-governance-smoke ok。性能方向也确认过:同一棵树六个生产者词表 best-of-three,head 3.22s vs parent 3.15s,属噪声范围。
对主干的风险
无可阻塞发现。这个 head 的必需检查全绿(含 Sign-off、kernel-static-checks、四个 test shard、stage2c、merge-gate、pytest),只差一次 update branch(mergeStateStatus=BEHIND)。
两条 P3,都是「测试没跟上」而不是代码错:
- 改判
other的那条分支在本树上没有任何夹具、也没有任何位点走到它。 八个 TS 位点全部落在attribute_read/call_result,唯一的other是 Python 位点;也就是说这条改动可以静默回退而全部检查仍然绿。 - 缓存键的修复是「论证正确」而非「测试锁定」。 要证伪需要给
_TREES加上限并制造 id 复用,目前没有这样的测试——而 parent 带着id(tree)键同样通过了同一套测试,说明绿灯本身不能证明这条性质。
我的整体评价
这是把评审意见逐条收干净的正例:三条发现各自用最小改动修掉,方向安全的一侧被刻意保留(自己 yield 仍不绑定),RFC 两个语言镜像都记了「这是分类学不是收缩」,并且主动声明了刻意不绑定的四类原因。PR 描述的每一条我都独立复现过,数字对得上。
两条 P3 值得在后续(不必阻塞本 PR)补上:给 TS 的 other 分支一个夹具或一条对标签集合的断言,再给缓存键写一个能真证伪的测试。可以接受。
English verdict: APPROVE - head 0beaecd closes the three non-blocking B2 findings with the smallest changes (module-function table re-keyed from id(tree) to (path, text hash) as _parsed already does, _is_generator stopping at nested scopes while still refusing a self-yielding def, and blockerFor answering other for object/array/template literals instead of misusing dynamic_key), adds three regression fixtures including the module-scope back edge the previous review asked for, and I independently reproduced every claim: 420 architecture tests pass, the drift smoke prints unresolved_producer_sites=40 with annotation_only=5, argument_name_only=10, attribute_read=7, call_result=14, other=1, unstable_local=3, parent and head measure identically (40 unresolved / 7 unresolved-with-values, versus 41 / 2 on main), the eight TypeScript sites are reclassified to attribute_read (5) and call_result (3) with none resolvable, the head is not slower than its parent (3.22s vs 3.15s best of three), all required checks on this head succeed, and two P3s remain: the relabelled TypeScript branch has no fixture and no site on this tree, and the cache-key fix is argued rather than pinned by a test.
…p3-findings Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
huangruiteng
left a comment
There was a problem hiding this comment.
Reviewed exact head: 5e1f469fc998130e205f4841f2d1e5624087447f (codex/b2-close-three-p3-findings, re-review after the branch was merged with main 736299027; the earlier approval no longer covers this head).
动机
上一轮 B2 评审通过了那条切片,但留下三个非阻塞发现,并注明"一起收口":_MODULE_FUNCTIONS 用 id(tree) 作键(只有当 _TREES 永不淘汰时才成立)、_is_generator 用 ast.walk(会钻进嵌套作用域,让一个只是内部定义了生成器的普通函数被当作生成器而丢掉绑定)、以及 TypeScript 侧把一切都写成 typescript_dynamic(而共享词表里 dynamic_key 指的是计算键或非字面量下标,对象字面量、数组字面量与模板表达式都不是)。三者都不改变残量数量,但它们决定这份证据能不能被读、会不会在别处改动后静默变成错的证据。
改动思路
三个修正各自只做最小的一件事:把派生表的键换成 _TREES 已经在用的 (path, text hash),从而消除两个缓存之间的隐式依赖而不是给依赖加保险;把生成器判定改成"遇到嵌套作用域就停",让嵌套作用域自己拥有它的 yield;把 Node 侧的未解析写入按 Python 同一套标签分类,typescript_dynamic 降级为兜底。第三个修正同时让 production.py 透传 blocker,缺字段时回退到旧标签,因此新旧 helper 都能工作。
具体改动
7 个文件、+1117/-106(相对当前 main):
loopx/semantics/python_production.py(+387/-93):三条有界绑定形式(同模块调用结果、局部变量有序重绑定、按键精确的容器写入)+ 上述三处修正。_MODULE_FUNCTIONS与_TREES同键;_is_generator改为作用域内遍历。scripts/semantic_production_scan.mjs(+35/-6):新增blockerFor,把属性/元素访问判为attribute_read、调用/构造/await 判为call_result、标识符判为unstable_local、对象/数组/模板字面量判为other;undefined视作无值而非未知,a || b/a ?? b与String(x)参与取值。loopx/semantics/production.py(+3/-1):透传 helper 给出的blocker。tests/architecture/test_semantic_producer_binding.py(+454)与test_semantic_python_production.py(+7):三条有界形式的正反用例,加上三个回归(嵌套生成器不使外层成为生成器、自身 yield 的函数仍不绑定、模块作用域回边)。- 两份 RFC 镜像(+133/-3、+98/-3):记录三处修正与 41 → 40 的实测移动。
关键代码讲解
loopx/semantics/python_production.py:260:_MODULE_FUNCTIONS的键从id(tree)换成(path, text hash)。原来的正确性依赖另一个缓存不淘汰——那是最难在评审里看见的一类缺陷:给_TREES加上界之后,被回收的 id 会被复用,派生表就会把别的模块的函数交回来,把一次调用绑到错误的被调方,而且没有任何症状。loopx/semantics/python_production.py:233:_is_generator现在用显式栈遍历,遇到FunctionDef/AsyncFunctionDef/Lambda就跳过。方向本来是"安全"的(只是少给证据),但这个切片的存在意义正是把可解析的情况变成可行动的证据,而旧行为与 docstring 声明的排除范围也不一致。scripts/semantic_production_scan.mjs(blockerFor):标签必须跨运行时同义,所以对象/数组/模板字面量给other(Python 对同样形状——dict 字面量或 f-string 出现在要求标量的位置——也答other),而不是dynamic_key。loopx/semantics/production.py:126:(r.get('blocker') or 'typescript_dynamic')是兼容性的落点:新 helper 给标签,旧 helper 仍得到原来的标签,不会出现无标签的未知。
我复核的关键点(都在这个 head 上自己跑过):
pytest -q tests/architecture→ 508 passed(含三个新回归)。semantic-vocabulary-drift-smoke→ ok,并打印unresolved_producer_sites=40、annotation_only=5, argument_name_only=10, attribute_read=7, call_result=14, other=1, unstable_local=3,typescript_dynamic组已消失。- 与 main 逐项对比(同机、同解释器、同 Node):main
736299027是 41 个位点、attribute_read=2, call_result=11, unstable_local=4, typescript_dynamic=8;本 head 是 40 个位点,八个 TypeScript 位点被重分类为五个attribute_read与三个call_result。RFC 记录的是同一条 41→40 与"八个位点没有一个变成可解析",与实测一致——所以这是一次重分类加一次真实解析,而不是把数字做小。 - 与合并前 head(
0beaecd8b)对比:残量分布逐项相同,说明本次 main 合并没有移动任何数字。 docs-governance-smokeok;git diff --check干净;loopx canary premerge --from-git-diffstatus passed、self_merge_allowed: true、manual_holds: 0。
遗留问题(非阻塞)
残量本身仍然是未证明,而不是已死:15 个位点按标签设计永远不可能成为证据,跨模块调用、对象字段、两跳局部变量与形参绑定都按设计保持未解析,可达性依然未被证明。这些都是 RFC 里写明的边界,不需要在本 PR 里假装解决。
另外一点与本 PR 的合并顺序有关:这个分支是叠在仍未合并的 #4664 之上的,head 里包含 #4664 的全部提交。因此合并本 PR 会同时把 #4664 的内容带进 main,#4664 应当作为已被取代而关闭,而不是再合并一次。
对主干的风险
最大风险是静默的错误证据:错误的模块函数表会凭空造出绑定,而且不会在输出里显形。本 head 用"与源缓存同键"直接消除了这个依赖,而不是给它加上界保护。第二个风险是标签语义漂移:把 dynamic_key 用在字面量形状上会污染共享词表,因此这里改成 other 并保留 typescript_dynamic 作为兜底,未知形状不会被丢掉。本改动不涉及运行时路径、quota 或持久化状态;回退成本一个 commit 组。本 head 相对上次评审只多了一个 main 合并,两处冲突都在两份 RFC 镜像的「只追加」账本与决策表里,两边的条目都保留。
我的整体评价
结论 APPROVE。"三个发现一起收口"这件事做到了:键的问题从根上消除(不是加保护),生成器判定与 docstring 对齐,TypeScript 侧接入共享标签并保留兜底。数量上不夸大——实测 41→40、八个位点重分类、没有一个变成可解析,RFC 与实测一致;三个修正本身没有移动任何数字。唯一需要在使用侧注意的是合并顺序(#4664 会被这个 PR 一起带进 main)。
English verdict: APPROVE - exact head 5e1f469 (re-review after the branch merged main 7362990; the only conflicts were the append-only execution ledger and the decision-log table in both RFC mirrors, resolved by keeping both sides' entries, and the merge moved no number). The three findings from the previous review are closed: _MODULE_FUNCTIONS is keyed by path and text hash exactly as _TREES is, so its correctness no longer depends on another cache never evicting; _is_generator stops at a nested scope, so an enclosing plain function keeps its binding while a function that yields itself is still excluded; and the TypeScript helper classifies unresolved writes with the shared blocker vocabulary (attribute_read, call_result, unstable_local, other) with typescript_dynamic kept only as the fallback, which production.py passes through with the same fallback. I measured both revisions on the same machine: main reports unresolved_producer_sites=41 with typescript_dynamic=8, attribute_read=2, call_result=11, unstable_local=4; this head reports 40 with attribute_read=7, call_result=14, unstable_local=3 and no typescript_dynamic group, matching the RFC's recorded 41 to 40 movement and its statement that none of the eight TypeScript sites became resolvable. tests/architecture is 508 passed, the drift smoke and docs-governance smoke are ok, git diff --check is clean, and canary premerge reports passed with self_merge_allowed true and manual_holds 0. Two notes rather than findings: the residue stays unproven (15 sites can never become evidence, and cross-module calls, object fields, two-hop locals and parameters stay unresolved by design), and this branch is stacked on the still-open #4664, so merging it lands that work too and #4664 should be closed as superseded.
Follow-up to #4664, which was approved with these three left open and a note to
close them together. Stacked on that branch because all three are its own code;
none of the three exists on
main.None of them moves a number. The residue stays at 40 sites with the split
annotation_only=5, argument_name_only=10, attribute_read=7, call_result=14, other=1, unstable_local=3._MODULE_FUNCTIONSkeyed onid(tree)_TREESnever evicts — a property of a different cache elsewhere in the file. Bound that cache and a reused id hands back another module's functions, binding a same-module call to the wrong callee with no symptom._TREESis._is_generatorusedast.walkblockerForsaiddynamic_keydynamic_keyas a computed or non-literal subscript. An object literal, an array literal and a template expression are none of those.other— what Python answers for the same shapes (a dict literal or an f-string where a scalar was required), so a reader of the residue gets the same reason from either runtime.Three regressions added: a nested generator does not make its enclosing function
one, a function that yields itself is still not bound, and the module-scope back
edge from the previous round (the review asked for both scopes; only the
function scope had a fixture).
Verification
pytest tests/architecture/— 420 passedsemantic-vocabulary-drift-smoke— okdocs-governance-smoke— okRefs #4447 B2.
🤖 Generated with Claude Code