Repository navigation
periodic_report: single-source the reportable trigger kinds - #8630
2430468136 wants to merge 1 commit into
Conversation
triggers.py restated core._REPORTABLE_TRIGGER_KINDS as _REPORTABLE_KINDS while already importing eleven names from core -- the strongest form of the duplicate-knowledge case. It now borrows the owner object and the restated name is retired. The neighbouring _NON_REPORTABLE_KINDS is a different vocabulary and is left alone. The weekly preset's enabled_kinds lists the same eight kinds, but it is a per-preset configuration (the portfolio preset selects only cadence_due), so the guard declares it as an exception rather than folding it into the owner: coupling it would auto-enable future kinds, a domain decision. The declaration is pinned to one site. New guard: folded-value census, identity, retirement, behavioural parity through _normalize_policy, and poison controls. Refs loopx-project#4447. Signed-off-by: psychologister <230141834+psychologister@users.noreply.github.com>
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
loopx-agent
left a comment
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
本次评审对应提交 91fc13a，运行时对照当前主干 b7c9178。
[P2] 全包扫描误拒绝受支持的独立报告配置。 在隔离源码夹具增加显式选择相同八个 kind 的第二份报告配置，设置 21,600 秒间隔。真实触发决策接受它并选中 cadence_due，新增测试仍将配置文件判为非法重复并退出 1。 同一配置由运行时正常消费；同值的 weekly preset 也已获当前测试豁免。该属性属于各 profile 自身，允许各自选择启用 kind。 在 core 和实际验证入口核验同一个集合。用 profile 行为测试验证各自启用选择，把全包同值匹配留给已有 advisory。 tree_findings · 词表收敛规范
动机 / Motivation
报告收据和触发策略维护各自的八个可报告 kind，triggers 已从 core 导入相关验证器。 受影响的调用方包括配置报告触发规则、评估候选事件及使用内置 profile 的调用方。收据、策略及候选共用可报告集合。原五种抑制 kind 和单独的预设启用配置保有各自语义。 20 次真实 CLI 调用的退出码、完整 JSON 和诊断配对一致，含默认、显式及重复设置、八个 reportable、五个抑制 kind、未知 kind 和两种预设。 实测源码路径与隔离合成输入。未执行真实模型、安装态和托管 CI。
规格依据为当前主干的 docs/architecture/rfcs/semantic-vocabulary-convergence-v0.md。核验 track_a_callers_preserve_behavior 与 same_value_candidates_advisory 两项。收敛 RFC 规定相同值的候选只供审阅。复用要求语义槽、作用域和合同相同。 调用合同 · 词表收敛规范
改动思路 / Approach
运行时重用现有定义。pytest 另对整个 Python 包执行新增源码规则。 对已知消费路径检查实际引用，再通过入口测试验证原行为。新增扫描将同值匹配用于整个 Python 包，独立数据也会触发失败。
具体改动 / Changes
-
core 定义报告收据可以接受的 kind，各 profile 决定自己启用哪几种，且抑制值继续只进入候选解释。 _REPORTABLE_TRIGGER_KINDS
-
收据、策略及候选共用可报告集合。原五种抑制 kind 和单独的预设启用配置保有各自语义。 _normalize_policy
-
20 次真实 CLI 调用的退出码、完整 JSON 和诊断配对一致，含默认、显式及重复设置、八个 reportable、五个抑制 kind、未知 kind 和两种预设。 对原新增测试分别运行空白夹具、独立合法数据和实际消费者重述三个版本。无注入通过，两类注入均被拒绝。 tree_findings
对主干的风险 / Risks
主干 93 项、候选 125 项通过。两侧同三条 pending-intent 用例因旧 Goal activation metadata 失败。Ruff、diff 检查、DCO 和与当前 main 的合入模拟通过。 运行时配对包括合法输入和原有拒绝。源码反例只新增独立数据，原校验分支和调用结果均保留。新增的测试失败会影响后续与这两个校验入口无关的变更。
我的整体评价 / Verdict
请求修改。在 core 和实际验证入口核验同一个集合。用 profile 行为测试验证各自启用选择，把全包同值匹配留给已有 advisory。 保留一个能区分实际消费者重复定义与合法独立数据的回归用例。
English verdict: REQUEST_CHANGES. The shared trigger vocabulary preserved all 20 CLI outcomes. The new package-wide guard rejects an independently valid report profile. Keep ownership enforcement on the validating callers and exercise configuration through the profile contract.
| @pytest.fixture(scope="module") | ||
| def tree_findings() -> dict[str, str]: | ||
| findings: dict[str, str] = {} | ||
| for path in sorted(PACKAGE_ROOT.rglob("*.py")): |
There was a problem hiding this comment.
[P2] 在隔离源码夹具增加显式选择相同八个 kind 的第二份报告配置，设置 21,600 秒间隔。真实触发决策接受它并选中 cadence_due，新增测试仍将配置文件判为非法重复并退出 1。 在 core 和实际验证入口核验同一个集合。用 profile 行为测试验证各自启用选择，把全包同值匹配留给已有 advisory。
What
periodic_report/triggers.pyrestated the reportable trigger-kind set as_REPORTABLE_KINDS(8 kinds) while already importing fromcore.py, the owner of_REPORTABLE_TRIGGER_KINDS. It now borrows the owner object; the restated name is retired. The distinct_NON_REPORTABLE_KINDSneighbour is left intact.Why
The strongest form of #4447: a module restating a constant whose owner it already imports from (no new import edge, no cycle). The by-name inventory cannot see it -- both names are private
_-bindings it does not catalogue, and they differ by name.presets.pylists the same 8 kinds inside_WEEKLY_PROGRESS_PROFILE["trigger_policy"]["enabled_kinds"], but that is a per-preset configuration, not the vocabulary: the portfolio preset resolvesenabled_kindsto["cadence_due"]only. It is a declared exception, pinned by its own test.Guard
New
tests/architecture/test_periodic_report_reportable_kind_owner.py: whole-tree folded-value census (owner + the one declared config site), identity, retirement, the non-reportable neighbour is a different set, the preset config is configuration-not-vocabulary, and behavioural parity throughtriggers._normalize_policy(each reportable kind accepted, non-reportable rejected). Poison-proven.Tests
Architecture guard + periodic_report focused tests pass locally; net regression 0 (the 3
test_periodic_report_pending_intent.pyfailures reproduce identically on the clean base tree).