Skip to content

fix(semantics): close the three non-blocking findings from the B2 review - #4682

Merged
huangruiteng merged 5 commits into
loopx-project:mainfrom
songoow:codex/b2-close-three-p3-findings
Sep 18, 2026
Merged

huangruiteng merged 5 commits into
loopx-project:mainfrom
songoow:codex/b2-close-three-p3-findings

Conversation

@songoow

@songoow songoow commented Sep 18, 2026

Copy link
Copy Markdown
Collaborator

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.

Finding What it was What it is now
_MODULE_FUNCTIONS keyed on id(tree) Correct only while _TREES never 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. Keyed by path and text hash, exactly as _TREES is.
_is_generator used ast.walk It descends into nested scopes, so a plain function that merely defined a generator inside itself read as a generator and lost its binding. Safe in direction, but it withheld evidence this slice exists to make actionable, and it did not match what the docstring says is excluded. The walk stops at a nested scope, which owns its own yields. A function that yields itself is still not bound.
blockerFor said dynamic_key The shared vocabulary defines dynamic_key as 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 passed
  • semantic-vocabulary-drift-smoke — ok
  • docs-governance-smoke — ok
  • Both RFC mirrors record the three fixes and that the numbers did not move.

Refs #4447 B2.

🤖 Generated with Claude Code

songoow and others added 4 commits September 17, 2026 11:01
…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
huangruiteng previously approved these changes Sep 18, 2026

@huangruiteng huangruiteng 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.

动机

这是 #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,都是「测试没跟上」而不是代码错:

  1. 改判 other 的那条分支在本树上没有任何夹具、也没有任何位点走到它。 八个 TS 位点全部落在 attribute_read/call_result,唯一的 other 是 Python 位点;也就是说这条改动可以静默回退而全部检查仍然绿。
  2. 缓存键的修复是「论证正确」而非「测试锁定」。 要证伪需要给 _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 huangruiteng 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.

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-smoke ok;git diff --check 干净;loopx canary premerge --from-git-diff status 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.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants