From 11fa1e2d26f970246d9a6a8c22fe7b7e4f00362e Mon Sep 17 00:00:00 2001 From: XingfenD Date: Sat, 3 Oct 2026 10:36:58 +0800 Subject: [PATCH] docs: re-pin the P14 spec after task-review adjudication MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The task-level review rated the split PASS with notes and found that the architecture guard did not cover one of the three purposes spec D-J states for it. Six places are corrected here before the branch review reads the spec, so the reviewer works against a frozen target rather than a moving one. - D-J is amended from three assertions to seven. The original three each have teeth (verified by mutation) but together they missed an entire dimension: nothing enumerated the composition root's own method set, so adding a method there left all three assertions PASS, go build/vet clean, and the full 11-package suite green. The god object could regrow silently. - The mutation list gains d through h, and mutation (a) is annotated with the bypass the review found in my own positive control: (a) turns red because it REMOVES CoverURL from CatalogService, not because anything detects the extra root method. Rewritten as a copy that keeps the original — the shadowing form (e) — mutation (a)'s design would have passed green. - Two fixes the review proposed are recorded as rejected, with the spike measurements, so they are not retried: asserting NumMethod()==0 on the value type is unsatisfiable because embedding *T puts its methods in both method sets (the legal tree already reports 22 — vacuously false, the mirror image of P13's vacuously true assertion), and Method.Func identity cannot detect shadowing because promoted methods are forwarding wrappers that already differ on the legal tree (5153408 vs 5148192 unshadowed, 5148288 vs 5148192 shadowed). - Line counts are pinned to the wc -l convention, verified against the same convention used for the 856 baseline and auth.go's 234. The metrics table gains a measured column: content.go 82, largest of the six files 298 (moderation.go, not submission.go as the spec predicted). - D-D records that ErrSubmissionConflict is also used by AccountService at account.go:136, outside the submission and moderation lines, which makes the SHARED ruling necessary rather than merely correct. - The risk table gains two Route C properties that were previously only in controller notes: the root has no named fields so s.objects is an ambiguous selector (a compile-time barrier earlier than the guard, and the reason the guard is the ONLY barrier against adding a concrete field), and go vet stays silent on a root method shadowing a promoted one (vet_exit=0 measured), which is why assertion 5 cannot be replaced by a reflect-based check. --- ...-10-03-p14-content-service-split-design.md | 51 ++++++++++++++----- 1 file changed, 39 insertions(+), 12 deletions(-) diff --git a/docs/specs/2026-10-03-p14-content-service-split-design.md b/docs/specs/2026-10-03-p14-content-service-split-design.md index 5150adf..b63601b 100644 --- a/docs/specs/2026-10-03-p14-content-service-split-design.md +++ b/docs/specs/2026-10-03-p14-content-service-split-design.md @@ -119,13 +119,19 @@ python AST 级扫描 `content.go` 内部调用图：同线内部调用 9 处，* | **D-A** | 拆分路线 | **Route C：组合根嵌入五个子 service + handler 侧收窄接口**。`ContentService` 保留为**组合根**（5 个嵌入指针字段、自身零方法），五个子 service 各持自己需要的依赖；五个 handler 的 `svc` 字段类型改为**消费方定义的窄接口** | §1.5 spike H1–H4 实测；H2 使 19 测试点 + serve.go 五处装配零改动，H3 使 catalog.go 的 nil 占位消失。**facade 与窄接口两条路线的收益同时拿到，不是二选一** | | **D-B** | 文件布局 | `content.go`（组合根 + 共享声明）+ `catalog.go` + `reaction.go` + `upload.go` + `submission.go` + `moderation.go`，共 **6 个文件**；命名与仓内惯例一致（`account.go`→`AccountService`、`bundle.go`→`BundleService`、`auth.go`→`AuthService`、`cleanup.go`→`CleanupService`） | §1.4 五条线 + §5c 归属映射 | | **D-C** | 子 service 构造签名 | `NewCatalogService(store, objects)`、`NewReactionService(store)`、`NewUploadService(store, users, objects, bundle)`、`NewSubmissionService(store, objects)`、`NewModerationService(store, users, objects)`。**每个只收它实测用到的字段**（§1.4），不多收 | §1.4 依赖映射；先例 `NewBundleService` 只收两个子仓储 | -| **D-D** | 私有 helper 归属 | `randomToken` → `upload.go`；`versionFromUpload` + `pendingKeys` → `moderation.go`；`ParseSubmissionEnvelope` + `SubmissionEnvelope` → **留 `content.go`（SHARED，且 handler 依赖须保持导出）**；`ErrUnsupportedMediaType` + `allowedCoverTypes` → `upload.go`（**独占**，实测仅 256/258 使用）；其余 11 个共享错误/类型 → 留 `content.go` | §5c 归属映射（含我对两处误归属的纠正记录） | +| **D-D** | 私有 helper 归属 | `randomToken` → `upload.go`；`versionFromUpload` + `pendingKeys` → `moderation.go`；`ParseSubmissionEnvelope` + `SubmissionEnvelope` → **留 `content.go`（SHARED，且 handler 依赖须保持导出）**；`ErrUnsupportedMediaType` + `allowedCoverTypes` → `upload.go`（**独占**，实测仅 256/258 使用）；其余 11 个共享错误/类型 → 留 `content.go` | §5c 归属映射（含我对两处误归属的纠正记录）。**RE-PIN 补记**（审查者 M-8 实测）：`ErrSubmissionConflict` 除投稿/审核两线与 `handler/submissions.go` 外，还被**两线之外的 `AccountService`（`account.go:136`，账号注销的 DeletionReport 路径）**使用——把它降级为小写即 `account.go:136:29: undefined: ErrSubmissionConflict` 编译红。故其 SHARED 判定**不仅正确而且必要** | | **D-E** | handler 窄接口 | 五个 handler 各自在**自己的文件内**声明消费方接口（`catalogPort`/`reactionPort`/`uploadPort`/`submissionPort`/`moderationPort`，小写=包内私有），只列它实测调用的方法；struct 字段与构造参数类型改为该接口 | §1.2 零交叉；先例 `cmd/catalog.go:79 catalogRenderer`。**接口定义在消费方**（Go 惯例），不放 service 包 | | **D-F** | `Approve` 170 行 | **本批不拆**。`moderation.go` 255 行在可接受范围；`Approve` 的 7 阶段内部重构（① 加载+状态校验 606–619 ② owner 查取 622–628 ③ 上传解析 630–644 ④ 重复性检查 switch 646–659 ⑤ 对象 Copy 661–681 ⑥ `WithTx` 事务 switch 683–759 ⑦ pending key 清理 + slog 766–772）**登记挂账**，留下一批（代码优雅维度） | 一批一个关注点。混入会让 diff 与审查面翻倍；P13 经验证明小而聚焦的批次审查质量更高。7 阶段映射已取证固化，下批可直接用 | | **D-G** | 行为不变契约 | 纯结构重构：**零行为变更**。验收 = ① 四门全绿（gofmt/vet/build/`go test ./...` 11 包）② **既有测试断言零修改**（只允许因构造函数/类型变化而改装配代码，不允许改任何 `assert`/`expect` 语义）③ 公开 API 语义不变（22 个方法签名逐字不变，10 个共享声明保持导出）④ HTTP 行为逐字节不变 | 重构的定义。既有 11 包测试就是本批的守卫 | | **D-H** | 死字段 | 删除 `ContentService.now`（§1.7）。`NewContentService` 的 **4 参数签名不变** → 19 个测试构造点与 2 个生产构造点**零改动** | §1.7 实测零引用 | | **D-I** | `cmd/catalog.go` 依赖面 | 改为直接构造 `service.NewCatalogService(store, objects)` 喂 `handler.NewGames(...)`，**删除 `nil` bundle 占位与不需要的 `users` 依赖**（依赖槽 4 → 2） | §1.5 H3；现状注释「bundle 不参与导出」正是 god object 代价的自白 | | **D-J** | 架构守卫（RED 先行） | 新增 `internal/service/architecture_test.go`，用 **reflect 钉住拆分结构**：① `ContentService` 恰有 5 个字段、**全部匿名（嵌入）、全部指针**、类型名恰为五个子 service ② 每个子 service 的**导出方法名集合**逐字等于其职责线的预期集合 ③ `ContentService` **无名为 `now` 的字段** | 防「方法迁回组合根」「重新长出具体字段」「死字段复活」。P12/P13 先例：守卫任务提前为 TDD 第一步，RED 输出本身即「有牙」证明 | + +> ⚠️ **RE-PIN（2026-10-03，任务级审查 F1 important）**：原 D-J 的**三条断言与其自述目的不匹配**——三项里的「防方法迁回组合根」**没有任何一条断言覆盖**（断言①只数字段、②只看子 service、③只查字段名）。实测（审查者 M-3b + 控制者独立复现）：在组合根上新增方法后**三断言全 PASS、`go build`/`go vet` 绿、全量 11 包测试也全绿** → god object 可静默回潮，零机械信号。另实测 `go vet` **不报告**组合根方法对提升方法的遮蔽（M-9，`vet_exit=0`）。 +> +> **D-J 增订为七类断言**（④–⑦ 为 re-pin 后补，已由 commit `e195be0` 交付）：④ `ContentService` 的**指针**方法名集合恰为五线导出方法之并（22 个）——抓「组合根新增方法」⑤ **源码扫描**：包内非测试 `.go` 里不得存在任何以 `ContentService` 为接收者的方法声明——**这是唯一能抓「遮蔽提升方法」的机械手段**（遮蔽后反射方法名集合不变、`go vet` 沉默）⑥ 五个子 service 的**字段名集合**逐字等于其依赖面（§1.4）——把「无可变内部状态」这个 §0 赖以论证可拆分的不变量变成机器契约（原断言③只禁字面名 `now`，故 `clock`/`logger`/`cache` 任何名字都能静默通过：钉的是名字而非不变量）⑦ 实例化 `NewContentService(nil, nil, nil, nil)` 后断言五个嵌入指针**均非 nil**——原断言①是纯静态类型检查，故构造器漏装某条线时 build/vet/守卫全绿，故障以运行时 nil 解引用 panic 出现（M-7）。 +> +> **两条被 Go 提升规则 spike 实测否决的修法**（勿再尝试，理由已写入 `architecture_test.go` 注释）：(i) 「断言 `reflect.TypeOf(ContentService{}).NumMethod() == 0`」**不成立**——嵌入的是 `*T`，其方法**同时进入值类型与指针类型的方法集**，故合法树上值类型 NumMethod 已是 22，该断言**恒假**（P13 恒真断言的镜像形态）。(ii) 「比 `Method(i).Func` 的同一性以侦测遮蔽」**不成立**——提升方法本身是 forwarding wrapper，合法树上 `root.CoverURL.Func`(5153408) 与 `CatalogService.CoverURL.Func`(5148192) **已不同**，遮蔽后为 5148288，两侧都 false → **无法区分遮蔽与否**。 | **D-K** | 派发形态 | **一个实现者子代理**做完 T1→T3。所有任务同动 `crearte-server` 单一工作树与 git index | AGENTS.md / `sdd-parallel-dispatch` §1「一仓一写者」；P12/P13 同款 | ### §2.1 被否方案 @@ -263,7 +269,7 @@ func NewGames(svc catalogPort) *Games { ### T1 —— 架构守卫（**RED 先行**）：`internal/service/architecture_test.go`（新增） -用 reflect 钉住拆分结构（D-J）。三类断言： +用 reflect 钉住拆分结构（D-J）。**七类断言**（①–③ 为原设计、④–⑦ 为 RE-PIN 后补，见 D-J 的增订说明）： 1. **组合根形态**：`reflect.TypeOf(service.ContentService{})` 恰有 **5 个字段**，每个 `Anonymous == true`（嵌入）、`Kind == reflect.Ptr`，且 `Elem().Name()` 的集合恰为 `{CatalogService, ReactionService, UploadService, SubmissionService, ModerationService}`。 2. **各线导出方法名集合**（逐字，防方法迁移）： @@ -273,6 +279,10 @@ func NewGames(svc catalogPort) *Games { - `SubmissionService` = `{CreateSubmission, ListMine, GetSubmission, UpdateSubmission, DeleteSubmission}` - `ModerationService` = `{ListQueue, Approve, Reject, Unpublish, Republish, UpdateWorkFeatures}` 3. **死字段不复活**：`ContentService` 与五个子 service **都不得有名为 `now` 的字段**（D-H）。 +4. **组合根方法集恰为提升之并**：`reflect.TypeOf(&ContentService{})` 的方法名集合逐字等于断言 2 五个集合的并（22 个）。**必须用指针类型**（子 service 方法全是指针接收者）。抓「组合根新增方法」。 +5. **源码不得声明组合根方法**：扫描包内非测试 `.go`，任何匹配 `^func\s+\(\s*\w+\s+\*?ContentService\s*\)` 的行都是违规。**这是唯一能抓「遮蔽提升方法」的手段**（遮蔽后断言 4 的名字集合不变、`go vet` 也沉默）。仓内有先例：`bundle/vector_test.go`、`cmd/catalog_test.go` 都用 `os.ReadFile`。 +6. **各线字段名集合**：五个子 service 的字段名逐字等于 §1.4 的依赖面——`CatalogService={objects,store}`、`ReactionService={store}`、`UploadService={bundle,objects,store,users}`、`SubmissionService={objects,store}`、`ModerationService={objects,store,users}`。抓「重新长出内部状态」与「依赖面被悄悄放宽」。 +7. **构造器不漏装**：`NewContentService(nil, nil, nil, nil)` 后五个嵌入指针均非 nil。传 `nil` 依赖是安全的：构造器只做纯组装、不解引用参数（既有测试已有多处以 `nil` bundle 构造，已证安全）。 **RED 相位要求**：T1 在 T2 之前提交时，`go test ./internal/service/` 必须以**编译错误**失败（`undefined: CatalogService` 等）。实现者须把该输出**逐字存证**到 `crearte-server/.superpowers/sdd-p14/impl-evidence/t1-red.txt`。Go 的 RED 不如 TS 那样能给出断言级消息，故**编译错误本身就是 RED 证据**——但必须存档，不能只在报告里描述。 @@ -280,8 +290,19 @@ func NewGames(svc catalogPort) *Games { - (a) 把某个方法（如 `CoverURL`）从 `catalog.go` 移回 `content.go` 并改接收者为 `*ContentService` → 断言 2 必须红； - (b) 给 `ContentService` 加一个具体字段（如 `store repository.ContentStore`）→ 断言 1 必须红； - (c) 给任一子 service 加回 `now func() time.Time` → 断言 3 必须红。 +- (d) 在组合根**新增**一个方法（不移除任何子 service 方法）→ 断言 4 **与** 断言 5 必须红； +- (e) 在组合根**遮蔽**一个提升方法（**保留**子 service 原件）→ 断言 5 必须红、**断言 4 必须保持绿**（这正是断言 5 不可省的证明）； +- (f) 给任一子 service 加一个**非 `now`** 的字段（如 `clock func() int64`）→ 断言 6 必须红、断言 3 必须保持绿； +- (g) 给某条线**放宽依赖面**（如给 `CatalogService` 塞 `users`）→ 断言 6 必须红； +- (h) 从 `NewContentService` 删掉某条线的组装行 → 断言 7 必须红。 每次 mutation 后 `git checkout -- ` 恢复并核 `git diff` 空。 +> ⚠️ **RE-PIN（审查者 §12-1）：mutation (a) 的原设计有一条绕行路径。** (a) 之所以会红，是因为它同时把方法从 `CatalogService` **拿走**了（断言 2 的集合缺员），**不是因为断言侦测到组合根上多出方法**。若把它改成「**复制**一份到组合根而保留 `catalog.go` 原件」（即 (e) 的遮蔽形态），(a) 的设计就会**误绿**——这是我 spec 里 positive control 自身的一个未被发现的盲区，由 (d)(e) 补上。 +> +> ⚠️ **RE-PIN（控制者自伤教训）：mutation 验证脚本的 `restore()` 绝不能用 `git checkout -- .`。** 若 fix-forward 编辑**尚未提交**，全量回滚会把它一起抹掉，导致后续各轮**全在测一棵没有修复的树**（本会话实测：第一轮后四条新断言被抹掉、`architecture_test.go` 回到 118 行，后四轮报「绿=3」= 原始三断言数，汇总显示四条「未按预期」）。修法：`restore()` 只回滚 mutation **触及的那几个源文件**，且每轮恢复后断言守卫文件的 `func Test` 计数仍为预期值，否则 **FATAL 终止**——让脚本在被自毁时大声失败，而不是静默产出「未按预期」的假结论。 +> +> ⚠️ **遮蔽型 mutation 的签名必须逐字取原文。** 控制者第一版 (e) 用正则 `^func \(s \*CatalogService\) (Approve\(.*?\))` 抽签名，非贪婪 `.*?` 把返回值 `(error)` 截断了 → 生成的遮蔽体 `return nil` 与空返回值冲突 → **编译红而非断言红**，脚本却把它报成「守卫拦住了」。mutation 没编译过 ≠ 守卫有牙。 + ### T2 —— 提取五个子 service（T1 由红转绿） 按 §3.1–§3.2 执行。**完成后 `go test ./...` 必须 11 包全绿且既有测试文件零修改**（`git diff --stat` 里不应出现任何 `*_test.go`，除 T1 新增的那个）。 @@ -307,15 +328,19 @@ docker run --rm -v "$PWD/src:/src" -w /src \ ### 结构指标（交付时须报实测数字） -| 指标 | 基线 | 目标 | -|---|---|---| -| `content.go` 行数 | 856 | **< 120**（组合根 + 共享声明） | -| 六个文件最大行数 | 856 | **< 300**（submission.go 预计 ~290） | -| `service/` 目录内 >400 行的生产文件 | 1（content.go） | **0** | -| handler 里 `service.ContentService` 引用 | 10 处 | **0** | -| `cmd/catalog.go` 的 `nil` 占位 | 1 | **0** | -| `ContentService` 自身声明的方法数 | 25 | **0**（全部提升） | -| 既有 `*_test.go` 修改文件数 | — | **0** | +> **RE-PIN（审查者 F5）：行数口径统一为 `wc -l`**（POSIX：数换行符），与 §0 的基线 856、`auth.go` 234 同口径——已实测核对：`git show dbf7fe5:src/internal/service/content.go | wc -l` = **856**、`wc -l auth.go` = **234**。**不要用 `len(text.split('\n'))`**：文件以换行结尾时它会多算一个空段（本批实测 content.go `wc -l`=82 而 split=83，六文件最大 `wc -l`=298 而 split=299，控制者据此报错过一轮数字）。本批两种口径都远低于目标，无实际影响；但若将来某文件恰落在边界（如 299 vs 300），口径歧义会直接决定 pass/fail。 + +| 指标 | 基线 | 目标 | 实测（`wc -l` 口径） | +|---|---|---|---| +| `content.go` 行数 | 856 | **< 120**（组合根 + 共享声明） | **82** ✅ | +| 六个文件最大行数 | 856 | **< 300**（submission.go 预计 ~290） | **298**（`moderation.go`，非 spec 预估的 submission.go）✅ | +| `service/` 目录内 >400 行的生产文件 | 1（content.go） | **0** | **0** ✅ | +| handler 里 `service.ContentService` 引用 | 10 处 | **0** | **0** ✅ | +| `cmd/catalog.go` 的 `nil` 占位 | 1 | **0** | **0** ✅（`NewPostgresUserStore` 亦归零） | +| `ContentService` 自身声明的方法数 | 25 | **0**（全部提升） | **0** ✅ | +| 既有 `*_test.go` 修改文件数 | — | **0** | **0** ✅（仅新增 `architecture_test.go`） | + +六文件实测行数（`wc -l`）：`content.go` 82 · `catalog.go` 110 · `reaction.go` 102 · `upload.go` 99 · `submission.go` 291 · `moderation.go` 298。 --- @@ -339,7 +364,9 @@ docker run --rm -v "$PWD/src:/src" -w /src \ | **import 块照抄导致 `imported and not used`** | §3.2 规则 4：每个文件只列实际用到的包。`go build` 会强制暴露 | | **`time` import 在 content.go 变悬空**（删 `now` 后） | §3.1 已点名，但**要求实现者自行 grep 确认**而非凭 spec 推断 | | **测试被迫修改 = 设计失败的信号** | §4 末条 + T3 验收硬指标「既有 `*_test.go` 修改文件数 = 0」。若实现者发现必须改测试，**停下来报告**而不是改 | -| **架构守卫本身写成败 assertion** | T1 的三条 mutation 自查（a/b/c）必须附红/绿输出。P13 教训：`max(a,b) ≥ k` 形态恒真、positive control 只钉一个子树 → **守卫自身要受同等审视** | +| **架构守卫本身写成败 assertion** | T1 的 mutation 自查（a–h）必须附红/绿输出。P13 教训：`max(a,b) ≥ k` 形态恒真、positive control 只钉一个子树 → **守卫自身要受同等审视**。**P14 实证了这条风险的两种形态**：(i) 恒真（P13）与恒假（本批被否决的 `NumMethod()==0` 修法）都要防；(ii) **守卫覆盖不全**比恒真更隐蔽——本批三条断言各自都真有牙（M-1b/M-2/M-5 双向验证实证），但合起来漏掉了「组合根长方法」整个维度，且 spec 自己的 positive control (a) 有绕行路径（见 §5-T1 的 RE-PIN） | +| **RE-PIN：组合根上直接访问依赖会编译失败（Route C 的额外性质，非缺陷）** | 组合根 `ContentService` **零具名字段**，故 `s.store` / `s.users` / `s.objects` / `s.bundle` 在组合根上都是 **ambiguous selector**（`catalog`/`upload`/`submission`/`moderation` 四个子 service 都有 `objects`，Go 提升规则下深度相同即歧义）。要访问须限定为 `s.CatalogService.objects` 这种形式。**这是比架构守卫更早的一道防线**：往组合根直接摸依赖的新代码编译期即失败。反过来也解释了为何 mutation (b) 加**具体字段**后守卫是唯一防线——加具体字段本身在 Go 里可编译（不与嵌入冲突） | +| **RE-PIN：`go vet` 不报告组合根方法对提升方法的遮蔽** | 审查者 M-9 实测 `vet_exit=0`：在组合根上写一个与提升方法同名同签名的方法（Go 深度规则下 depth 0 胜出）会**静默接管**全部调用，lint 层零信号。故断言 5（源码扫描）不可省——反射方法名集合在遮蔽后不变、断言 4 会保持绿（已由 mutation (e) 实证）。**这两条同属「组合根一旦长出代码就静默出问题」的同一风险面**，也是 §2.1 方案 B（将来若删除组合根）的收尾条件之一：只要组合根存在，就必须有断言 4+5 守着它 | ---