Skip to content

ci: qualify the chat bundle in a browser off the critical path - #5295

Merged
huangruiteng merged 3 commits into
loopx-project:mainfrom
songoow:codex/ci-critical-path
Sep 30, 2026
Merged

huangruiteng merged 3 commits into
loopx-project:mainfrom
songoow:codex/ci-critical-path

Conversation

@songoow

@songoow songoow commented Sep 29, 2026

Copy link
Copy Markdown
Collaborator

Problem

In Python Tests, every heavy lane (test-shard ×4, typescript-core ×3, kernel-static-checks, dashboard-acceptance, stage2c-suite ×4, windows-powershell, presentation) needs chat-bundle. chat-bundle takes 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-bundle builds, runs chat_bundle.py verify --source and uploads chat-bundle-${{ github.sha }} right away.
  • The new chat-bundle-browser lane (needs: [changes, chat-bundle], same core_tests condition) downloads that exact artifact. It runs the same three smokes in the same order: smoke:personal-workspace-packaged, smoke:chat-turn-acceptance-retry and smoke:chat-upgrade. It does not rebuild.
  • The checks aggregate now also requires chat-bundle-browser to succeed. merge-gate and its review_gate.py contract are unchanged. The required checks are still Sign-off and merge-gate.
  • presentation's step is renamed to "Verify the source-bound Dashboard artifact", because browser qualification is now proven through checks.

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 therefore merge-gate, fail. The gate result for every profile (docs, presentation, full) is unchanged: a failed, cancelled or skipped browser lane cannot produce a green merge-gate. docs/development/frontend-delivery.md is updated to match.

Validation

  • pytest tests/test_python_ci_workflow.py: 289 passed. The checks aggregate 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 by checks.
  • 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.
  • Not yet observed: the real runner timings for the new layout. This PR's own run is the first measurement. The expected result is a chat-bundle of about 1 minute, with the browser lane running in parallel with the shards.

Future-facing pass

I considered also folding the checks, pytest and stage2c-correctness-e2e aggregate hops into merge-gate. I deferred it: only the pytest coverage-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 into merge-gate and change the review_gate.py contract, which is a larger review than this change warrants.

🤖 Generated with Claude Code

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>
@songoow

songoow commented Sep 29, 2026

Copy link
Copy Markdown
Collaborator Author

First runner measurement on head 573759910:

before (median of 25 PR runs) this PR
chat-bundle 5.2 min 0.4 min
heavy lanes start after ~6 min (+ queue) 1.8–2.8 min
chat-bundle-browser (parallel) — 4.8 min, success
checks requires browser lane — success at 20 min

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 main failures that other open PRs are fixing, and they are unrelated to this diff:

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

动机

本次按 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

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Signed-off-by: song <22676124+songoow@users.noreply.github.com>
@songoow

songoow commented Sep 30, 2026

Copy link
Copy Markdown
Collaborator Author

Updated to exact head 58532ca1a6f8f94326d2432b1f4ba5cfb15c6ac1.

What changed: one signed merge of origin/main (649826221) into the branch; it merged cleanly. The PR diff itself is unchanged: it still only moves the chat-bundle browser smokes into the parallel chat-bundle-browser job, which checks requires.

Local validation on the merged head:

  • pytest tests/test_python_ci_workflow.py tests/test_dco_workflow.py tests/test_sonarcloud_workflow.py tests/test_contributing_guide.py: 316 passed
  • unittest discover -s scripts/ci: 7 tests OK
  • examples/github-actions-runtime-smoke.py: ok

Why the previous run was red:

  • typescript-core (3/3) failed on host_process.test.ts: the timeout, leader_exit and closed_pipes variants of "descendants cannot keep working after managed execution returns" (child kept changing state after return, '' !== '40'). This is a timing flake, not something this PR causes. The PR touches no TypeScript or runtime code. The same test fails on and off in unrelated PRs: feat(collaboration): wake the requesting lead once a delegated result is accepted #5304 (timeout variant, shard 1/3) and codex/revocable-agent-preferences (closed_pipes variant, shard 2/3). It passes in most other recent runs, and a different subset of variants fails each time.
  • checks, pytest and merge-gate failed only because of that shard. typescript-coverage was skipped, so checks saw TYPESCRIPT_RESULT=skipped. The new chat-bundle-browser job passed (BROWSER_RESULT=success).

Red checks you can still expect from main: the latest main push run (649826221) fails tests/architecture/test_turn_contract_generation.py::test_new_independent_twin_cannot_hide_behind_generated_pair and tests/control_plane/test_prompt_upgrade_hook.py::test_live_decision_adds_only_existing_required_read_channel[quota_cli_invocation|loopx_turn_run_once]. Both also fail locally on this merged head, so the new run will probably show them in test-shard. They are not caused by this PR, and they are being fixed separately.

The push dismissed the earlier approval, so I am requesting re-review on this head.

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

English verdict: APPROVE

没有阻塞性发现。按 LoopX PR review capability policy 12,对当前完整 PR 重新评审,不继承旧 head 的批准。评审 head:58532ca1a6f8f94326d2432b1f4ba5cfb15c6ac1;固定比较基线:649826221289cd4cb3dd8880d016e0afbbaca0fc。这是代码评审结论,不代表合并或取消必需检查。

动机

旧工作流把构建、来源校验和三个浏览器验证放在同一个 artifact producer 中。核心测试消费者即使只需要已经构建好的包,也必须等待浏览器完成。目标不是降低测试门槛,而是解除这个额外串行依赖,同时保持同一提交的 UI 包和浏览器合格证明最终汇合。这个可独立回滚的 CI 图调整完成了所述有界目标;没有把它扩大成整个 CI 性能治理已完成的声明。

改动思路

最强的反对理由是:提前发布 artifact 后,消费者可能把“已构建”误当成“已通过浏览器验证”。本 PR 在原有 owner 内解决它:producer 只负责构建及 source fingerprint,新增 sibling job 消费相同 SHA 命名的包,原有 checks 再要求这个 sibling 成功。没有另建资格数据库、手工确认字段或一套 UI 测试。Doing nothing 保留无价值的等待;仅移动 upload step 而不拆分 job,依赖仍等待整个 producer;将浏览器设成 advisory 则违反现有资格要求。

普通贡献者仍提交 PR、查看失败 lane、修复后重新提交,不必添加标签或重复确认。持续工作维度是有依据的受控取舍:消费者可提前工作,浏览器失败时会多做一部分最终不能合入的测试;现有超时和工作流取消机制限定成本,修复后由新运行重新资格化,不复用失败运行的资格状态。这里证明的是依赖图及恢复语义,不是未经测量的线上耗时提升。

具体改动

关键代码讲解

  1. chat-bundle producer:仍由已有 impact plan 决定是否构建,保留 build、verify --source、缺失文件报错和 SHA artifact 名称。只是把浏览器资格从 producer 的完成条件移出;消费入口、包布局和构建 owner 没变。
  2. chat-bundle-browser:等待 changes 与 producer,只对 core qualification 运行,下载同一 SHA 包,再按原顺序执行 packaged workspace、失败重试和 upgrade 三项 smoke。此路径没有再次运行产品构建,也没有 continue-on-error,测试的是 producer 的包而非另一个产品快照。
  3. checks aggregation:保留 always 聚合和现有必需检查名,将 browser result 加入 needs、环境输入和独立 success 断言。失败、取消、意外跳过均不能只靠其他 lane 成功放行。

完整差异为三文件、74 行增加和 25 行删除,分别是工作流拓扑、耐久回归测试和 frontend delivery 文档;消费者步骤改称 source-bound,避免提前声称 browser-qualified。最近集成提交改变了共同的 bundle source owner,因此这次重新构建并验证当前包,不以旧评审的结果代替。没有改动前端源码、配置入口或第一屏,也不需要新增 frontend companion。

对主干的风险

独立反例直接运行工作流里的 Bash:三个旧结果的 64 种组合和新四结果的 256 种组合,只有全部 success 可以通过。将新增 browser 断言删除后,“browser failure、其余 success”的同一 fixture 错误返回零;当前 head 对它返回一。这证明测试能发现最危险的丢门禁变化,而不只是复述 YAML。真实 classifier 和 merge-gate CLI 还覆盖 docs、presentation、full,以及非 PR 的全量路径;矛盾 exemptions、缺失 checks 被拒绝,恢复结果后重新通过,needs 字典重排不改变完整资格输出。

本地原生验证:相关 pytest 316 项通过;CI owner unittest 7 项通过;GitHub Actions runtime smoke、Ruff、diff check 通过。标准 premerge canary 的 4 项直接检查和 12 项选中 smoke 全部通过。当前源的 build/install、source verify、三个上述 npm smoke、最终 source verify 全部通过,包括实际编译 UI 的浏览器交互及失败恢复。服务端数据和升级响应为隔离 fixture,不证明线上数据新鲜度或 GitHub hosted artifact 运输;没有读取或修改活跃 Goal。

语义与 CI 对齐

资格仍取自现有 impact plan 与必需 merge-gate,不从观察到的远端 check 名称猜规则,也不查询、轮询或等待 CI。固定基线和当前 head 用同一命令复现三项既有失败:generated pair 的 2 != 1,以及两个参数化 read-channel case 的 turn_start_capability_hook_dispatch 差异。失败身份和完整可观察 assertion 详情一致,相关测试及 runtime owner 未被此 PR 改动,仅归一化内存地址和耗时;这些是 pre_existing_unrelated,不是 request-changes 理由。另有本机 Bash 3.2 空数组 nounset 问题,在 base/head 的原脚本同样失败;未借此声称 Ubuntu workflow shell 已被本地执行。合并时仍须单独尊重必需检查,不把已有红项记成通过。

我的整体评价

APPROVE:long_horizon 是上述有明确资格、成本及恢复边界的 accepted tradeoff;user_experience 保持现有提交、诊断、修复重跑的最短有效路径。复用既有命令、artifact contract 和分类/聚合 owner,没有新权限、可变资格注解或平行决策源。未来向重构检查考虑了统一 artifact producer 与资格 owner:当前拆分正好做到职责分离,继续抽象会增加间接层,未作无关抽取。实际 hosted 调度耗时与 artifact 服务仍是安装后观察范围,不影响已验证的图和本地资格结论;没有执行合并。

@huangruiteng
huangruiteng merged commit 97bfca2 into loopx-project:main Sep 30, 2026
26 of 31 checks passed
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