feat(rules): bootstrap 3 doc families (service-impl, k8s-crd, functional-options)

[bborbe]

Summary

Second multi-doc bootstrap PR after PR #17 validated the pattern. 3 docs (~500-770 lines), 6 rules, single bot review cycle.

`rules/index.json`: 62 → 68 entries (3 new families).

Rules added

family / id	level	owner
`go-service-impl/no-context-object-injection`	MUST	go-architecture-assistant
`go-service-impl/provider-vs-registry-choice`	SHOULD	go-architecture-assistant
`go-k8s-crd/use-bborbe-k8s`	SHOULD	go-architecture-assistant
`go-k8s-crd/generated-client-not-dynamic`	MUST	go-architecture-assistant
`go-functional-options/singular-option-type`	SHOULD	go-quality-assistant
`go-functional-options/with-prefix-option-functions`	SHOULD	go-quality-assistant

Notable from this PR

• `go-k8s-crd/use-bborbe-k8s` and `generated-client-not-dynamic` are about taking the right abstraction (bborbe/k8s generic primitives) vs. the wrong one (hand-written boilerplate / dynamic client). These are the kind of rule that prevents 300-line PRs reinventing the same race conditions.
• `go-service-impl/provider-vs-registry-choice` is a sizing rule, not a binary rule — closed set → switch, open set → registry. Both shapes are legitimate; the rule is "match the shape to the problem."
• CLAUDE.md doc-agent table updated for all 3 new docs.

Pre-push checks

• `grep` clean for personal paths
• One trading-domain leak caught in my own example (`ProcessMarket/Limit/Stop` — order-type terminology) and fixed before push (`ProcessImage/Video/Document`). Pre-existing `OrderType` references in the service-impl doc are NOT touched in this PR — out of scope; bot may flag them as pre-existing.
• All 6 rule IDs unique against 62 existing entries
• All 3 owner agents exist
• `make build-index` regenerated; `check-index` passes (PR feat(precommit): add check-index target to catch walker-output drift #13 drift guard)

Test plan

• `make precommit` clean
• 6 entries in index with consistent schema
• Pre-emptive grep clean (post-fix for my own ProcessMarket leak)
• Bot review

[ben-s-pull-request-reviewer]

Now I have all the data needed for the consolidated report. Let me compile the final JSON verdict:

{
  "verdict": "request-changes",
  "summary": "The PR adds 3 new doc families with 6 new rules correctly registered in rules/index.json and mapped in CLAUDE.md. However, two critical issues were found: the go-service-implementation-patterns.md examples use raw http.HandlerFunc return types that contradict the established go-http-handler-refactoring-guide.md (which requires libhttp.WithError), appearing in both the Bad and the Good example sections; and the Factory Pattern Implementation example at lines 301-308 has a non-exhaustive switch without a default case that returns (nil, nil), creating a nil panic risk. Additionally, CreateSaramaConfig at docs/go-functional-options-pattern.md:108 uses the Create* factory prefix while returning an error and containing a loop in the body — violating the very factory rules this doc is teaching.",
  "comments": [
    {
      "file": "docs/go-service-implementation-patterns.md",
      "line": 456,
      "severity": "critical",
      "message": "go-service-impl/MUST: The 'Bad' example at line 456 returns http.HandlerFunc, which directly contradicts docs/go-http-handler-refactoring-guide.md (lines 46-77, 282-318) that requires libhttp.WithError for HTTP handlers. Both Bad and Good examples in this section return raw http.HandlerFunc — the Good example should return libhttp.WithError and be placed in pkg/handler/ per the established HTTP handler refactoring guide."
    },
    {
      "file": "docs/go-service-implementation-patterns.md",
      "line": 500,
      "severity": "critical",
      "message": "go-service-impl/MUST: The 'Good' example at line 500 returns http.HandlerFunc, directly contradicting the go-http-handler-refactoring-guide.md which mandates libhttp.WithError (or libhttp.WithErrorTx for DB transactions). The section presents this as the correct pattern but it violates the HTTP handler architecture rules. Fix: use libhttp.WithError return type and place in pkg/handler/ with factory wiring."
    },
    {
      "file": "docs/go-service-implementation-patterns.md",
      "line": 301,
      "severity": "critical",
      "message": "go-service-impl/no-context-object-injection MUST: The switch in 'Factory Pattern Implementation' lacks a default: case returning an error. For a function returning (UserService, error), omitting default causes the compiler to infer return nil, nil — a nil panic at runtime when an unknown UserType is passed. The 'Good' example at lines 82-91 correctly uses default: errors.Errorf; the Factory Pattern Implementation example does not."
    },
    {
      "file": "docs/go-functional-options-pattern.md",
      "line": 108,
      "severity": "major",
      "message": "go-factory-pattern MUST (go-factory/no-error-return + go-factory/no-conditional-in-body): CreateSaramaConfig uses the Create* prefix (factory convention) while returning error AND containing a for loop to apply options. This violates two MUST-level factory rules. Since the doc teaches the functional-options pattern to readers who may copy this into pkg/factory/, they will trip both rules. Rename to NewSaramaConfig (constructor convention) or clarify this is not a factory.go function."
    },
    {
      "file": "docs/go-kubernetes-crd-controller-guide.md",
      "line": 6,
      "severity": "major",
      "message": "go-k8s-crd/use-bborbe-k8s SHOULD: The RULE block is inserted at the very top of 'Section 0. Before you start' before the bborbe/k8s API bullet list (line 32) that explains what k8s.NewEventHandler and k8s.NewResourceEventHandler actually are. The Good example calls these functions before the reader knows what they do. Move the RULE block to after the API list, or add a forward reference."
    },
    {
      "file": "docs/go-kubernetes-crd-controller-guide.md",
      "line": 85,
      "severity": "major",
      "message": "go-k8s-crd/generated-client-not-dynamic MUST: The RULE block is placed as a subsection of '## 2. Repository layout' (CRD library layout) but the rule applies to the consumer/ controller side, not the CRD library. This misplacement will confuse readers about which part of the repo the rule targets. Reposition to the consumer service section."
    },
    {
      "file": "docs/go-functional-options-pattern.md",
      "line": 519,
      "severity": "major",
      "message": "go-functional-options/singular-option-type SHOULD: The enforcement description uses 'wrong-singular pattern' and 'violates' language that implies a definitive error, but the doc's own pre-existing content (line 588) marks the same pattern as 'Acceptable but less clear' — a preference, not a violation. The SHOULD level is correct but the wording is too strong. Use 'suboptimal naming' instead of 'wrong-singular pattern' to align with the doc's existing stance."
    },
    {
      "file": "docs/go-service-implementation-patterns.md",
      "line": 212,
      "severity": "major",
      "message": "go-service-impl/no-context-object-injection MUST: The Enforcement description says '*<Name>Context' / '*<Name>Deps' (pointer types only) but the Bad example passes svcCtx ServiceContext as a value, not a pointer. The enforcement would miss the value-passed case shown in the example. Either update enforcement to also match value types, or update the Bad example to pass *ServiceContext to match the enforcement description."
    },
    {
      "file": "docs/go-functional-options-pattern.md",
      "line": 203,
      "severity": "nit",
      "message": "go-functional-options/singular-option-type SHOULD: The 'Options with Validation' example returns func(*ConsumerOptions) as an inline type, but the new singular-option-type RULE promotes custom named types like ConsumerOption. Multiple existing examples in the 'Advanced Patterns' section (lines 203, 220, 232, 245, 268, 691, 697, 703) use inline func(*XxxOptions) instead of the named XxxOption type. Consider using named types consistently in the 'Good' examples to model the recommended pattern."
    },
    {
      "file": "docs/go-functional-options-pattern.md",
      "line": 550,
      "severity": "nit",
      "message": "go-functional-options/with-prefix-option-functions SHOULD: The Enforcement mentions function_declaration returning *Option or *ConfigFunc, but the Applies when also mentions bare nouns like BatchSize(n) with no suffix. The enforcement description doesn't explain how bare nouns are distinguished from unrelated functions that return a func type. Clarify in the enforcement description how bare nouns are detected."
    }
  ],
  "concerns_addressed": [
    "correctness: rules/index.json - 6 new rules verified: all IDs unique against 62 prior entries, all owner agents (go-architecture-assistant, go-quality-assistant) exist in agents/ directory — CONCERN ADDRESSED",
    "correctness: docs/go-service-implementation-patterns.md - go-service-impl/no-context-object-injection ast-grep pattern correctly identifies struct-field-bundle anti-pattern, but enforcement description has a pointer/value type mismatch with its own Bad example — RAISED as critical comment",
    "correctness: docs/go-kubernetes-crd-controller-guide.md - go-k8s-crd/generated-client-not-dynamic dynamic client smell is well-described; 'first-party CRD' boundary is clear from the enforcement description (schema known at compile time = first-party) — CONCERN ADDRESSED",
    "correctness: docs/go-functional-options-pattern.md - go-functional-options/singular-option-type ast-grep pattern on type_alias_declaration/type_declaration correctly distinguishes singular vs plural naming — CONCERN ADDRESSED",
    "tests: rules/index.json - index.json schema is consistent with existing entries; make precommit could not be verified in this environment (sandbox) — UNABLE TO VERIFY (needs CI run)",
    "correctness: CLAUDE.md - doc-agent table correctly maps go-service-implementation-patterns.md → go-architecture-assistant, go-kubernetes-crd-controller-guide.md → go-architecture-assistant, go-functional-options-pattern.md → go-quality-assistant — CONCERN ADDRESSED"
  ]
}

superseded by new automated review

[ben-s-pull-request-reviewer]

Now I have all the data needed. Let me compile the consolidated report and emit the final verdict.

---

Consolidated PR Review Report

PR: feat(rules): bootstrap 3 doc families (service-impl, k8s-crd, functional-options)
Branch: feat/bootstrap-3doc2 → master
Files changed: 5 files, 266 insertions

---

Automated Checks

• LICENSE file: Present ✅
• make precommit: Could not execute (permission denied in environment) — noted, not blocking

---

MUST FIX (Critical)

None. No critical issues found.

---

SHOULD FIX (Important)

1. docs/go-functional-options-pattern.md line ~526 — Bad example is syntactically misleading

The go-functional-options/singular-option-type Bad example shows type ConsumerOptions func(*ConsumerOptions) with the comment "and the struct is also ConsumerOptions?". In Go, you cannot have two top-level declarations with the same name in the same package — this is a compile error. The intent (name collision between type alias and struct) is not actually demonstrated by code that won't compile. A real collision would involve different names (e.g., type ConsumerOptionsFunc func(*ConsumerOptions) paired with type ConsumerOptions struct{...}). The example should be fixed or the comment clarified.

2. docs/go-service-implementation-patterns.md line ~31 — func init() in Bad example not cross-referenced

The go-service-impl/provider-vs-registry-choice Bad example uses func init() to register processors, which simultaneously violates go-architecture/no-globals-or-singletons (MUST). Readers who encounter only this rule block may miss that init() is independently a MUST violation. Consider adding a brief cross-reference note in the Bad section.

---

NICE TO HAVE (Optional)

3. docs/go-service-implementation-patterns.md line ~230 — Good example references undeclared emailService struct

The go-service-impl/no-context-object-injection Good example shows return &emailService{...} but the type emailService struct{...} definition is omitted. Adding the struct definition would make the example self-contained and better demonstrate the private-struct-matches-interface pattern.

4. All 6 new rules lack rules/go/*.yml ast-grep files

This is consistent with the repo's existing state (only ~16 of 61 Go rules have mechanical YAML enforcement), but it's worth tracking: the enforcement field for all 6 rules says "judgment" / "ast-grep follow-up", meaning no mechanical enforcement exists yet. Not actionable in this PR, but worth a future pass.

---

Manual Review Notes

• All 6 rules/index.json entries are well-formed with correct schema (7 required fields present, alphabetical ordering correct)
• anchor equals id verbatim for all 6 entries ✅
• All owner values in index.json match the Agent column in the updated CLAUDE.md alignment table ✅
• All 3 new doc rows in CLAUDE.md table are correct ✅
• No contradictions found between new doc content and existing go-http-handler-refactoring-guide.md patterns ✅

---

Verdict

This is a clean, low-risk PR. The 2 important issues are documentation-accuracy concerns (a compile-error example and a missing cross-reference), not structural or correctness bugs. The 6 new rule entries in rules/index.json are all correct and consistent.

{
  "verdict": "request-changes",
  "summary": "PR #18 bootstraps 3 new rule families (service-impl, k8s-crd, functional-options) with properly registered index.json entries and aligned CLAUDE.md table. Two SHOULD-level documentation issues were found: the Bad example in go-functional-options/singular-option-type contains code that would not compile in Go (same-name collision is a compile error), and the go-service-impl/provider-vs-registry-choice Bad example uses func init() without cross-referencing the separate no-globals-or-singletons MUST rule. Neither is a structural bug but both should be fixed before merge.",
  "comments": [
    {
      "file": "docs/go-functional-options-pattern.md",
      "line": 526,
      "severity": "major",
      "message": "go-functional-options/singular-option-type Bad example: `type ConsumerOptions func(*ConsumerOptions)` cannot compile in Go — you cannot shadow a type name with another declaration in the same package. The intent (naming collision) is correct but the example needs a real-world shape, e.g. a type alias `ConsumerOptionsFunc func(*ConsumerOptions)` paired with a struct `ConsumerOptions` — different names, same semantic collision. Fix: use two distinct identifiers that demonstrate the collision without triggering a compile error."
    },
    {
      "file": "docs/go-service-implementation-patterns.md",
      "line": 31,
      "severity": "major",
      "message": "go-service-impl/provider-vs-registry-choice Bad example uses `func init()` to register processors, which simultaneously violates `go-architecture/no-globals-or-singletons` (MUST level). Readers who encounter this rule block in isolation may not connect it to the independent MUST violation. Consider adding a cross-reference note in the Bad section, e.g. `// also violates go-architecture/no-globals-or-singletons`."
    },
    {
      "file": "docs/go-service-implementation-patterns.md",
      "line": 230,
      "severity": "nit",
      "message": "go-service-impl/no-context-object-injection Good example returns `&emailService{...}` but the `type emailService struct{...}` definition is not shown. Making the struct definition explicit would make the example self-contained and better demonstrate the private-struct-matches-interface pattern."
    }
  ],
  "concerns_addressed": [
    "correctness: rules/index.json 6 new entries verified valid schema, unique IDs, correct owner agents — all pass",
    "correctness: docs/go-functional-options-pattern.md new doc — structure correct, 2 RULE blocks well-formed (minor example issue flagged as SHOULD)",
    "correctness: docs/go-kubernetes-crd-controller-guide.md new doc — structure correct, 2 RULE blocks well-formed, no issues found",
    "correctness: docs/go-service-implementation-patterns.md new doc — structure correct, 2 RULE blocks well-formed (2 issues flagged as SHOULD/nit)",
    "correctness: CLAUDE.md doc-agent table updated — all 3 new docs properly listed with correct owner agents"
  ]
}

superseded by new automated review

[ben-s-pull-request-reviewer]

PR Review Report

PR: #18 feat(rules): bootstrap 3 doc families (service-impl, k8s-crd, functional-options)
Target: master | Source: feat/bootstrap-3doc2 (ded6b4a)
Scope: Feature bootstrap — 3 new guides + 6 new rule entries in index

---

Automated Checks

• LICENSE file: Present ✓
• llms.txt entries: All 3 new docs present ✓
• README.md tables: All 3 new docs present ✓

---

Must Fix (Critical)

None.

---

Should Fix (Important)

File	Line	Issue
docs/go-service-implementation-patterns.md	19	go-service-impl/provider-vs-registry-choice rule says applies_when is "a fixed compile-time set (< ~10 types)". The ~10 qualifier is ambiguous — it's unclear whether this means count-based or compile-time-extensibility-based. The prose (line 21) clarifies "closed set → switch; open set → registry" which is the right semantic distinction; the < ~10 types parenthetical adds confusion. Recommend removing the count qualifier entirely and relying solely on the closed/open set characterization.

---

Nice to Have (Optional)

File	Line	Issue
docs/go-kubernetes-crd-controller-guide.md	360	Mentions ptr.To() from k8s.io/utils/ptr but the guide doesn't import or show usage of this package. Since this is a newly bootstrapped doc, consider adding a brief import note or inline example showing ptr.To(true) for the XPreserveUnknownFields call.
docs/go-functional-options-pattern.md	588–646	The "Option Type Names" section covers 4 alternative naming suffixes (XxxModifier, XxxConfigFunc, etc.) beyond the recommended XxxOption. This section is informative but adds length to an already-long doc. Consider whether alternatives belong in the main doc or a separate reference section to keep the primary guidance focused.

---

Plan Concerns — Resolution Status

Concern	Status
rules/index.json: 6 new entries (62→68), IDs unique, schema consistent	Resolved — all 6 IDs unique, all owners (go-architecture-assistant, go-quality-assistant) exist
rules/index.json: verify 3 owner agents exist	Resolved — both owners present in agents/
docs/go-functional-options-pattern.md: singular/plural conventions correctly described	Resolved — singular XxxOption / plural XxxOptions pair correctly documented throughout
docs/go-kubernetes-crd-controller-guide.md: bborbe/k8s abstraction recommendations	Resolved — k8s.Type, k8s.EventHandler[T], k8s.NewResourceEventHandler[T] primitives documented with correct shapes
docs/go-service-implementation-patterns.md: provider-vs-registry sizing rule	Resolved — rule correctly states "closed set → switch; open set → registry"
CLAUDE.md: doc-agent table updated with 3 new docs	Resolved — all 3 docs properly mapped to their owner agents
rules/index.json: index well-formed JSON	Resolved — valid JSON, 68 entries

---

Summary

The PR correctly bootstraps 3 new documentation families and registers 6 new rules in rules/index.json. All doc-agent mappings in CLAUDE.md, README.md, and llms.txt are accurate. One rule (go-service-impl/provider-vs-registry-choice) has an ambiguous < ~10 types> qualifier that should be removed to avoid confusion about whether the rule is count-based vs. extensibility-based.

{
  "verdict": "approve",
  "summary": "PR #18 correctly bootstraps 3 new doc families (service-impl, k8s-crd, functional-options) with 6 new rule entries in rules/index.json. All doc-agent mappings in CLAUDE.md, README.md, and llms.txt are accurate and consistent. One nit: the go-service-impl/provider-vs-registry-choice rule's '< ~10 types' qualifier is ambiguous and should be removed in favor of the clearer 'closed set vs open set' prose already present.",
  "comments": [
    {
      "file": "docs/go-service-implementation-patterns.md",
      "line": 19,
      "severity": "major",
      "message": "go-service-impl/provider-vs-registry-choice: remove '< ~10 types' qualifier from applies_when — the rule's prose already correctly characterizes the decision as 'closed set → switch; open set → registry', which is the right semantic. The count-based qualifier creates ambiguity about whether the rule is triggered by cardinality or by compile-time vs runtime extensibility."
    }
  ],
  "concerns_addressed": [
    "correctness: rules/index.json — 6 new entries (62→68) have unique IDs and consistent schema; both owner agents exist",
    "correctness: docs/go-functional-options-pattern.md — singular XxxOption / plural XxxOptions pair correctly described throughout",
    "correctness: docs/go-kubernetes-crd-controller-guide.md — bborbe/k8s abstraction recommendations are accurate (k8s.Type, k8s.EventHandler[T], k8s.NewResourceEventHandler[T])",
    "correctness: docs/go-service-implementation-patterns.md — provider-vs-registry rule correctly states 'closed set → switch; open set → registry'",
    "correctness: CLAUDE.md — all 3 new docs properly listed in doc-agent table with correct owner agents",
    "tests: rules/index.json — valid JSON, 68 well-formed entries"
  ]
}

[ben-s-pull-request-reviewer]

Based on the comprehensive review (4 concurrent agents + automated checks), here is the consolidated report:

---

PR Review: feat(rules): bootstrap 3 doc families

Diff Summary

• 4 files changed: 3 Markdown docs + 1 JSON rules index
• +71 −51 lines — documentation refinements, no Go source changes
• Scope: docs-only; this is a CLAUDE.md plugin repo

Automated Checks

• ✅ LICENSE: exists at root with current year
• ✅ Precommit target: exists (would validate JSON syntax and links)

Agent Review Results

Agent	Verdict
go-quality-assistant	Clean — JSON valid, Go snippets correct, links valid
go-factory-pattern-assistant	Clean — no violations, NewEmailService is textbook correct
go-test-coverage-assistant	Clean — rule explanations improved, no coverage gaps for doc-only changes
license-assistant	Clean — LICENSE present and properly formatted

Findings

None. The diff is documentation-only with clean prose, correct Go snippets, and a properly synchronized rules/index.json.

---

{
  "verdict": "approve",
  "summary": "Clean documentation-only PR that refines 3 existing rule families (functional-options, k8s-crd, service-impl) with clearer applies_when language, better bad/good examples, and proper rule relocation in the k8s-crd guide. rules/index.json is in sync with all doc changes. No issues found.",
  "comments": [],
  "concerns_addressed": [
    "correctness: ast-grep patterns in docs — all Go snippets verified syntactically correct by go-quality-assistant",
    "correctness: no-context-object-injection pointer/value scope — addressed: applies_when now explicitly covers both shapes",
    "correctness: generated-client-not-dynamic consumer scope — addressed: rule relocated to section 4 (consumer side), enforcement text updated to 'consumer service'",
    "tests: no test coverage for new rules — not applicable: this is a docs-only plugin repo, not a Go service",
    "correctness: singular-option-type ast-grep patterns — addressed: enforcement text clarifies this is clarity-focused, not a correctness fix"
  ]
}