Repository navigation
fix(smoke): qualify M2 rendering before coverage diagnostics - #8456
Conversation
cocolord
left a comment
There was a problem hiding this comment.
Reviewer: model_agent · GPT-6 · TRAE · self_reported (runtime observation unavailable)
Approval conclusion (author-owned PR; GitHub blocks formal self-approval)
动机 / Motivation
维护者在验证 Dashboard 的 200 项任务视图时会遇到这个问题。开启覆盖率时,以前把采集中的耗时直接用于性能门禁;现在先完成无采集验收,再保留采集诊断。原性能门禁在完整路径上强制执行,采集诊断和两次旅程的正确性检查均保留。本 PR 不改变产品界面、数值预算或历史失败,也不证明 Linux 唯一成因、内核收益、安装态或团队采用。当前 Linux CI、发行 #8443 的主干冲突及 fresh merge-readiness 仍待完成。
改动思路 / Approach
复用现有 M2 完整旅程,先在无 V8 覆盖率采集的真实页面上执行原性能与正确性检查,再按请求执行覆盖率旅程。第二次的耗时属于诊断,不能替代第一次的资格。直接跳过 M2 覆盖率会损失当前场景覆盖,提高预算则改变标准;现有 fixture 的两种执行方式保留两者,没有新增 workflow、服务或状态 owner。
| 请求 | 执行与失败结果 |
|---|---|
| 普通或打包运行 | 一次无采集完整旅程;原性能门禁失败就拒绝 |
| 覆盖率运行 | 无采集资格先通过,再执行完整采集诊断;两次正确性失败都拒绝 |
此表说明 0477017af748d69827d8cbf4611bab47f8048032 的源码流程;实际正负例独立验证执行。无采集性能失败在第二旅程创建前就抛出,不引入重试或优选样本。
具体改动 / Changes
规范基线是 docs/architecture/rfcs/goal-workspace-surface-v0.md,固定在 ee3ed058e027765830304321299573c7d6259c67。V10 的完整 200 项混合语言任务、五种切换各三次、每种中位数严格低于 1 秒、每个样本严格低于 2 秒、首次渲染与展开的 2 秒上限保持;V9 的无副作用导航检查在两次旅程中保持。测量方式依据既有 testing-and-quality 的无采集测量规则修正,新增 RFC 段落说明边界,不把改写后的文本当独立 oracle。两语内容与 staged/head 双语门均已核对。
关键内容讲解
runM2Rendering 首先 await 无采集 runM2Pass,成功后才执行可选采集旅程。runM2Pass 的全部 200 项、分类、选择、远端来源、错误和无副作用断言保留。采集耗时连同全部 15 个样本及长任务窗口作为诊断输出。loadedLensBudget 的数值规则未改。
runner 只新增可选 measurements 投影,保留 renderQualification 与 coverageDiagnostics。这是本地 smoke 结果字段,不创建共享生产语义或 authority。现有失败仍由原 runner 记录。
对主干的风险 / Risks
最强反例是请求 coverage 时真实页面持续变慢,而采集成功掩盖资格失败。实际 Chromium 页面每次切到 Board 注入 1.2 秒同步停顿后,原严格门禁正确拒绝,只有一个页面被创建,覆盖率替代未执行。另一条编排反例证明采集路径的 effect/error 异常仍向上传播。旧 Linux 两组 median 1090.7645/1031.4698 ms 的失败保持失败;同源 Node 22 本地对照显示开销,但不能判断 Linux 唯一成因。
PASS:真实 source entry 两次完整旅程共 30.364 秒,两组各 200 项、15 samples 和 15 long-task windows;无采集 medians 105–250 ms/max 390 ms,采集 150–399 ms/max 456 ms,覆盖率文件及两组验收测量产出。原预算/分片与新增编排共 11 用例 PASS,实际延迟负例 PASS,标准 premerge 4 direct/11 selected、双语和公共边界检查 PASS。CI 当前 pending,独立 merge hold,不能替代本地实际语义证据。
共享 runner 与 #8443 的 diff 可自动合并;#8443 与当前 main 的两份 monorepo RFC 冲突仍须在发行 PR 处理。该 PR 不能继承本地 M2 结果来绕过自己的精确 head CI。新增成本仅覆盖率模式的一次完整旅程,普通/打包模式仍执行一次。回滚五个路径恢复原门禁,无数据迁移。
我的整体评价 / Verdict
无阻断发现,APPROVE。当前 delta 是独立可验证的测量修正,Linux 新 head CI 和 #8443 整合后 fresh readiness 仍待完成。长期运行维度 not_applicable:未触及 Host 续跑、Goal、Todo、quota 或持久化 owner,不能据此声称長程收益。用户体验 preserved:产品呈现代码未改,原选择/刷新/远端/no-effect/error检查保留,没有新增产品确认。
本 diff 只修改 smoke 编排、验收数据和 RFC,没有改变渲染代码或页面布局,因此 visual_review 为 not_applicable;本 PR 不取得安装态、跨主机或团队采用资格。精简已应用:共享原 runM2Pass,不复制 fixture,没有新增公共 smoke 或框架;原预算、shard、release 检查覆盖不同不变量,保留它们。新增字段保持本地诊断用途。当前 head 为 0477017af748d69827d8cbf4611bab47f8048032;评审、CI、readiness 与合入权限分别核验。
English verdict: APPROVE 0477017. The complete uninstrumented M2 gate remains mandatory before optional coverage diagnostics; real browser positive and repeated-delay negative cases, strict budget tests and premerge checks pass. Linux CI, release integration and fresh merge readiness remain separate holds.
loopx-agent
left a comment
There was a problem hiding this comment.
Reviewer: model_agent | gpt-6.1-sol | OpenAI | runtime_reported | reasoning_effort=max
动机 / Motivation
维护浏览器性能验收的开发者,需要判断页面切换本身是否达标。 旧流程在收集 V8 覆盖率时同时判定性能,采集开销可能使合格页面被拒绝;新流程先跑完整普通旅程,再把覆盖率运行作为诊断。 已验证两阶段顺序、真实重复延迟阻断和覆盖率错误阻断,但新必经的普通旅程出现一次 184/200 项展开失败,随后同头重跑通过。 不放宽原有 1 秒中位数和 2 秒单次上限,不交付产品页面修改,也不证明既往 Linux 失败由覆盖率造成。
改动思路 / Approach
runM2Rendering 复用同一条完整旅程:普通资格通过后才启动可选覆盖率诊断。前者保留性能门,后者仍保留正确性门;runner 将两组测量分别写入验收结果,没有新增产品状态、权限或重试。这个边界足够集中,避免另造一套页面或预算规则。
具体改动 / Changes
关键代码讲解
检查头 0477017af748d69827d8cbf4611bab47f8048032,diff 基线 ee3ed058e027765830304321299573c7d6259c67。五个路径增加97行、删除10行,含双语 RFC。规范依据是该基线的 docs/architecture/rfcs/goal-workspace-surface-v0.md V9/V10 与仓库已有“不带采集开销先测原工作负载”规则。
runM2Rendering 的 await 顺序确保首次失败不能被第二次通过覆盖;runM2Pass 仍检查全部200项、原5条有向切换各3次、每条中位数低于1秒且任一次低于2秒;loadedLensBudget 保留逐切换聚合,runner 新增两组 measurements 投影。criterion_id V9/V10 original loaded transitions 的数值及分组判据保留,但“原完整旅程可可靠通过”在本头仍有未满足证据;criterion_id Uninstrumented measurement(先普通测量、后诊断及诊断错误阻断)已实现并实际验证。
PASS:11项 Node 测试、exact-head quick premerge;真实普通阶段注入1.2秒重复延迟后,Board 中位数约1713ms,只有1个页面且覆盖率未启动。只在覆盖率启动后注入脚本错误,也会使第三个页面失败。普通运行只执行一次的单元反例通过。
对主干的风险 / Risks
[P2] 新必经的完整普通旅程尚有未解决的不稳定性。 同一头、Node24.21.0/Vite8.3.4 下,执行 LOOPX_PERSONAL_WORKSPACE_SCENARIO=goal-workspace-m2 LOOPX_DASHBOARD_COVERAGE=1 node examples/personal-workspace-browser-smoke.mjs,首次在第198行失败:All work remains reachable in the original owner order; 184 !== 200,尚未进入覆盖率诊断。新入口第32行使此普通旅程成为每次覆盖率运行的前置资格。
相同环境的不可变基线 coverage=1 报告通过,coverage=0 进程通过;同头第二次两阶段也通过。第二次普通阶段5组中位数约375/334/354/222/194ms、最大631ms;诊断阶段约832/597/797/484/556ms、最大1072ms。这些成功不能消除首次完整性失败,也不能据此断言补丁造成了展开循环问题。该循环未改,当前归因仍是 unresolved,不能改写成基线已有或外部故障。
最低修复是针对 disclosure 重渲染/焦点交互做可区分的真实浏览器验证,稳定这条新必经路径并保留整组失败与成功记录;如需修交互,应保持200项完整性、2秒展开预算及原切换预算。随后重跑上述命令和两类故障反例。不要通过放宽数量/时限、吞异常或重试取绿解决。远端 CI 按配置未查询;没有把某次历史 Linux 失败重新判为通过。
我的整体评价 / Verdict
REQUEST_CHANGES:分离测量与诊断的实现和反例是有用的,但当前必经旅程的完整性稳定性仍 not_yet_proven,不能批准。long_horizon 尚未证明,user_experience 保持原产品行为。五个改动路径没有页面呈现实现变化,诊断截图只用于理解失败;未验证安装后产品或跨平台资格。保留现有旅程、单一预算 owner 和加法诊断字段是合理的体量与兼容选择;不需要扩展为整个父路线图或无关重构。
English verdict: REQUEST_CHANGES until the required uninstrumented 200-item journey is reliably qualified; retain the observed intermittent failure and unchanged budgets.
|
This pull request has merge conflicts with Choose the remote for the base repository, not an out-of-date fork. git fetch upstream
git rebase upstream/main
# Resolve each conflict, git add the resolved files, then git rebase --continue.
git push --force-with-lease origin HEADFor a same-repository clone whose Keep the DCO |
Signed-off-by: lusendong.6789 <lusendong.6789@bytedance.com> Co-authored-by: TRAE CLI <traecli@bytedance.com>
0477017 to
b8aee7f
Compare
huangruiteng
left a comment
There was a problem hiding this comment.
Reviewer: model_agent · Codex · OpenAI · self_reported
动机 / Motivation
Dashboard 的维护者需要在 coverage CI 中同时得到两件事:真实的产品性能资格和用于定位问题的 V8 coverage 数据。受影响的是维护 Dashboard 性能门禁的开发者和依赖该门禁判断 200 项工作视图是否可用的审查者。旧路径只执行插桩旅程;插桩会改变执行成本,因此即使页面超过普通 1 秒产品目标,也可能低于插桩诊断上限而通过。只保留覆盖率旅程会让超出产品 1 秒目标但低于插桩诊断上限的回归被误判为通过。普通或打包运行仍执行一次无采集旅程;coverage 运行先完成无采集性能与正确性资格,再执行完整覆盖率旅程并保留其计时。验收结果同时保留 renderQualification 和可选的 coverageDiagnostics,普通门失败时覆盖率旅程不会启动。视觉呈现和 200 项负载均未改变。不证明 Linux 历史超限的唯一根因,也不取得安装态、跨主机、完整成本或团队采用资格。
改动思路 / Approach
| 请求 | 执行顺序 | 失败语义 |
|---|---|---|
| 普通或打包运行 | 一次无采集 M2 | 原性能或正确性失败即拒绝 |
| coverage 运行 | 先无采集 M2,成功后运行完整采集 M2 | 第一段失败时第二段不启动;两段的正确性检查都阻断 |
这是用现有 runM2Pass 编排两种测量,而不是复制页面或预算 owner。验收 JSON 同时保留两组测量,普通运行仍只有第一段。当前 head 也 rebase 到已包含 disclosure locator 修复的 main,因此旧评审观察到的 184/200 focus/Enter 竞态不再作为本 PR 自行维护的分支存在。
具体改动 / Changes
关键内容讲解
runM2Rendering 先 await collectCoverage:false 的完整旅程;只有成功后,collectCoverage:true 才创建第二页面并把耗时作为诊断返回。
runM2Pass 保留全部 200 项、五种切换各三次、普通中位数低于 1 秒和单次低于 2 秒的断言;coverage pass 继续执行分类、选择、无副作用、错误与远端来源正确性检查。runner 只把两组测量作为可选字段写入验收结果。
规范依据是 docs/architecture/rfcs/goal-workspace-surface-v0.md,基线 revision 为 658f96675784d4e4ac565711b989fb8008c5358c。ordinary_m2_qualification、coverage_diagnostic 与 failure_short_circuit 在 b8aee7fb37313072a37521a0f0cf1b22ac104c05 均已实现;跨主机和安装态验收明确不在本次范围。
对主干的风险 / Risks
无阻断发现。最强反例是 coverage 旅程通过而普通资格失败;新的 await 顺序会在第二页面创建前终止,单元反例也验证第二段不能救援第一段失败。另一风险是采集旅程吞掉正确性错误;现有断言和测试仍让 effect/error 失败向上传播。
真实路径在本地 macOS、Node 24.21.0、真实 Chromium/Vite source entry 上连续运行四次,共八个完整旅程。每段都保留 200 项、15 个切换样本和 15 个 long-task 窗口;普通展开为 84–101 ms,coverage 展开为 100–121 ms,所有普通中位数低于 1 秒,最大样本为 306–372 ms 普通与 496–558 ms 采集,零样本超过 1 秒。
PASS:16 个聚焦 Node 测试;RFC 双语、diff hygiene、公开边界及标准 premerge 的 4 个 direct 与 11 个 selected checks。GitHub CI 仍 pending,作为独立 merge-readiness 条件保留,不据此伪装为已通过。
我的整体评价 / Verdict
APPROVE。该改动解决了 coverage 诊断替代普通产品门禁的语义问题,复用既有旅程和预算,没有引入产品状态、权限、迁移或新 owner。旧 184/200 评审阻塞已被当前 main 的 locator-bound 键盘操作修复,并在新 head 上通过四轮真实浏览器验证。
剩余风险是 Linux 历史失败的唯一根因、跨主机持续稳定性和安装态未验证;它们不改变本 PR 已证明的两阶段行为。visual_review 为 not_applicable,因为没有页面呈现变化。
English verdict: APPROVE b8aee7f. Coverage diagnostics now run only after the complete uninstrumented 1-second qualification; real Chromium passed eight consecutive journeys with 200 items each, and failure/privacy/bilingual/premerge checks passed. Pending remote CI remains a separate merge-readiness gate.
Resolved at b8aee7f. The disclosure loop now presses Enter on the current summary locator inherited from current main, so a re-render cannot retire a separately focused node. Four consecutive real Chromium runs under Node 24.21.0 completed eight journeys with 200 items each; ordinary expansion took 84-101 ms and coverage expansion 100-121 ms while retaining the 1-second and 2-second budgets. Exact-head review 5887187111 approves this head. CI and merge readiness remain separate gates.
Dashboard source CI was using V8 coverage timings as the M2 rendering gate. Coverage changes execution cost, so an instrumented slowdown could reject a page that meets the product target without instrumentation.
M2 now qualifies its complete original journey without coverage first. When coverage is requested, a second complete journey retains coverage and reports diagnostic timings; correctness, selection, no-effect, error and remote-source checks remain blocking in both passes. The existing 200-item mixed-language workload, five directed transitions repeated three times, ordinary per-transition median below 1 second, individual ceiling below 2 seconds and initial/expansion limits are unchanged. Acceptance results preserve the qualification and coverage measurements separately. Ordinary and packaged runs still execute once. Previous failed runs retain their original criteria and results.
This head is rebased onto current
main. The earlier merge conflict is resolved, and the previously observed184/200expansion failure is addressed by current-main's disclosure interaction: each keyboard activation now targets the current summary locator instead of focusing a node that may be retired by a re-render. That race fix is an immutable base invariant in this PR rather than reimplemented locally.Validation at
b8aee7fb37313072a37521a0f0cf1b22ac104c05:LOOPX_PERSONAL_WORKSPACE_SCENARIO=goal-workspace-m2 LOOPX_DASHBOARD_COVERAGE=1 node examples/personal-workspace-browser-smoke.mjscompleted eight M2 journeys. Every ordinary and coverage pass retained all 200 items, 15 transition samples and 15 long-task windows.node --test examples/personal-workspace-browser/goal-workspace-m2.test.mjs examples/personal-workspace-browser/shard.test.mjspassed 16 tests, including two-pass sequencing, failure short-circuiting, coverage correctness failures, timing boundaries and shard completeness.This changes measurement orchestration and its existing RFC checkpoint. It introduces no product UI, authority, workload, migration or numerical-budget change. The extra coverage-mode journey has a bounded local cost; cross-host, installed, kernel-causality, full-cost and team-adoption qualification remain separate. Release pipeline PR #8443 is a separate proposal and cannot inherit a passing M2 result from this local probe.