feat(semantics): say why each producer site stayed unresolved (B2) - #4581
Conversation
`unresolved_producer_sites=41` mixed three different things, so the number could not be acted on: paths a future slice could resolve, paths that are genuinely dynamic, and paths that can never become evidence at all. Ten of the sites are `keyword_unproved` rows, where a field-named keyword argument does not prove an output role by design, and five are bare annotations that declare the field with no value to resolve. Counting those together with a resolvable local made the total look reducible when it is not. Record the blocker at the point the analyzer gives up and report it: argument_name_only, annotation_only, unstable_local, call_result, dynamic_key, attribute_read, serialized_value, typescript_dynamic, other. Every reported site now carries its label and the smoke prints the counts. This is a reporting refinement only: the site total is unchanged at 41, no judgement changes, and no registry value or budget moves. Measured on this branch: 41 -> 41 sites, blockers argument_name_only=10, call_result=11, typescript_dynamic=8, annotation_only=5, unstable_local=4, attribute_read=2, other=1. Refs loopx-project#4447 (Track B, B2: unknown paths are visible). Signed-off-by: song <22676124+songoow@users.noreply.github.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
32aa4dd to
da54294
Compare
huangruiteng
left a comment
There was a problem hiding this comment.
动机
评审 head:da5429487cf196b2f5d3913a976abddb4d7763e2;base:main(merge-base 6979d528b)。
交付判定(policy 6):fragmented —— 报告目标本身完成得很好,但同一提交改变了 validate_production 的返回格式,却没有更新它自己改动的那个文件里仍按旧格式断言的用例,导致 head 上该文件是红的。因此本 review 是 REQUEST_CHANGES,修复点见下。
为什么这个改进值得做:unresolved_producer_sites=41 把三类东西混在一起——未来切片能解决的路径、本质动态的路径、以及永远不可能成为证据的路径(argument_name_only:字段名相同的关键字参数不证明输出角色;annotation_only:只有注解、没有值可解析)。混在一起会让 41 这个数字看起来"可减少",从而误导 B2 排期。
改动思路
入口是 production.py::validate_production,被 examples/semantic-vocabulary-drift-smoke.py(第 489 行的 unknown 列表与新的 blocker 汇总行)消费;判定权在 python_production.py——标签在分析器放弃解析的那一刻赋值,production.py 只负责渲染。这个分工是对的:只有分析器知道为什么放弃。
标签是类型化的:argument_name_only / annotation_only / unstable_local / call_result / dynamic_key / attribute_read / serialized_value / typescript_dynamic / other,并且明确"只记录第一个放弃原因,不代表整条路径的证明"。
具体改动
5 个文件、+123/-14:Production 行新增 blocker;分析器在各处 give-up 点赋值;validate_production 返回 site:line [label];smoke 打印带标签的列表与 unresolved_producer_blockers=... 汇总;测试覆盖每个可达标签、已解析行不带标签、以及"每个 unresolved 行都有标签"的不变量。
关键代码讲解
python_production.py 的 give-up 点:dynamic_key 出现在无法静态解析的下标/字典键,attribute_read / call_result / unstable_local 分别对应"未解析对象的属性"、"调用返回值"、"参数/重赋值/遮蔽名",annotation_only 与 argument_name_only 明确标注为永不构成证据的两类——这正是本次改动让 41 变得可执行的关键区分。
production.py:195 把返回格式改为 f'{row.site}:{row.line} [{row.blocker or "other"}]',只对 unresolved 行加标签。问题就在这一行的格式变更:同文件里既有的 test_dynamic_path_remains_visible_and_cannot_supply_missing_value 仍断言裸 site:line。
对主干的风险
阻塞项(P2):tests/architecture/test_semantic_production.py:79 在 head 上失败——assert ['...emit:1 [other]'] == ['...emit:1']。我在同一 worktree 上双向验证:merge-base(6979d528b)该文件 32 passed,head 1 failed / 32 passed。也就是说本 PR 的 head 让仓库必需的检查变红,而该文件就在本 PR 的 diff 里。最小修复:把该断言改为带标签的契约(更稳的写法是断言解析出的 site 与 label,或在 row 构造器里显式给出 blocker,避免依赖 other 兜底)。复跑:pytest -q tests/architecture/test_semantic_production.py tests/architecture/test_semantic_python_production.py。
其余部分我独立复核通过:smoke 在 head 上 ok,unresolved_producer_sites=41(不变),41 条 unknown 条目全部带标签,汇总计数 5+10+2+11+1+8+4 = 41 与站点数一致;标签不是对输出文本做子串分类,而是在 give-up 点赋值;除了那一条断言外,tests/architecture/test_semantic_python_production.py 与 test_semantic_production.py 的其余用例通过。
P3(非阻塞):row.blocker or "other" 的兜底意味着"标签丢失"会表现为 other 变多而不是失败;当前靠单测的"每个 unresolved 行都有标签"兜住,建议保留该断言并把 other 增长视为信号。
我的整体评价
baseline(41 条裸条目)与 head(41 条带标签 + 汇总计数)对比:站点集合完全不变,可观测性明显提升,没有放宽任何判定、没有调整注册表或预算——这是正确的"先让未知可见、再谈解决"的顺序。
体量与收益匹配,产物薄而聚焦(复用既有 row 与渲染器,没有新增报告 schema)。唯一问题是"改了共享函数返回格式却没带上同文件断言"的碎片化收尾:修好那一行即可,不需要改设计。修好后我会按同一 head 之外的新 head 重新核验并给出通过结论。
English verdict: REQUEST_CHANGES - exact head da54294; the reporting change itself is verified (smoke ok, 41 labelled entries whose blocker counts reconcile, labels assigned at the analyzer's give-up points, no site resolution, registry or judgement change), but the head is red because validate_production's new 'site:line [label]' return format was not carried into the existing assertion in the same edited file: tests/architecture/test_semantic_production.py passes 32/32 at the merge base and fails 1/32 at the head. Minimum repair: update that assertion to the labelled contract (assert the parsed site and label, or pass an explicit blocker from the row helper), then rerun pytest -q tests/architecture/test_semantic_production.py tests/architecture/test_semantic_python_production.py. One non-blocking P3: the 'or other' fallback can mask a missing label.
…oduced The B2 classification change made validate_production report 'site:line [label]', but the assertion in the same edited file still expected the bare 'site:line' form, so the head was red. Pass the blocker explicitly from the row helper and assert the labelled entry, so a lost label cannot pass as the `other` fallback. Refs loopx-project#4447 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: song <song@users.noreply.github.com>
huangruiteng
left a comment
There was a problem hiding this comment.
动机
我在这个 PR 的上一版 head(da5429487c)上给过 REQUEST_CHANGES,核心意见是:语义 drift smoke 只打印一个 unresolved_producer_sites=41 (not proven safe),让人无法判断这 41 条里哪些本来就不可能变成证据(例如纯字段名的关键字参数、只有注解没有取值),哪些是扫描器能力缺口,于是"覆盖率预算"这件事在评审里没法推进。这一版(29b997edff)就是对那条意见的回应。
改动思路
给每个未解析的生产者站点补上类型化原因标签,并把它们聚合出来,而不是新增一套报告:
Production增加可选的blocker字段(默认None),只有unresolved的行才带标签,已解析的行永远不携带。- 扫描器里原本
return set(), True的每个分支改成blocked('<label>'),标签取自明确的分支类型:dynamic_key、call_result、unstable_local、attribute_read、serialized_value、other;keyword_unproved分支标为argument_name_only,裸注解赋值标为annotation_only。 validate_production的输出从site:line变成site:line [label](缺标签回落other,仍然可见)。- smoke 新增一行
unresolved_producer_blockers=...聚合并保持原计数不变;测试同步断言新契约。
具体改动
loopx/semantics/python_production.py(+56/-19):Production.blocker及其文档、blocked()收集器(每次record前清空,避免跨行泄漏)、各解析分支的标签。loopx/semantics/production.py(+8/-1):TS 路径按unresolved打typescript_dynamic标签;validate_production输出带标签的站点。examples/semantic-vocabulary-drift-smoke.py(+16):summarise_blockers()与新的聚合行。- 测试(+70):
test_reported_sites_carry_a_blocker_label_and_summarise与更新后的test_dynamic_path_remains_visible_and_cannot_supply_missing_value(显式传入 blocker,避免"标签丢了也照样过")。
对主干的风险
风险很低,而且我在本 head 上独立复核了三点:
- 不改变证据语义:未解析站点依旧不能提供缺失值(
resolve对动态路径仍抛错),覆盖率计数仍是 41,预算没有被动过。 - 不影响既有调用方:
blocker有默认值,外部构造Production的地方不需要改;仓库内validate_production的消费方只有 drift smoke 与架构测试,二者同 PR 更新。我rg过Production(与unresolved_producer_sites,没有发现第三处解析该字符串形态的消费者。 - 标签不会静默丢失:没有标签的未解析行在格式化时回落为
other(仍可见),而测试显式传 blocker,避免把other当成"测试通过"。
实际输出(本 head):unresolved_producer_sites=41 与 unresolved_producer_blockers=annotation_only=5,argument_name_only=10,attribute_read=2,call_result=11,other=1,typescript_dynamic=8,unstable_local=4。验证:109 个架构语义用例通过、semantic-vocabulary-drift-smoke: ok。
两条非阻塞小建议(P3,不影响合入):标签集合目前只写在 Production.blocker 的文档串里、没有闭集断言,建议提成模块级 tuple 并在记录时校验成员,免得笔误悄悄变成新标签;另外 record() 取的是 blockers[0],嵌套失败(例如"调用结果的参数里含动态下标")会被记成外层原因,若在意可以保留有序原因列表。
我的整体评价
APPROVE。 这版正好落在我上一条意见要求的位置:既没有把未解析站点"洗白"成已覆盖,也没有用散文解释代替可操作的数据,而是让 41 这个数字第一次可以被分类判断。改动范围小、可回滚、有对应负例断言。上一版的阻塞项已解除,我没有发现新的阻塞问题。
关键代码讲解
python_production.py:24(Production.blocker):新增字段带默认值,且只在unresolved为真时写入(blocker if unknown else None),保证"已解析却带原因"这种矛盾状态不会出现。python_production.py:403(blocked()收集器):把原来各处return set(), True的布尔信号替换成带类型的blocked('<label>');blockers.clear()在每个record()开头执行,标签不会跨行串味,取blockers[0]作为该行的主因。production.py:195(validate_production报告):保持"未解析行必须出现"的既有不变量,仅把输出扩成site:line [label],并对缺标签的行走other兜底,所以最坏情况是标签不准,而不是站点消失。examples/semantic-vocabulary-drift-smoke.py:466(summarise_blockers):从字符串尾部解析[label]做聚合,输出可以直接回答"这 41 条里有多少是永远无法成为证据的"。有了这一行,argument_name_only=10与annotation_only=5这类不可约项与call_result=11这类可约项第一次被分开统计。
语义与 CI 对齐
semantic_alignment = new_semantics_justified(candidate_decision = local_only)。新增的 blocker 标签是扫描器内部的诊断标签,不进入 loopx/semantics/vocabulary_v0.json 的受管词表,也不改变任何注册词表的值域、owner 或 producer 关系,因此符合"按语义角色收敛、不按名字合并枚举"的约束:本 head 的 semantic-vocabulary-drift-smoke: ok(coverage 26/26)、架构语义用例 109 通过即为证据。CI 侧按管理策略不轮询远端检查,只使用本 head 的本地必需验证。
English verdict: APPROVE - reviewed exact head 29b997e. This head answers the earlier request-changes: every unresolved producer site now carries a typed reason (call_result, dynamic_key, unstable_local, attribute_read, serialized_value, annotation_only, argument_name_only, typescript_dynamic, other) and the drift smoke aggregates them, so the 41-site total is actionable instead of opaque. The unresolved count is unchanged, unresolved sites still cannot supply a missing value, the new field is defaulted so existing constructors keep working, and the only in-repo consumers (smoke plus architecture tests) were updated with an assertion that fails if a label is dropped. Validation at this head: 109 architecture tests pass and the drift smoke reports ok with unresolved_producer_blockers=annotation_only=5,argument_name_only=10,attribute_read=2,call_result=11,other=1,typescript_dynamic=8,unstable_local=4. Two P3 notes: the label set is documented rather than asserted as a closed set, and record() reports the first cause when resolution nests.
|
Merge-readiness readback at the reviewed head |
…esolved Signed-off-by: song <22676124+songoow@users.noreply.github.com>
exact-head 复核(
|
huangruiteng
left a comment
There was a problem hiding this comment.
动机
问题是站得住的。B2 的退出标准是"unknown paths are visible"(Refs #4447),但在本 PR 之前,唯一可见的东西是一个 unresolved_producer_sites=41 的总数,而它把三种完全不同的东西混在了一起:后续切片有希望解析的、本质动态的、以及永远不可能成为 evidence 的(字段同名关键字参数不能证明输出角色;裸注解只声明字段没有值)。其中 15 个(argument_name_only=10 + annotation_only=5)是结构上不可约的,却被算进了同一个可约总数里。
我在本 head 实跑确认了这一点:41 = 5 + 10 + 2 + 11 + 1 + 8 + 4,也就是 15 个不可约项确实混在里面。一个不可行动的总数不是"可见的未知路径",而是让人要么忽略这个数字,要么为一个不可能缩减的面去排期。
改动思路
方向正确,而且选的是最小形状:在既有的行模型上加一个带默认值的字段,在分析器已经放弃的那些点顺手记下原因,渲染层把它拼进站点字符串,smoke 再加一个计数。没有新模块、没有新 CLI 选项、没有新的 registry 值。
这一点值得肯定,因为它避免了两种常见退化:一是事后从站点文本反推原因(那就会变成仓库自己禁止的 prose/substring 分类),二是另开一张 (site,line) -> reason 的旁表(那会复制行身份,并可能和分析器实际放弃的点漂移)。现在 reason 由 python_production 拥有、由 production.py 渲染、由 smoke 计数,职责清楚。
作者也把边界写清楚了:"Reporting refinement only: the site total is unchanged, no judgement changes, no registry value or budget moves",并且把新的输出行原样贴了出来。站点集合确实没变(41 → 41),我逐条比对过 41 个站点,除了追加的 [label] 后缀之外完全一致。
具体改动
5 个文件、+131/-19:
loopx/semantics/python_production.py(+44/-12):Production增加blocker: str | None = None;resolve/lookup/enum_object_value在每个放弃点调用blocked(label);record清空按行计数、取blockers[0],并对裸注解覆盖为annotation_only、对字段同名关键字覆盖为argument_name_only。loopx/semantics/production.py(+6/-2):validate_production渲染{site}:{line} [{row.blocker or "other"}];TypeScript 行在 unresolved 时标typescript_dynamic。examples/semantic-vocabulary-drift-smoke.py(+16):summarise_blockers与新输出行unresolved_producer_blockers=...。- 测试(+65/-5):5 个参数化标签用例、
test_resolved_rows_carry_no_blocker、test_every_unresolved_row_is_labelled,以及 smoke 侧的"计数必须等于站点数"断言。
我自己跑的验证:
python examples/semantic-vocabulary-drift-smoke.py(base6b3264fce与 head21d3bf974):两边都是 41 个站点,其余汇总行一致;head 多出unresolved_producer_blockers=annotation_only=5,argument_name_only=10,attribute_read=2,call_result=11,other=1,typescript_dynamic=8,unstable_local=4。pytest tests/architecture -q(head):288 passed。- 直接用
scan_python_production打探针:call 派生容器、attribute 容器、参数容器、以及 unresolvedenum_result路径(见下面两条)。
对主干的风险
P2(非阻塞):lookup 的兜底把可判定的原因塌缩成 other,而仓库里唯一那个 other 站点正是被它误标的。lookup 结尾是 blocked('unstable_local' if isinstance(container, ast.Name) else 'other')——只要容器不是裸名字就一律 other,即使分析器此时已经知道原因。我实测三种形态:call 派生容器(v = fetch() 后 v["k"])→ other、attribute 容器(o.payload["k"])→ other、参数容器(p["k"])→ unstable_local。这解释了仓库里唯一的 loopx/control_plane/turn_driver/executor.py::_run_task_validator:522 [other]:value = validator(plan, result)(executor.py:457)被 bound() 代入 value["recovery_kind"],真正的放弃原因是"call 派生的映射下标",而不是正文所说的 "an IfExp over a dynamic read"。后果对本 PR 自己的目的很具体:正文把 call_result=11 与 unstable_local=4 称作"the candidate list for the next B2 slice",而这个站点恰好是 call-result 依赖,却不在那份候选清单里。最小修复:在兜底处按容器实际形态标 call_result / attribute_read,并补一个 call 派生容器的负例。
P3(非阻塞):enum_result 这条构造没有传 blocker,所以任何 unresolved 的 enum result 都会被渲染成 other。该分支仍然是五参数构造 Production(f'{source.path}::{scope}', node.lineno, 'enum_result', frozenset(values), unknown)(第 510 行),而其它构造点本次都更新了。我端到端复现:某 consumer return Action.RUN if v else compute()(scope 不是注册的 return 函数)产出 Production(..., form='enum_result', values={'run'}, unresolved=True, blocker=None),validate_production 报 m.py::consumer:6 [other],而真实原因同样是 call_result。这个缺口没被新测试拦住,是因为 test_every_unresolved_row_is_labelled 只扫 record 产出的行;它在仓库里目前是 latent 的——当前扫描没有任何 unresolved 的 enum_result 行,唯一的 other 来自上面那条路径。最小修复:像 record 一样把 blocker if unknown else None 传进去,并补负例;这样正文"every reported site carries its label"的说法才完全成立。
另记两点观察(不作为 finding):标签是"第一个放弃原因"而不是整条路径的证明——作者已在正文明确披露;标签词表目前是模块自有的字符串约定而非 typed enum,长期来看枚举更好,但这属于增量内的合理取舍。
我的整体评价
这是一次干净的、真正承重的可读性修复:把一个混了三类路径的总数拆成可行动的标签计数,站点集合与总数完全不变(41 → 41),并且遵守了仓库对"状态分类要类型化、不要 prose 规则"的要求——标签产生于分析器自身的放弃点,而不是事后文本推断。基线与 head 我都实跑过,41 个站点逐条比对一致,head 上 288 个 architecture 测试全绿。
两条记录项(lookup 兜底把可判定原因塌缩为 other、enum_result 构造丢失标签)都不阻塞这次 post-merge audit:站点仍然被报告、被计数,也没有任何判定或授权行为被改变。但建议后续按最小修复补上,其中第一条优先——它是当前唯一一个 other 站点,而它恰好落在正文自称的下一片候选清单里;补法与测试增量都很小。
English verdict: APPROVE (exact head 21d3bf9)
Summary
unresolved_producer_sites=41mixed three different things, so the number could not be acted on: paths a future slice could resolve, paths that are genuinely dynamic, and paths that can never become evidence at all.keyword_unprovedrows — a field-named keyword argument does not prove an output role, by design. Five more are bare annotations (effective_action: str | NoneonQuotaDecisionPacket,QuotaRunDecision,EffectObservation,SettledReplayRoute,_QuotaDecisionRoute) that declare the field with no value to resolve. Counting those together with a resolvable local made the total look reducible when it is not.argument_name_only,annotation_only,unstable_local,call_result,dynamic_key,attribute_read,serialized_value,typescript_dynamic,other. Every reported site carries its label and the smoke prints the counts.#4573 has merged; this branch was rebuilt on current main and now contains a single commit.
Issue Or Task
Validation
unitpassed(replayed)test_semantic_python_production.py: 5 parametrised classification cases (one per reachable label), plus "resolved rows carry no blocker" and "every unresolved row is labelled".test_semantic_production.py: every reported site ends in a label, the summary counts add up to the site count, andargument_name_only/annotation_onlystay separable. pytest is not installable in the authoring sandbox, so cases were replayed by calling the same functions directly; CItest-shardis authoritative.regression_paritypassedexamples/semantic-vocabulary-drift-smoke.py --report: 41 → 41 sites before and after; smoke stillok; F1 did not fire. Labels account for every site (41 = 5+10+2+11+1+8+4), so no site is silently dropped from the report.staticpassedpy_compileon all changed files.integrationpassedloopx canary premerge --from-git-diff --git-diff-base origin/main(tierstandard):ok: true, 11 selected, 0 failures — the selected catalog canaries include the drift smoke itself.other=1remains (executor.py::_run_task_validator, anIfExpover a dynamic read) and is deliberately left generic rather than given a bespoke label. The classification does not itself resolve any site — the 11call_resultand 4unstable_localentries are the candidate list for the next B2 slice, and the 15argument_name_only+annotation_onlyentries are now visibly not candidates.Frontend / Visual Evidence
N/A — no user-visible change.
🤖 Generated with Claude Code