test: align Todo priority integration assertions - #4819
huangruiteng merged 4 commits into
Conversation
Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>
Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com> (cherry picked from commit 40770d1)
|
@huangruiteng 请复审这个独立的主干测试修复。它只对齐 #4771 已建立的 priority 契约:冲突声明写前拒绝、text-only edit 保留结构化 priority 的兼容显示;生产代码不变。 本地 exact head:16 个定向测试通过,Ruff/diff check 通过,premerge 3/3 canary 通过。GitHub review-request API 对当前账号返回权限不足,因此在这里请求 review;远端检查仍在 runner 队列。 |
…-priority-conflict-test Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>
|
同步记录:已用普通、带 Sign-off 的 merge commit 合入 |
huangruiteng
left a comment
There was a problem hiding this comment.
动机
本 PR 修复的是 #4771 已经确立、但两条 Python 集成断言仍未同步的 Todo priority 契约:冲突的 legacy [Pn] 前缀与结构化 priority 必须在写入前拒绝;仅修改文本时,canonical provider 必须保留结构化 priority,并在兼容读模型中继续显示前缀。旧断言会让无关 PR 的测试分片失败,并诱导维护者错误修改已经正确的生产运行时。
改动思路
保持生产代码与 authority owner 不变,只校正两个既有、长期有价值的集成测试。team-plan 负路径现在验证 priority 冲突抛错且没有 Todo 落盘;canonical-provider 路径现在验证 correction、note 和 edit 后仍保留 [P0] 兼容显示及规范化 title。入口、状态 owner、失败责任和读回消费者均沿用现有 Todo contract。
具体改动
tests/test_steward_team_plan_apply.py:把“已有前缀优先”旧假设改为“冲突声明被拒绝”,并显式验证没有 Todo 写入。tests/control_plane/test_local_coordination_authority.py:把 correction、note、provider-owned edit 的期望更新为保留结构化 P0 的兼容显示;edit 同时验证规范化 title。
这是纯测试变更,共两文件 +13/-13,没有生产、CLI、持久化、前端或 Lark 行为变化。
对主干的风险
未发现阻塞项。最大风险是测试误把新的错误行为写成契约;这里通过 production priority validator、canonical read model 与真实本地 canonical provider 路径交叉核对,结果一致。exact head cc5af175b2049442786398a8fb22c86dab176ea1 的本地验证:
- 相关 pytest:
16 passed; - Ruff:通过;
git diff --check:通过;- risk-based premerge:direct checks、Python compile 与 3/3 Todo canaries 全部通过,无 manual hold。
没有查询或等待远端 CI;评审依据是 exact-head 本地证据。残余风险仅是分支当前落后 main,合并前仍需 exact-head merge-readiness 复核。
我的整体评价
APPROVE。这个切片范围恰当:不围绕绿灯改生产代码,而是让既有集成测试重新准确表达已经落地的结构化 priority 契约。现有 coverage 扫描未发现覆盖这两个失配点的重复测试;同作者同期 PR 也不是同形态的测试脚手架,因此不存在低价值 smoke 堆叠问题。面向未来的 bounded refactor 在此不需要:authority 已由 TypeScript priority contract 持有,测试只应对齐,不应再造 owner。
English verdict: APPROVE - exact head cc5af175b2049442786398a8fb22c86dab176ea1; test-only correction aligns two stale Python integration assertions with the shipped structured-priority contract. Local evidence: 16 focused tests, Ruff, diff hygiene, compile, and 3/3 selected Todo canaries passed; no blocking finding.
huangruiteng
left a comment
There was a problem hiding this comment.
动机
更新分支后,main 已包含原先的 canonical-provider 断言修复;当前 PR 剩余的唯一差异,是让 steward team-plan 测试对齐 #4771 已确立的结构化 priority 契约。冲突的 legacy [Pn] 前缀与结构化 priority 必须在写入前拒绝,不能由测试继续认可静默覆盖。
改动思路
生产 owner 保持不变,只修正一个既有集成测试:构造文本 [P2]、结构化 P0 的冲突输入,验证 settlement 抛出 ValueError,再验证 Todo 没有落盘。这样同时锁住拒绝语义和无副作用边界。
具体改动
tests/test_steward_team_plan_apply.py:将旧的“文本前缀获胜”断言替换为“冲突声明被拒绝且无 Todo 写入”。
exact-head base-to-head diff 仅此一处测试,+6/-8;没有生产、CLI、持久化、前端或 Lark 行为变化。
对主干的风险
未发现阻塞项。最大风险是测试误写契约;这里已从 production priority validator 反查错误路径,并通过完整的 steward team-plan 测试文件覆盖普通成功路径与新负路径。exact head 98ec87b8c21ac2f9b80858b9bbca488d0c20cd46 的本地验证:
- 相关 pytest:
15 passed; - Ruff:通过;
git diff --check:通过;- risk-based premerge:direct checks 与 Python compile 通过,无 failure 或 manual hold;此单测试 diff 未选择额外 catalog canary。
没有查询或等待远端 CI;评审依据是更新后 exact-head 的本地证据。
我的整体评价
APPROVE。更新 main 后范围进一步收敛成一个必要的测试契约修复,既不会重复 main 已有修复,也没有同形态 smoke 堆叠。面向未来不需要额外重构:priority authority 已由 TypeScript contract 持有,Python 集成测试只负责验证调用结果与无写入副作用。
English verdict: APPROVE - exact head 98ec87b8c21ac2f9b80858b9bbca488d0c20cd46; the updated branch now contains one test-only correction that rejects conflicting priority declarations before persistence. Local evidence: 15 focused tests, Ruff, diff hygiene, compile, and the risk-based premerge gate passed; no blocking finding.
Problem
PR #4771 made Todo priority a structured field and intentionally established two behaviors that older Python integration assertions did not reflect:
[Pn]text prefix silently override the structured priority;[Pn]display.Current
main@2caada69dfails one test for each stale assertion. These same baseline failures are currently visible in unrelated PR CI, including #4790 and #4803.Change
40770d1b0(originally carried inside feat(coordination): add reviewed whole-Goal coordination-authority promotion #4799) so the smaller baseline repair can unblock unrelated PRs without waiting for the larger authority-promotion feature.Production code and runtime semantics are unchanged.
Validation
git diff --checkpass.loopx canary premerge --from-git-diffpasses.The existing TypeScript team-plan and Todo-priority tests already assert the same contract. Local TS execution was unavailable because this worktree has no
tsxinstallation; no TypeScript or production files changed.