Repository navigation
refactor(periodic-report): import the public-safe slug shape from its owner - #6922
Conversation
songoow
left a comment
There was a problem hiding this comment.
Reviewer: model_agent; OpenAI GPT-6 Astra; provider OpenAI; self_reported.
审阅版本:a5ae893fd00f97847155ebf599c0cc4a83fe8e4c;base:main 5853e151d97d9129bda9dce45a92a0944dcb9565。已阅读全部 6 个文件的 diff、共享定义、四个直接调用方及 architecture guard。以下是源码评审;未执行测试或 CI。
动机
四个 periodic-report 模块仍各自声明相同的 public-safe slug 正则,而既有 guard 通过按文件计数的例外允许它们继续存在。把这些调用点接回已有定义,可以让以后调整 slug 格式时只需修改一处规则,同时撤销相应例外。
改动思路
改动只统一“字符串是否符合 slug 格式”的判断。各调用方仍负责自己的输入规范化、错误消息及后续业务校验;没有把语言标签、版本号或具体状态词的规则一并收进 slug 定义。测试从记录四个未转换文件,改为记录并核对全部七个消费者。
具体改动
关键代码讲解
- 共享定义仍为
^[a-z][a-z0-9_.-]{0,127}$,没有改正则内容或编译 flags;该文件的本次改动仅为说明更新。 - adapters._token、archive._token和audience._token继续使用原来的
str/strip、128 字符上限、lower()和fullmatch()。我核对了 source/item、archive snapshot 和 audience selector 的调用,未发现处理顺序或返回值变化。 - bindings._token 与 _version继续区分 slug 与版本格式,允许数字开头和
+的版本规则没有被误替换。adapters 的语言格式也仍有自己的正则;只有不再需要正则模块的 archive、audience 删除了import re。 - 消费者清单及 importer 核对避免删除清单项后静默减少覆盖;对象绑定断言同时检查导入对象、实际引用及遗留本地定义。原有字面值/拼接扫描和不可折叠构造的仓库扫描仍保留,边界用例也保留 128/129 字符、非法首字符和非 ASCII 字符的区分。
对主干的风险
源码显示本次接受集合、规范化行为和错误消息保持一致;未涉及持久化格式、外部发送或 UI。共享模块只依赖标准库,periodic-report 的 core 在 base 中已使用同一共享定义,因此这里没有建立从共享定义反向导入 capability 的依赖环。仓库调用搜索未发现对被移除私有 _TOKEN_RE 名称的外部引用。
本轮只阅读测试设计,未执行导入、guard 或 periodic-report 用例,不能把作者报告的通过次数当作本轮实测结果。这不构成另一个阻断项。
我的整体评价
暂未发现本次改动引入的阻断缺陷。生产增量小,四个调用点都直接使用已有规则,测试同步清空例外并覆盖消费者清单,和 PR 要解决的重复定义问题相称。按普通代码评审意见发布 COMMENT。
English verdict: COMMENT — no blocking defect was established in this source review. The four consumers preserve their existing normalization and matching behavior while using the shared slug definition; runtime and test execution were not performed in this review.
… owner Four periodic_report surfaces kept their own compile of the lowercase public-safe slug. The canonical owner (loopx/public_safe_text.py) and the guard that protects it already existed; the guard declared these four by file and count because another branch was rewriting their import blocks at the time. That branch is no longer in flight, so convert them. Observable result: deleting a local copy can no longer change what a periodic-report token accepts. Every module that states the shape is now either the owner or a module holding the owner's object. The guard keeps both directions: the per-file declaration budget is now empty, a new importer must be listed as a consumer (new reconciliation test), and the existing folded-value scan still reports re-spellings. Validation: guard 46 passed; periodic_report + guard 449 passed; import boundaries 72 passed; ruff check clean; mypy clean on the four modules. Mutations: local copy in audience.py -> 3 red; inline literal in adapters.py -> 3 red; dropping one consumer wiring -> 1 red (reconciliation test). Residual: ruff format --check already failed on adapters.py and the guard file at this base revision; the pre-existing spans are left untouched. Refs loopx-project#4447 Signed-off-by: Twiink <154496350+Twiink@users.noreply.github.com>
a5ae893 to
d3e66be
Compare
|
Status on the rebased head Remaining red, and why it is not this diff:
Both are the dashboard contract assertion introduced by
The rebase also moved the base from |
loopx-agent
left a comment
There was a problem hiding this comment.
Reviewer: model_agent; model=gpt-6.1-sol; provider=OpenAI; reasoning_effort=max; declaration_source=runtime_reported; execution_observation_id=5c1c8ff291119b0dcabd47cf7d3181f8bb2e0ee31c2d0661492ab7702d3978ea
审阅 head:d3e66be113c5b206b20efc72f99fb23f65aaca25;不可变基线:d95839c16b9e98dbd0c3aa4341b717056547e464。完整增量6文件、+77/-44;未发现阻塞问题。本次审查的是新的main合入后完整增量,没有继承旧a5ae893上的source-only意见。
动机
维护周期报告来源、归档、收件人和投递绑定的开发者。
以前四个报告入口各自复制同一个标识格式;以后修改共享规则时,调用方可能仍使用旧规则。现在四个入口直接复用既有 owner,普通输入保持原结果,规则修改也会传播到实际入口。
四个真实公开入口的 84 组同输入结果或完整错误与基线一致;模拟修改共享规则后,基线四处未采用,新提交四处全部采用。
本片只收敛这一种格式,不调整报告内容、收件人选择、版本格式或发送权限,也不宣称整个仓库的语义词汇已经收敛。
四份完全相同的局部定义使下一次格式修改仍要依赖人工同步。基线守卫明确承认这四处例外;本片把它们接回现有来源,消除的是可实测的维护分叉。它没有新建报告能力,也没有要求用户为了内部格式整理而重新配置已有报告。
改动思路
已有 public_safe_text 负责完全相同的格式;四个报告模块保留各自标准化、业务判断与错误处理,只共享这一条最小规则,避免增加另一套配置或决策 owner。
本 PR 删除四个本地编译点,复用已有 pattern,并清空现有守卫的例外预算;不移动报告业务逻辑、不改变持久格式,也不抽取新的通用 AST 框架。
保留各模块的_text、长度边界、lower和业务检查是必要的:源码来源、归档tag、收件人规则和sink依赖虽然共用标识格式,仍有不同业务语义。版本号原来允许数字开头及加号,语言tag也有自己的语法;这两类规则没有被错误收敛进slug。生产修改净减少代码,测试沿用原守卫而非增加一套生产扫描器。
具体改动
依据改动前的守卫契约,spec_revision=d95839c16b9e98dbd0c3aa4341b717056547e464,以及#4447:
“Converting one means deleting its entry” implemented,四份编译点及例外条目同时退出;
“Declared file count moves fail either way” implemented,保留计数负例并核对完整importer集合;
“Track A” implemented于本格式的真实维护收敛,父任务其他词汇与最终集成验收保持原范围。
关键代码讲解
- PUBLIC_SAFE_SLUG_PATTERN表达式没有变化。实际仓库扫描从owner加四处例外,变为一个literal owner;七个现有消费者均导入它。没有因为Python正则缓存碰巧共享对象就认定收敛。
- adapters._token保留原类型/空白/长度/大小写处理,公开source builder的完整内容、digest和异常文本按同输入比较。
- archive._token继续由原archive builder校验输入及生成manifest,未放宽资源URI、内容摘要或私有字段边界;仅将同一格式切换到owner。
- audience._token仍只标准化标识;实际收件人选择继续使用既有结构化规则,不从标题/摘要推断。
- bindings._token替换格式来源,_VERSION_RE和依赖策略独立保留。真实公开binding输入中的1.0+build仍被当作版本处理,不会套用slug规则。
现有守卫清空DECLARED_INDIVIDUAL_SITES;新增导入者与声明消费者集合的核对。评审中删除一个consumer条目会让实际reconciliation断言失败,恢复一处本地编译也会成为offender。扫描器仍有明确有界折叠/名称识别范围,不宣称任意动态别名都已覆盖。
对主干的风险
在不可变base/head上运行相同四个真实公开入口,覆盖 84组完整值或异常:普通标识、大小写与trim、128/129长度、数字/下划线开头、斜杠/加号、空白/换行、Unicode及非字符串;输出、digest和错误全文完全一致,无语义字段被归一化。
另在隔离进程中只模拟修改既有owner,再导入消费者,走四个公开入口。旧基线四处继续采用局部旧规则,独立断言失败;新头四处全部采用新owner,断言通过。这个反事实证明实际决策来源改变,不依赖对象identity或只在新helper上自证。
相关周期报告加守卫:基线 445 passed,新头 450 passed;新增差额来自消费者/集合守卫。守卫与import-boundary组 73 passed。当前六文件Ruff通过;本地premerge 6项直接检查、5项选中检查全部通过,零hold。最初评审命令误指向不存在的测试路径,没有执行用例;已保留该执行记录并使用仓库真实测试路径完成上述两侧验证。依wait_for_ci=false,未读取远端CI。
语义与 CI 对齐
本片选择reuse_existing:没有新增产品状态、枚举、持久格式或发送权限。AST守卫是测试期约束,原格式错误仍由原调用方抛出;没有把某项测试通过提升为报告已外发、UI已安装或整个语义项目已完成。没有新可选开关,现有默认与调用体验按完整结果保持。
我的整体评价
APPROVE。这个明确的四处owner复用任务是goal_achieved,父#4447继续保留原验收边界。长程行为preserved:报告生成、归档、绑定与后续消费者未新增状态或调度;使用体验preserved:无需新增配置、导航、复制或确认轮次,原结果与错误可直接读回。
复用最近的既有格式owner是最小生产修复,现有守卫的36行净增长保护具体维护缺口。无需为了本片把所有验证器、版本语法或扫描器迁入新框架。未验证外部sink实际投递、安装后界面或任意动态正则语义,这些不属于此次等价引用变更的已交付主张。审批仅针对上述未变head,合并仍需独立readiness与维护者权限。
English verdict: APPROVE — d3e66be replaces four duplicate slug validators with the existing owner while preserving all84 paired real public-entry outcomes and errors. A future-owner-edit oracle fails on the baseline and passes through all four head callers. Baseline445/head450 related tests,73 guard/import-boundary cases, Ruff and native premerge passed. The bounded scanner and this local refactor do not certify arbitrary aliases, external delivery or parent vocabulary completion. CI was not consulted.
Goal And Delivered Outcome
Outcome basis / optional anchor: [Task]: Semantic vocabulary convergence follow-ups after RFC PR #4433 / 语义词表收敛后续任务 #4447 Track A (semantic vocabulary convergence), plus the declaration recorded in
tests/architecture/test_public_safe_slug_owner.py.Goal/source and gap:
loopx/public_safe_text.pyowns the lowercase public-safe slug shape. The guard declares fourcapabilities/periodic_reportfiles by file and count instead of converting them, and its own docstring records why: another branch was rewriting their import blocks, so a seventh edit would have conflicted with two pending pull requests. That branch is no longer in flight (no open pull request touches these files; last change 2026-09-30 / 2026-10-04), so the declaration is now the gap: a local copy can drift from the owner while the guard stays green.Observable before -> after, with the validation row that proves it: before,
adapters.py,archive.py,audience.pyandbindings.pyeach compiled^[a-z][a-z0-9_.-]{0,127}$, and the guard tolerated exactly one site per file. After, all four importPUBLIC_SAFE_SLUG_PATTERN, the per-file declaration budget is empty, andtest_every_module_that_imports_the_owner_is_a_declared_consumerpins the importer set. Mutation: restoring the local copy inaudience.pyturns three guard tests red.Issue/task and intended base: Related to [Task]: Semantic vocabulary convergence follow-ups after RFC PR #4433 / 语义词表收敛后续任务 #4447. Base
mainat d95839c (rebased from 5853e15 after fix(workspace): reuse shared source namespace digest rule #6925).Author Declaration
Implemented against
tests/architecture/test_public_safe_slug_owner.pyat 5853e15 ("Converting one means deleting its entry, and a declared file whose count moves fails the guard either way") and [Task]: Semantic vocabulary convergence follow-ups after RFC PR #4433 / 语义词表收敛后续任务 #4447 Track A.loopx/capabilities/periodic_report/{adapters,archive,audience,bindings}.pytest_consumer_holds_the_owner_object_and_applies_itDECLARED_INDIVIDUAL_SITES,loopx/public_safe_text.pytest_every_slug_site_is_the_owner_or_a_converted_consumertest_every_module_that_imports_the_owner_is_a_declared_consumerruff format --checkat this base revision.Scope And Continuation
Validation
staticpassedruff checkon the six changed files; configuredmypy(17 source files) clean;loopx check --scan-pathon each changed path reports a clean public boundaryunitpassedpytest -q tests/architecture/test_public_safe_slug_owner.py46 passed;pytest -q tests/capabilities/test_periodic_report*.py tests/architecture/test_public_safe_slug_owner.py449 passed;tests/architecture/test_control_plane_import_boundaries.py+ guard 72 passedregression_paritypassedaudience.py-> 3 red; inline literal at theadapters.pycall site -> 3 red; one consumer entry dropped -> 1 red (reconciliation test)manualpassed_tokenhelpers still lower-case and bound the value before applying the owner's pattern_tokenhelpers, which now call the owner object; the folded-value scan covers spellings this change does not use. Residual:ruff format --checkalready failed onadapters.pyand on the guard file at this base revision, and those pre-existing spans are left untouched rather than mixed into this diff.Frontend / Visual Evidence
Type of Change
LoopX Area
Technical Direction
Shared-authority RFC fixture impact
Boundary Checklist
none.Signed-off-bytrailer (git commit -s).