37a0a8ddd13e45d33ccbdaaf2977ed96c26a9124
3
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 XingfenD
|
33c2d8d4b8 |
docs: re-pin the P14 spec after branch-review adjudication
The branch review approved with notes but overturned two universal claims I had already committed to the spec. Both overturns were independently reproduced before acting on them, and both were correct. FR-1 (important): assertion 5's source scan required a NAMED receiver, but Go permits omitting an unused one, and a shadowing body is exactly the case that often needs none. Under func (*ContentService) CoverURL(...) the old pattern matched nothing, so build, vet and all seven assertions stayed green while the shadow was effective. The regex literal in §5-T1.5 is corrected to the optional group now in the code, and the claim that assertion 5 is the only mechanical way to catch shadowing is scoped: it was false before the fix, and after it the assertion's reach is wider than the original text said, because NumMethod() exposes only exported names — so an unexported root method is also caught by the source scan alone. FR-5: my adjudication of F4 said the two layers 'cover completely'. That is a universal claim and an orphan private helper on the wrong line is its counterexample (assertion 2 filters on IsExported, and Go does not report unused methods). The wording is narrowed to what the layers actually cover, with the reason the verdict still holds: the third layer can only ever produce dead code. Both are recorded as instances of the same defect I keep committing — stating a conclusion before establishing its range. §8 rule 5 already required universal claims to have universal-range evidence; these were two places I did not follow my own rule. The lesson is written into the spec rather than only the ledger: when claiming a guard is the only thing that catches a failure mode, enumerate the forms first (named/unnamed receiver, value/pointer, exported/unexported), or the claim becomes the next reviewer's counterexample. |
||
|
XingfenD
|
11fa1e2d26 |
docs: re-pin the P14 spec after task-review adjudication
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. |
||
|
XingfenD
|
ba1fe3ecb0 |
docs: P14 design + plan for splitting the ContentService god object
Registers the next batch before implementation starts, so the design is versioned and reviewable rather than living only on disk. Scope: crearte-server only. content.go is 856 lines / 30 functions, the sole outlier in internal/service (next largest production file auth.go is 234 lines; content.go alone is 22% of the directory) and carries five unrelated responsibility lines in one struct whose five fields are all shared repository dependencies with no mutable internal state. Three survey findings make the split low-risk rather than aspirational: - the seven cross-line calls all target package-level helper functions, with zero method-to-method cross-line calls, so splitting creates no cross-service callbacks and no import cycle; - each of the five handlers uses exactly one line's methods with zero overlap, so each dependency face narrows from 22 methods to its own 2-6 with no adapter; - 11 of the 19 test construction sites only construct and never call methods, and a Go embedding spike (H1-H4, run in golang:1.24-alpine, vet clean) confirmed the promoted method set satisfies consumer-defined narrow interfaces — so the composite root keeps every construction site and all five serve.go wirings unchanged while handlers still narrow. The survey also found ContentService.now is a dead field: zero s.now references in the file, no test seam injecting it, while the sibling services that share the pattern do use theirs (auth.go reads s.now(), cleanup.go has SetNow). It is removed as part of the split rather than being assigned a line. Two risks were falsified by measurement before designing: no code inside the package reads ContentService's private fields (AccountService holds repository.ContentStore, not *ContentService, so it is untouched), and the type is never interface-ised, type-asserted, or used as a method value. Deliberately out of scope: the 170-line Approve function (its seven phases are mapped and logged for a later batch — one concern per batch), memory_content.go (814 lines but a test double with zero non-test references), and handler error-mapping dedup (92 WriteError sites but only 2 errors.Is checks, so the duplication does not justify itself). Verification plan: four gates in the dockerized toolchain plus structural metrics (content.go under 120 lines, largest of the six files under 300, zero service.ContentService references left in handlers, zero existing test files modified) and three mutations the new architecture guard must fail on. |