Repository navigation
refactor(periodic-report): decide the field rules in one owner - #7210
loopx-agent merged 1 commit into
Conversation
Three decisions were each stated twice or three times in this package: the language tag compiled in adapters.py and project_progress.py, the supporting content kinds listed in the same two modules with their members spelled in a different order, and the Goal/Agent identity rule compiled three times under two binding names -- pending_intent.py and request_action.py called it _IDENTITY_RE, cadence_journal.py called it _ID. field_shapes now owns all three and the five faces import it. Every site only asked membership or fullmatch, so the wiring is behaviour-preserving; the two supporting-kind literals were already equal as sets, only reordered. incremental.py keeps its own _IDENTITY_RE and is declared rather than migrated: it is case-insensitive and admits up to 160 characters where the owner is lowercase-led and caps at 128, so merging the names would change what the incremental reader accepts. The declaration pins both halves -- that it stays distinct and that it stays the only such site. No budget anchor moves. re.compile constants and underscore-private closed sets are outside the inventory's counted families, and the drift smoke emits identical output on base and head, so nothing in vocabulary_v0.json needed retightening. Refs loopx-project#4447 Track A. Signed-off-by: ZhixiLi <20579155+xianth123@users.noreply.github.com>
There was a problem hiding this comment.
🔵 Needs a closer look
Required exact-head validation and production-entrypoint parity evidence remain unverified, including confirmation that reported test failures are pre-existing.
No open issues 🎉
What changed in this PR
This PR centralizes shared periodic-report validation rules in field_shapes.py, reducing duplicate definitions while preserving existing acceptance behavior.
Changes:
- Moves language patterns, identity patterns, and supporting content kinds into one shared owner used by five modules.
- Preserves
incremental.py’s distinct, wider identity rule. - Adds ownership guards for renamed or inline copies, consumer wiring, and portable path handling.
| File | Description |
|---|---|
tests/architecture/test_periodic_report_field_shapes_owner.py |
Adds ownership, distinct-rule, and validation regression guards. |
loopx/capabilities/periodic_report/request_action.py |
Uses shared identity validation for requests and failure codes. |
loopx/capabilities/periodic_report/project_progress.py |
Uses shared language and supporting-kind rules. |
loopx/capabilities/periodic_report/pending_intent.py |
Uses shared Goal/Agent identity validation. |
loopx/capabilities/periodic_report/field_shapes.py |
Defines the three shared field rules. |
loopx/capabilities/periodic_report/cadence_journal.py |
Uses shared identity validation for cadence records. |
loopx/capabilities/periodic_report/adapters.py |
Uses shared language and supporting-kind validation. |
Review effort: Balanced
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
APPROVE — 未发现当前 head 引入的阻断问题。 完整评审 head f423e7d698267b0d680cb7058569679ce63a4e1f，基线 6f8b181400dc650234891b32b2bd99f3f3836164。原 bot 提出的真实入口与旧失败归因证据缺口已在本轮独立补验；不是按作者计数直接批准。
动机
维护周期报告字段校验、运行相关测试的贡献者。
原来语言、身份和支撑内容类别分别在多个模块独立定义，身份规则还藏在不同名字下；本次将同一问题归到一个包内所有者，原有不同用途的宽身份规则继续保留。
42组完整报告、渲染与旧日志重放结果保持一致；新增所有者守卫通过，未发现当前 head 引入的阻断问题。
本次不改变报告语言、身份或内容类别的接受范围，不触发模型生成或外部发布，也不完成整个语义收敛计划。
既有 Turn 路由测试失败仍保留；真实模型、外部 sink、实际 Windows 系统及整体长期收益不在这一规则复用切片的已验证范围。
改动思路
三个包内格式不变量放入既有 periodic_report 的小型 field_shapes 所有者，五个消费者直接导入；保留 incremental 的不同语义，比新增注册服务、跨运行时桥或把所有身份规则强行合并更合适。
本 PR 的边界是合并实际相同的三类字段规则、保留既有行为并保护真实消费；旧 Turn 路由和整体语义收敛继续由原 owner 承担。
语言在两处、同一身份格式在三处、相同支撑类别在两处独立定义。只对变量名做相等比较，会漏掉 cadence 的 _ID 以及重新命名的复制。当前删除了这些独立定义，让五个模块消费同一个值所有者；不会因为名字接近就把 incremental 的宽格式合并进去。保留原状有具体同步成本，建立新的全局注册/运行时服务则超出此切片需要。
恢复方案的分阶段整体验收不作为此局部内部修复的附加门。本轮仍核普通消费者、失败/恢复和结果读回，不继承历史结论，也不以测试或记录数量替代长期收益。
具体改动
修改前的规范 docs/architecture/rfcs/semantic-vocabulary-convergence-v0.md，固定 revision 6f8b181400dc650234891b32b2bd99f3f3836164：I1 对已登记核心词表的全局 owner 资格不由本次包内未登记规则修复认证，列为范围外；I5 的预算和硬锚点没有放宽；I6 的本地所有者与实际消费者在同一 diff 可见，当前原生语义检查通过。没有把作者新增的 guard 当成修改前的独立规范，也没有把局部通过当整个收敛计划完成。
关键代码讲解
field_shapes拥有相同的两种 compiled pattern 和三个 supporting 成员。原两个 set 只是排列不同；消费者只问 membership，改为 frozenset 不改变迭代输出。两种 matcher 的 pattern/flags 与对应原调用相同。build_project_progress_periodic_report_source继续沿原 source、adapter、document 和 HTML/Markdown renderers。支持内容仍不能伪装 primary；language 的接受、默认和完整错误保持，未改变条目排序或上限。admit_cadence_window只换身份规则来源，原锁、配置、待发窗口和消费重放仍归 cadence owner。本轮直接消费基线写出的完整旧 journal，跨到后一期和另一个 peer 后仍保持同一 pending window，未写第二份。- 新守卫按 compiled 文本和无序 set 成员扫描当前包，能够发现不同变量名或函数内的直接复制；它是有界静态模型，不能证明任意动态表达式。
incremental.py保留大小写/长度均不同的原格式。pending_intent与request_action的 Goal/Agent 和 failure_code 仍走原完整验证/错误顺序。
完整七文件 +381/-19，其中产品 +43/-19，新所有者28行，测试338行。Guard 覆盖定义来源、真实对象消费、格式面、不同规则和可移植路径等不同缺口；不是额外新增一套产品协议。同域简化已应用于五个消费者，未发现需要引入框架或继续拆分的理由。
对主干的风险
独立复核先在基线冻结原完整输入和结果，再将相同42组输入送到当前真实消费者：source、document、两种渲染和 bundle，以及原 journal/错误，完整 422197字节逐字一致，无归一化。SHA256 615187d4b987ebcf0ee55793d569e99d43b8226298666e374321eff1ae383595。这覆盖合法/非法语言和身份、类别/visibility、缺源及更宽 incremental 格式；不是只比正则布尔值。
25项新增守卫通过。另用真实源码/AST执行8次POSIX/Windows相对路径表示断言；换回旧 str 的4项负控制全部失败，重新命名的私有复制也被真实扫描器发现。这里没有运行真实 Windows 操作系统。基线与当前原相关套件均 420 passed / 12 failed，12个完整失败身份和错误细节在只去掉 pytest 临时目录序号与对象地址后完全一致：旧 Turn route 非 host executable，在报告 hook 之前失败，变化的字段规则不决定该路由。原始失败全部保留，不称为已修复；变更不变量有独立完整产物与 IO 通过证据。
标准 native premerge 6 direct / 5 selected、Ruff 与 compile 通过。已有 disabled/corruption 用例在两个版本均通过，关闭周期报告仍无生成或发布。未执行作者宣称的完整 architecture/canary 大套件，也未查询、等待远端 CI；本轮不将这些未执行范围计为通过。当前 main 10d23a80988ceab757ea6cb98d16a76cf65e4e9c 与基线在 periodic_report 无差异；相关报告来源修复没有改变此处三个格式规则。
我的整体评价
三个包内格式不变量放入既有 periodic_report 的小型 field_shapes 所有者，五个消费者直接导入；保留 incremental 的不同语义，比新增注册服务、跨运行时桥或把所有身份规则强行合并更合适。 本 PR 的边界是合并实际相同的三类字段规则、保留既有行为并保护真实消费；旧 Turn 路由和整体语义收敛继续由原 owner 承担。 当前切片减少真实同步点，并以完整普通消费者和旧日志验证保留原义，可以批准。旧测试维护问题仍需要原 owner 处理；本评审不要求此 PR 顺带改动未触及的 Turn 执行规则或用宽松断言消除失败。
检查完整规则替换及既有报告生成、HTML/Markdown消费者，没有渲染布局或控件变更；本轮完整合成产物逐字节一致，视觉审查不适用。 未执行真实模型、外部 sink、实际 Windows 系统或全体 UI；不以来源级产物一致性认证这些资格。 用户和贡献者不新增配置、导航、复制、确认或恢复轮次。字段校验与仓库守卫仍是执行约束，不把它们称作建议；局部单一所有者改善不等于量化长期收益。批准后还需原生 closeout 与独立合并就绪检查，评审不授合并权。
English verdict: APPROVE — f423e7d698267b0d680cb7058569679ce63a4e1f. The full seven-file refactor preserves42 complete report/render/error/journal observations byte-for-byte, keeps the distinct incremental identity rule and passes25 new guards plus independent negative controls.420 package tests pass and the same12 pre-existing Turn-route failures remain fully attributed; native standard premerge and lint/compile pass. No live sink/model or real Windows qualification is claimed.
What
loopx/capabilities/periodic_report/field_shapes.pybecomes the one owner of three field rules this package used to decide more than once, and five modules import it instead of restating them.adapters.py,project_progress.pyREPORT_LANGUAGE_PATTERNfailure_code)pending_intent.py(_IDENTITY_RE),request_action.py(_IDENTITY_RE),cadence_journal.py(_ID)REPORT_IDENTITY_PATTERNadapters.py,project_progress.py— two literals whose members were listed in a different orderSUPPORTING_CONTENT_KINDSEvery migrated site only asked membership or
fullmatch, so the wiring is behaviour-preserving. The two supporting-kind literals were already equal as sets; only their spelling order differed.Why the value layer, not just the name layer
The third identity site is the reason. A name-keyed scan finds two
_IDENTITY_REcopies and missescadence_journal.py, which compiles the same pattern under_ID. The repo's own semantic inventory is name-keyed and does not countre.compileconstants at all, so no budget would have noticed a third or fourth spelling. The guard therefore folds compiled pattern text and set members (order-insensitive) and asks which modules carry them, at any depth — a copy inlined inside a function is caught the same way (arm P3).incremental.pyis declared, not migratedIt keeps its own
_IDENTITY_RE: case-insensitive, up to 160 characters, against the owner's lowercase-led 3..128. That is bounded-context reuse of one name, not a copy — merging it would change what the incremental reader accepts.DECLARED_DISTINCT_SITESpins both halves so the declaration cannot rot: the two rules must stay different, and that file must stay the only one declaring the name.No budget anchor moves
examples/semantic-vocabulary-drift-smoke.pyemits byte-identical output on base and head (1513 bytes each), counters unchanged atsame_runtime_forks=2/2,same_runtime_fork_definitions=5/5,multi_value_twins=8/8. Soloopx/semantics/vocabulary_v0.jsonis untouched here — unlike #7155, which did move three ratchets. The two PRs therefore cannot conflict on an anchor line.Local validation
tests/architecture+tests/canarytest_content_digest_single_owner.py::test_schema_string_site_is_the_same_question_as_the_owner_bare_shapetests/capabilities/*periodic*+*cadence*FAILEDlists byte-identicalruff check(what CI runs)The head/base delta in the architecture suites is exactly the 25 new guard cases.
mypyis green but reports17 source files, none of which this PR touches — that gate does not cover this change, and I am not counting it as evidence.ruff formatis not run: the repository is not format-clean on its own (35 of 40 sampledloopx/*.pyfiles), and CI runs onlyruff check, so no unrelated reformatting is included.Mutation control
Each arm was applied to a copy of this head, run, and reverted (one mutation per tree, control run green first).
project_progressrecompiles the language rule under its old name_LANG2)adapters._normalize_editorialincrementalmerged onto the owner (declaration goes stale)cadence_journal's_IDcopy returns next to the ownerpending_intentgoes back to a local copyWorth reading together: P4, P7 and P8 keep the package suite fully green — behaviour is unchanged, so only the ownership guard can see them. That is the whole argument for a structural guard next to the behavioural ones.
Portability note from #7155, applied up front
The review on #7155 flagged
str(path.relative_to(root))producing backslashes on Windows while the tables spell forward slashes. Every scanner in this guard goes through one_rel()that callsas_posix(), andtest_scanned_sites_use_one_path_spellingpins both spellings directly (a Windows-style relative path still differs understr()and still folds to the same site string), so this file cannot fail a correct Windows tree.Not in scope
loopx/control_plane/work_items/pending_capability_intent.ts:10declaresconst TOKEN_RE = /^[a-z][a-z0-9_.:-]{2,127}$/— the same rule as a second runtime in another subsystem. It is a cross-runtime pair and cannot import this owner, but because the binding stems differ (TOKEN_REvsREPORT_IDENTITY_PATTERN) it is not currently listed underdual_runtime_twins, so no counter sees it. Flagging it for the RFC owner rather than folding it here.SKILLS_SUBDIR, the remaining counted same-runtime fork, whosekiro_cli_goal_mode/__init__.pysite is being edited by open PR feat(kiro-cli): opt-in gated loopx agent with preToolUse gate #5291.