SW-04 gate_enabled() 去除环境变量覆盖:判官门禁真相只在校准状态文件 - #11
Conversation
- gate_enabled() 不再读 JUDGE_GATE_ENABLED(原 优先级 env > 状态文件 > 默认); D8:未过校准门槛的判官只能出报告,env 开关等于给未校准判官留后门 - 删除随之死亡的 gate_var_name();CLI 无状态文件回退改为默认开启 - 测试:test_gate_enabled_respects_env 断言的是本卡移除的 env 覆盖语义, 按卡面替换为 test_gate_enabled_ignores_env / defaults_on_without_state (规格与既有测试冲突,已按工单裁决记录于 PR 描述)
There was a problem hiding this comment.
🟡 Changes recommended
CI does not persist calibration state, allowing stale or incorrectly enabled gate behavior.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR makes judge-gate status depend solely on judge-calibration.yml, removing environment-variable overrides and updating tests and CLI messaging.
Changes:
- Removes environment-based gate control.
- Defaults to enabled without a state file.
- Updates gate behavior tests.
File summaries
| File | Summary |
|---|---|
tests/test_judge.py |
Updates tests for environment-variable ignoring and default behavior. |
src/nsc/eval/gate.py |
Reads calibration state exclusively; CI persistence of that state remains unresolved. |
Review details
Suppressed comments (1)
src/nsc/eval/gate.py:25
- This change leaves the published gate contract inconsistent with the implementation:
docs/GATES.md:10,14-15andspec/rubrics/pairwise_protocol.md:30still sayJUDGE_GATE_ENABLEDcontrols the gate, andwrite_gate_state()still documents writing that variable. Operators following those instructions will believe calibration disables CI when the variable is now ignored; update the affected contract/documentation (or retain a clearly documented compatibility path) together with this behavior change.
def gate_enabled() -> bool:
"""判官是否允许参与门禁。真相:judge-calibration.yml > 默认开启(SW-04:无 env 覆盖)。"""
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| val = os.environ.get(gate_var_name()) | ||
| if val is not None: | ||
| return val.strip().lower() not in ("0", "false", "off", "no", "") | ||
| """判官是否允许参与门禁。真相:judge-calibration.yml > 默认开启(SW-04:无 env 覆盖)。""" |
There was a problem hiding this comment.
确认问题真实,但修复涉及资产层真相与 CI 流程的所有权决策,超出本卡(SW-04 的范围=去除 gate_enabled() 的 env 覆盖)能自行决定的面:\n\n1. judge-calibration.yml(状态文件)按 ADR-0006/D28 属"真相在 git"的候选——把它提交进库等于把校准结果变成资产层,需要 ADR + 人类确认;\n2. 或者改 workflow(judge-calibration.yml 用 artifact/cache 持久化状态、llm-eval.yml 读取)——CI 层改动也已在 PR 描述"遗留"一节记录为待 owner 决策;\n3. 本 PR 已把惰性化的 gh variable set 动作与 env 注入记录在案(不再被代码读取)。\n\n已在 PR 描述补充"遗留"清单。建议:如果 owner 同意"校准状态文件入库",我可以另开一张卡(提交 judge-calibration.yml + workflow 改为读文件 + 删除 gh variable 动作),一张卡做完不扩散本 PR。请裁决。
这个 PR 改的是哪一层?(必选其一)
spec/profiles/brands/cases/export/)→ 必须打标签asset-change并附 ADRsrc/tests/)→ 无需 ADR(未改 eval/thresholds.yaml)工单
Closes # · 工单号:SW-04(上游依赖卡,Lab 仓 docs/WORK_ORDERS.md §上游依赖卡)
问题
gate_enabled()原优先级是 环境变量 > judge-calibration.yml > 默认开启。D8 规定"未过校准门槛的判官只能出报告",而环境变量允许任何人在本地/CI 越过校准关闸强制开门禁(或反过来关掉已校准门禁)——门禁真相被运行时环境稀释。改动
gate_enabled()只读judge-calibration.yml(judge_gate_enabled),无状态文件默认开启gate_var_name();python -m nsc.eval.gate无状态文件的回退分支改为默认开启并更新提示文案eval/thresholds.yaml的judge_gate_enabled_var键未动(A 资产层,改它需 ADR + 人类确认;现在它是无害的死配置,可在后续卡清理)旧测试
test_gate_enabled_respects_env断言的正是本卡要移除的 env 覆盖语义。按工单最小合理方案:替换为test_gate_enabled_ignores_env(env=true 不得越过校准关闸 / env=0 不得关掉已校准门禁)与test_gate_enabled_defaults_on_without_state。其余既有测试未动。遗留(不在本卡范围,记录待owner决策)
.github/workflows/llm-eval.yml仍向步骤注入JUDGE_GATE_ENABLED: ${{ vars... }},本 PR 后该注入对代码不再生效(惰性化);CI 门禁行为改为"状态文件存在则按文件,否则默认开"。judge-calibration.ymlworkflow 写仓库变量的动作同样惰性化。docs/GATES.md与judge/calibration.py报告文案中的 JUDGE_GATE_ENABLED 字样为文档性提及,未动。验收命令(贴出你本地跑通的输出)
检查表
prompts/影响生成结果吗?