Conversation
Every heavy lane waited ~5 minutes for `chat-bundle`, of which ~4.5 minutes was the Playwright qualification, before downloading the artifact. Publish the bundle once it is built and verified, and run the same three browser smokes in a parallel `chat-bundle-browser` lane that consumes that exact artifact. `checks` now requires the browser lane, so `merge-gate` still cannot pass on a bundle that failed qualification. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: song <22676124+songoow@users.noreply.github.com>
|
First runner measurement on head
The browser lane finishes long before the Python shards, so it is off the critical path as intended. The three failing shard cases are pre-existing
I will re-run once those land. |
Pick up the main fixes from loopx-project#5288 and loopx-project#5292. Co-Authored-By: Claude Opus 5.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.
动机
本次按 LoopX PR review capability 的 policy revision 12 评审完整 head 9e69e28ad3e9fc146a4e21c78081e9db1210327d,对照不可变基线 5ab23b3f67a2c1098717ebb450572c8158683153。没有阻断性发现。旧流程把 bundle 构建、三个检查和上传串在同一个 producer 内,使只需已构建资产的后端、TypeScript 和展示验证也必须等待浏览器。这是 S12 开发体验的具体依赖瓶颈,不是降低测试要求的理由;本 PR 移除了这条串行依赖,同时保留最终资格判断。
改动思路
将“构建并验证来源”和“在浏览器里验证包”分离,但仍使用一个 chat-bundle-${github.sha} artifact。普通贡献者的最短路径还是提交改动、读取具体失败、修复后重跑;没有增加配置、确认或手工复制资产。后续消费者可提前开始,checks 则等待新增 browser lane,再由原有 merge-gate 判定是否合格。浏览器失败时可能已花费其他测试资源,这是有界并行的成本交换,并非失败被豁免:各 job 的超时、取消和最终拒绝仍存在。
我搜索了两端的 workflow、scripts/ci/{impact_plan,review_gate}.py、dashboard npm scripts、packaged smoke 与相邻 frontstage workflow。复用原有 artifact、测试入口和 CI 分类 owner 是合适的;没有创建第二个资格状态、运行时配置或新的 Python/TypeScript 产品决策源。单纯加大超时或缓存不能消除原来的 needs 依赖;再引入通用 pipeline 框架则超出这个问题。
具体改动
关键代码讲解
.github/workflows/python-tests.yml:91–115的chat-bundle仍执行scripts/chat_bundle.py build --install和verify --source,随后立即上传。这里的提前发布只给验证 job 提供输入,不等于发行或部署。:117–143的chat-bundle-browser同时依赖分类和 producer,用同一 checkout SHA 下载同名 artifact。它不重新构建产品 bundle,继续运行原来的三个 npm 入口,也没有continue-on-error。:343–360的checks保留always(),新增needs.chat-bundle-browser.result并逐项要求success。失败、取消和非预期跳过均不能被其他三个成功结果抵消;原有review_gate.verify()继续拒绝失败的checks。tests/test_python_ci_workflow.py将真实 Bash 聚合矩阵扩为四个 lane;frontend delivery 文档与 presentation step 名称同步区分“已构建验证”与“浏览器已合格”,没有误称提前拿到的 artifact 已完成全部资格验证。
正向路径是精确 SHA 构建 → 同一 artifact → 并行消费者/browser → 四 lane 全成功 → 原有 merge gate。负向路径是 artifact 下载失败或 browser failure/cancelled/skipped → checks 非成功 → merge gate 拒绝。docs-only、presentation-only、full 的分类、豁免和成功/意外跳过路径均通过真实 Git + 原 CLI 在 base/head 对照;字典顺序变化不改变判定,矛盾的豁免不能逃逸。四 lane 的 256 组合全部符合独立的“所有必需 lane 成功”判据;删除 browser 的检查语句会让失败错误地通过,该变异被判据识别。
本地验证(均为该 head 的 source checkout):
uv run --extra test python -m pytest -q tests/test_python_ci_workflow.py tests/test_dco_workflow.py
uv run --extra test python -m unittest discover -s scripts/ci -p 'test_*.py'
uv run --extra test python examples/github-actions-runtime-smoke.py
uv run --extra test python -m ruff check tests/test_python_ci_workflow.py
uv run --extra test loopx --format json canary premerge --from-git-diff --git-diff-base 5ab23b3f67a2c1098717ebb450572c8158683153 --tier standard --no-progress
uv run --extra test python scripts/chat_bundle.py build --install
uv run --extra test python scripts/chat_bundle.py verify --source
cd apps/presentation/dashboard
uv run --extra test npm run smoke:personal-workspace-packaged
uv run --extra test npm run smoke:chat-turn-acceptance-retry
uv run --extra test npm run smoke:chat-upgrade结果:309 pytest、7 CI 单测、runtime smoke、Ruff、canary 的 4 个直接检查和 13 个选中检查均通过;同一基线对应 pytest 为 117 项。实际编译包的 24 个 workspace 浏览器场景、接受失败/重试检查和旧 tab 升级检查全部通过,完成后再次 verify --source 通过。边界也已核对:workspace 使用真实包/HTTP server,但业务状态由合成路由提供;retry 构建的是 SSR 测试入口并访问隔离的真实 HTTP 后端,不是重新构建产品包;upgrade 使用隔离的两代资产。这些测试没有写入活动 Goal,也不证明生产环境状态或已安装发行包。
语义与 CI 对齐
docs/development/testing-and-quality.md 的必需检查仍是 Sign-off 与 merge-gate,不是另造一个必需 check 名。新增内部 lane 接回既有 checks,docs/presentation/full、loopx_ci_job_plan_v1、原 DCO 和 consumer artifact identity 均保留。此 PR 是默认 CI 拓扑调整,没有 capability opt-in/default-off 宣称;改动和文档明确披露旧/新行为。
对主干的风险
主要风险是提前上传后把 browser lane 漏出最终 gate,或下载/源码 SHA 不一致;真实 Bash 的全矩阵、丢弃检查语句的变异、producer/consumer identity 与 native packaged 验证共同覆盖这些风险。消费者可能先执行但不会获得合并资格;artifact 仍是测试输入,而不是部署授权。没有修改 loopx/**、apps/**、packages/** 或持久化/用户配置,故不需要配套前端/Lark 改动或第一屏设计预览。
本地 Bash 3.2 执行现有 PR 分类 shell 时出现 extra[@]: unbound variable;在同一 Git 输入下,基线和这个 head 的完整错误一致,出错空数组语句未变。真实分类 CLI 与拒绝路径另外验证通过。这是本地 shell/Ubuntu workflow 版本差异,不是本 PR 回归,也不据此要求该 PR 修改无关代码。没有查询或等待远端 CI;Windows runner、GitHub artifact 服务和整体 wall-clock 收益未在本机重现,不能把作者历史计时当作我的测量结果。
我的整体评价
APPROVE。74 增/25 删、三个文件形成了可单独回滚的完整 CI 改动,价值是解除不必要的依赖同时守住失败闭环,不是测试数量或行数本身。未来向的收敛检查已考虑 artifact owner 与最终资格 owner:继续复用现有实现,不需要额外抽象。建议维持这个边界,不把提前上传推广成“已通过全部资格验证”。评审批准不是 merge-readiness 或合并授权;本次不合并。
English verdict: APPROVE
Problem
In
Python Tests, every heavy lane (test-shard×4,typescript-core×3,kernel-static-checks,dashboard-acceptance,stage2c-suite×4,windows-powershell,presentation) needschat-bundle.chat-bundletakes about 5.2 minutes (median over 25 recent PR runs). About 4.5 of those minutes are the Playwright qualification step, which runs before the artifact upload. So every run's critical path (chat-bundle→ Python shard, about 22 minutes → aggregates →merge-gate) starts with roughly 4.5 minutes of browser work that no consumer needs as input.Change
chat-bundlebuilds, runschat_bundle.py verify --sourceand uploadschat-bundle-${{ github.sha }}right away.chat-bundle-browserlane (needs: [changes, chat-bundle], samecore_testscondition) downloads that exact artifact. It runs the same three smokes in the same order:smoke:personal-workspace-packaged,smoke:chat-turn-acceptance-retryandsmoke:chat-upgrade. It does not rebuild.checksaggregate now also requireschat-bundle-browserto succeed.merge-gateand itsreview_gate.pycontract are unchanged. The required checks are stillSign-offandmerge-gate.presentation's step is renamed to "Verify the source-bound Dashboard artifact", because browser qualification is now proven throughchecks.Behavior change: consumers now start while browser qualification is still running, not after it. If a browser smoke fails, the other lanes still run to completion instead of being skipped, and
checks, and thereforemerge-gate, fail. The gate result for every profile (docs, presentation, full) is unchanged: a failed, cancelled or skipped browser lane cannot produce a greenmerge-gate.docs/development/frontend-delivery.mdis updated to match.Validation
pytest tests/test_python_ci_workflow.py: 289 passed. Thechecksaggregate test now covers the full 4×4×4×4 result matrix, including the browser lane. A new assertion checks that the browser lane downloads the producer's artifact, never rebuilds, runs the three smokes in order, and is required bychecks.python -m unittest discover -s scripts/ci -p 'test_*.py': OK (the merge-gate contract is unchanged).examples/github-actions-runtime-smoke.py: ok.loopx canary premerge --from-git-diff(tier standard): 13 selected, 13 executed, 0 failures; public-boundary scan clean.self_merge_allowed: false. This changes merge-gate qualification, so it is left for maintainer merge.chat-bundleof about 1 minute, with the browser lane running in parallel with the shards.Future-facing pass
I considered also folding the
checks,pytestandstage2c-correctness-e2eaggregate hops intomerge-gate. I deferred it: only thepytestcoverage-combine hop is on the critical path (queue median about 1.6 minutes, p90 about 7.6 minutes). Removing it would move the coverage floor and the Sonar input intomerge-gateand change thereview_gate.pycontract, which is a larger review than this change warrants.🤖 Generated with Claude Code