SW-03 Pass 契约文案资产化:搬入 spec/passes/contracts.yaml(ADR-0015) - #10
Conversation
- 新增 spec/passes/contracts.yaml(ADR-0015,资产层 A5):p3 三条输出格式契约
+ p5 必现视觉/命名/字数目标模板(${name} 占位,string.Template 填充)
- nsc.passes.contract_text() 统一读取注入;p3 常量与 p5 构造函数改为消费 spec
- 文案逐字节不变(对拍 origin/main 的字面量,见 PR 描述验证)
- prompts/** 不动(GEPA 生成物);契约变更经 spec_sha 使缓存失效
- 测试:tests/test_pass_contracts.py(3 例,先红后绿)
There was a problem hiding this comment.
🟡 Changes recommended
contract_text() currently risks stale/missing contract strings due to caching and silent fallbacks, and the new ADR is future-dated.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR asset-ifies p3/p5 “mechanical contract” strings by moving them out of src/nsc/passes/*.py into spec/passes/contracts.yaml (per ADR-0015 / SW-03), and updates the passes to read these contracts via a shared nsc.passes.contract_text() accessor.
Changes:
- Added
spec/passes/contracts.yamlto store p3 static contracts and p5 template contracts (using${name}placeholders). - Updated
p3_beatsheetandp5_dialogueto consume contracts fromspec/viacontract_text()(+ new_dialogue_length_target()helper). - Added
tests/test_pass_contracts.pyto assert spec key presence and verify p3/p5 are sourcing contracts from spec.
File summaries
| File | Description |
|---|---|
tests/test_pass_contracts.py |
New tests validating contracts asset presence and pass-side consumption. |
src/nsc/passes/p5_dialogue.py |
Replaces embedded contract strings with contract_text() + string.Template substitution. |
src/nsc/passes/p3_beatsheet.py |
Sources _SP_CONTRACT / _FACT_CONTRACT / _SC_CONTRACT from spec/passes/contracts.yaml. |
src/nsc/passes/__init__.py |
Adds contract_text() and YAML loading for the new contracts asset. |
spec/passes/contracts.yaml |
New asset file containing p3/p5 contract text and templates. |
adr/0015-pass-contract-strings-as-asset.md |
New ADR documenting the decision and constraints. |
Review details
- Files reviewed: 6/6 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.
| _CONTRACTS_PATH = Path("spec/passes/contracts.yaml") | ||
|
|
||
|
|
||
| @lru_cache(maxsize=1) | ||
| def _contracts() -> dict[str, Any]: | ||
| """SW-03 / ADR-0015:Pass 契约文案真相在 spec/passes/contracts.yaml(资产层)。""" | ||
| if not _CONTRACTS_PATH.exists(): | ||
| return {} | ||
| return yaml.safe_load(_CONTRACTS_PATH.read_text("utf-8")) or {} | ||
|
|
||
|
|
||
| def contract_text(pass_name: str, key: str) -> str: | ||
| """读一个 Pass 的契约文案;含 ${name} 占位(string.Template),由调用方填充。""" | ||
| return str(_contracts().get(pass_name, {}).get(key) or "") |
There was a problem hiding this comment.
已修正(f446ff6 + 047e2fa):1) 去掉 lru_cache,_contracts() 每次重读(文件小、调用频率低),同进程内的 spec 编辑立即生效——补了 test_contract_text_rereads_asset(同进程改文件必须读到新值);2) 文件不可读/键缺失不再静默返回空串,改抛 PassFailure 带诊断句(fail fast,符合 AGENTS.md §7 禁静默降级)——补了 test_missing_asset_fails_fast。代价是一次多余的小文件 IO,换来新鲜度与可诊断性。
Copilot review(PR #10): - _contracts() 的 lru_cache 会让同进程内的 spec 编辑读到陈旧契约 → 改为每次重读 (文件小、调用频率低) - 文件不可读/键缺失原返回空串(静默丢约束)→ 转 PassFailure 诊断句 - 回归:test_missing_asset_fails_fast / test_contract_text_rereads_asset
这个 PR 改的是哪一层?(必选其一)
spec/profiles/brands/cases/export/)→ 必须打标签asset-change并附 ADRsrc/tests/)→ 无需 ADRADR:adr/0015-pass-contract-strings-as-asset.md(status: proposed,等确认)
工单
Closes # · 工单号:SW-03(上游依赖卡,Lab 仓 docs/WORK_ORDERS.md §上游依赖卡)
问题
p3/p5 把"机械复述给模型的输出格式契约"(
_SP_CONTRACT/_FACT_CONTRACT/_SC_CONTRACT、必现视觉/命名/字数目标文案)硬编码在src/nsc/passes/*.py,违反 AGENTS.md §2 "禁止在 prompt/代码里硬编码自然语言知识"。这些文案也不能进prompts/<pass>.json(GEPA 生成物,禁止手改)。改动
spec/passes/contracts.yaml:p3 三条静态契约 + p5 三个模板(动态数据用${name}占位,string.Template 由代码填充品牌/字数数据)nsc.passes.contract_text(pass_name, key)统一读取;p3 模块常量、p5_visual_contract/_naming_contract/新_dialogue_length_target改为消费 spec验收命令(贴出你本地跑通的输出)
检查表
_dialogue_length_target不存在)prompts/影响生成结果吗?