Conversation
b1561af to
fcba6c9
Compare
huangruiteng
left a comment
There was a problem hiding this comment.
English verdict: APPROVE
Reviewed head: fcba6c957b8e88b0530fc54f0a9483b77417cd33;immutable merge base: b15413ffcb9d362e6b30c960a6be49359dab1471。无阻断项。
动机
GH-C37 要求一个可复用、公开安全的交互好坏案例。本 PR 解释“删除了代码,不代表消费其路径的执行清单已经更新”:失效参数可能使整个验证入口无法开始,后来贡献者还会被误导。这是有用的文档交付,不代表 Windows 故障本身或所有清单检查已经修复。
改动思路
以仓库树为真实来源、执行清单为可验证的持久声明,区分发现清单、修正引用和证明剩余覆盖。好案例来自实际分片选择器,坏案例来自 prototype 退役后仍引用已删测试的工作流及贡献任务。它描述的是变更生产者与执行消费者之间的边界,不是把某个 canary 分组另起为 IP;IP-024 的修复写回、IP-038 的诊断归因也不直接约束清单与树的集合一致性。
具体改动
全量差异只有 catalog 一个文件,+86/-1:在 State And Boundary 的家族矩阵和登记表加入 IP-039,补充 trigger、预期行为、可视模型、bad smell 与验证引用。当前矩阵保留其他五列;没有 runtime、CI、权限、安装技能或首屏布局改动。
关键内容讲解
- 登记行及案例入口:明确维护者职责、无需用户中断以及检查剩余引用的行为,未把维护指导变成新的执行授权。
- 预期行为:把自动集合比对和字面参数导致的收集失败联系起来。引用的两个测试函数和 339 行均存在;退役提交确实删除了相应 Python provider 测试。
- 验证部分复用现有结构 smoke、coverage audit 和分片测试,没有为了新文案新增只断言文字的测试。
对主干的风险
最大风险是把文档登记完整误当成所有执行清单都已被机器保护。coverage audit 证明的是家族覆盖,不是 Windows 工作流已执行;本次没有查询远端 CI,也不声称已修复历史坏案例。原生完整 catalog smoke 通过,coverage audit 为 required 38 / covered 38 / missing 0 / drift 0,两个引用的分片测试通过;单文件公开边界扫描无错误、diff whitespace 检查通过。仓库扫描另有未改动合成 fixture 的状态投影警告,不是本次内容泄露或验证失败。
我的整体评价
APPROVE。长期推进和用户体验的收益是明确失效执行入口的真实归因与维护边界,而不是增加测试数量。新增案例有独立的消费契约,范围和代码量适当;没有额外兼容分支可删。面向后续改动的检查已考虑聚合清单校验与现有 fixture 复用,这个文档 PR 不需要扩展成通用扫描器或引入新的持久状态。后续维护仍应保持指导、实际机器检查与故障修复三个结论分开。
Contributor task GH-C37 asks for one new public-safe good/bad case in the interaction pattern catalog. This entry covers a boundary the catalog does not have yet: a path list that drives execution is a claim about the tree, and a retirement that does not inventory those lists removes the work they ran as well as the code they named. The good case is the shard contract that already exists: `tests/test_python_ci_workflow.py::test_typescript_shards_select_every_test_file_exactly_once` asserts the selector returns exactly the set of `tests/control_plane_ts/*.test.ts` for one, three and four shards, and rejects an out-of-range shard. The bad case is the same lane without that guard: loopx-project#5105 retired the Stage 0 Python prototype and left three manifests naming removed paths, and because pytest resolves its arguments before it collects, the Windows lifecycle step aborted with `ERROR: file or directory not found` and `no tests ran in 0.00s` — its other ten files stopped executing while `windows-powershell` reported failure on every pull request. The entry follows the catalog's template and is registered in the family table and the Pattern-To-Canary matrix, so the structural contract holds: 39 table rows and 39 detail headings, one family per id. Validation: `loopx check --scan-path docs/concepts/interaction-pattern-catalog.md` -> errors=0 with a clean public boundary scan, and the catalog's own smoke check (`require_catalog_structure` from `examples/interaction-pattern-catalog-smoke.py`) passes against the edited text. Signed-off-by: kokokoXUY <13682395396@163.com>
fcba6c9 to
4b086d1
Compare
|
Closing this: the board now retires GH-C37, and I should follow that rather than argue
For the record, this entry did cite consuming tests ( The catalog is not left needing this: the coverage audit is clean without the entry. |
Goal And Delivered Outcome
interaction pattern catalog, with trigger signals, user channel, agent channel, state contract, bad
smell, and a validation reference. The catalog had no entry about the boundary this keeps meeting: a
path list that drives execution is a claim about the tree, and a retirement that does not inventory
those lists removes the work they ran along with the code they named.
Executed List Must Resolve Against The Tree, registered in the State And Boundary family table and in
the Pattern-To-Canary matrix, with a detail section.
loopx canary coverage-auditreportsrequired_patterns: 38,covered: 38,missing: 0,drift: 0.main.The Good And Bad Case
tests/test_python_ci_workflow.py::test_typescript_shards_select_every_test_file_exactly_onceasserts the shard selector returns exactly the set of
tests/control_plane_ts/*.test.tsfor one, threeand four shards, and rejects an out-of-range shard — a list checked against the tree instead of
maintained by hand.
prototype, the step still passed a removed test to pytest; because pytest resolves its arguments
before it collects, it aborted with
ERROR: file or directory not foundandno tests ran in 0.00s,so the step's other ten files stopped executing while
windows-powershellreported failure on everypull request.
Correction In This Revision
The first revision of this branch truncated the
State And Boundaryrow of the canary matrix to itspattern-id column, dropping the archetype, trigger-surface, fixture and failure-meaning columns. The
structural smoke did not catch it, because it only checks that an id appears in the matrix, not that the
row keeps its content.
loopx canary coverage-auditdid:The row is now appended in place (
IP-037, IP-038 |→IP-037, IP-038, IP-039 |), so every remainingcolumn is untouched.
Validation
staticpassedloopx canary coverage-audit→required_patterns: 38,covered: 38,excepted: 0,missing: 0,drift: 0. This is the check that caught the truncation in the first revision.staticpassedloopx check --scan-path docs/concepts/interaction-pattern-catalog.md→ exit 0,errors=0, clean public boundary scan.unitpassedrequire_catalog_structurefromexamples/interaction-pattern-catalog-smoke.pypasses against the edited text: no duplicated id, no table row without a detail heading, no detail heading without a row, no id under two families, no matrix id without a row.manualpassedreal checkout of
main, not against the generator's output. The catalog-CI smoke also asserts snippetsin
docs/state-interaction-model.mdand the self-repair reference; those files are unchanged by thisPR, and the catalog-side contract it does own is covered by the two checks above.
Frontend / Visual Evidence
Not applicable: documentation only.