add rule: no-same-package-private-helper-for-business-logic

[bborbe]

Summary

New judgment-tier rule go-composition/no-same-package-private-helper-for-business-logic (MUST, owner go-architecture-assistant). Closes the gap where new business logic added as a same-package private helper escapes both the mechanical funnel (ast-grep pattern matches pkg.Func cross-package syntax only) and the existing go-composition/no-package-function-calls-in-business-logic rule (cross-package only).

Why

Real case from PR bborbe/recurring-task-creator#16: shipped a private buildFrontmatter helper doing business logic (default seeding + placeholder rendering + provenance override). Local /coding:pr-review flagged 9 adjacent cross-package findings on the same file; I dismissed them as "pre-existing patterns" (broken-windows fallacy). The new code was untestable in isolation — only reachable through Publisher.Publish integration tests. User caught the missing interface seam manually post-review and asked for the guide to be updated so it gets caught automatically next time.

The new rule explicitly names the same-package case + the test-pyramid implication (business-logic helpers need their own *_test.go).

Changes

• docs/go-composition.md — new RULE section with Bad (buildFrontmatter from PR feat(architecture): bootstrap 6 rule blocks in go-architecture-patterns #16) / Good (FrontmatterFormatter interface) code, exempt list (pure local formatting helpers, test helpers, generated code), concrete trigger criteria, and codification note
• rules/index.json — registers the rule (judgment-tier, owner go-architecture-assistant, trigger **/*.go); index grew from 139 → 140 entries
• CHANGELOG.md — feat entry under ## Unreleased

Test plan

• Index JSON valid (jq parsed cleanly when appending)
• Doc section follows existing RULE format (same headers as sibling rules in go-composition.md)
• Future pr-review run on the recurring-task-creator follow-up PR exercises the new rule

[ben-s-pull-request-reviewer]

Good. Now I have all judgment rules. Let me do the Step 4b-i candidate filter manually:

Changed files: CHANGELOG.md, docs/go-composition.md, rules/index.json

Step 4b-i — judgment rules whose triggers match changed files:

Rule	Trigger	Matches	Decision
changelog/conventional-prefix-required	CHANGELOG.md	✅	Applicable
changelog/preamble-frozen	CHANGELOG.md	✅	Applicable
go-composition/no-same-package-private-helper-for-business-logic	**/*.go	❌	SKIP (no .go files changed)
All other rules	various	❌	SKIP (trigger doesn't match)

Step 4c-sel CLASSIFY — 2 applicable, 0 skipped beyond candidates:

1. changelog/conventional-prefix-required — applicable — CHANGELOG entry must have feat:/fix: prefix
2. changelog/preamble-frozen — applicable — CHANGELOG edit must not insert above preamble or inside it

Step 4d-sel ADJUDICATE:

• CHANGELOG.md: New ## Unreleased section added correctly after the preamble block, not inside it. Entry uses feat: prefix — correct. ✅
• docs/go-composition.md: New rule documentation block added correctly within an existing section. The block is well-formed with all required fields (owner, applies_when, enforcement, trigger criteria, bad/good examples, exemptions, codified date with PR reference). ✅
• rules/index.json: New rule entry is valid JSON, has all required fields (id, anchor, level, owner, enforcement_type, enforcement, applies_when, doc_path, trigger). ✅

Mechanical funnel: ast-grep runner required multi-command approval; noted as unavailable. No Go/Python code files changed — no mechanical findings possible from ast-grep YAMLs.

License check: LICENSE file exists. ✅

Context guides: No main.go, k8s/*.yaml, .env, or teamvault files changed. ✅

Now I have everything needed for the report.

---

Must Fix (Critical): None.

Should Fix (Important): None.

Nice to Have (Optional): None.

Selector Mode Traceability:

• Candidates: 63 judgment rules matched by glob
• Applicable: 2 (changelog/conventional-prefix-required, changelog/preamble-frozen)
• Skipped: all others — none matched changed file globs

---

Next Steps: /coding:commit to land the release commit tagging v0.23.0 once CI passes.

{
  "verdict": "approve",
  "summary": "PR adds a well-formed judgment-tier rule `go-composition/no-same-package-private-helper-for-business-logic` (MUST, owner go-architecture-assistant) with proper documentation, exemptions, bad/good examples, and a codified history entry. CHANGELOG entry uses correct `feat:` prefix and is placed correctly after the preamble. rules/index.json entry is valid and complete. No violations found on the changed files themselves.",
  "comments": [],
  "concerns_addressed": [
    "correctness: trigger criteria in docs/go-composition.md are specific (4-part checklist with exemptions) — sufficiently precise to avoid false positives on trivial helpers",
    "correctness: rule is intentionally judgment-tier with no ast-grep pattern — the rule itself documents this as a feature (semantic distinction requires human judgment)",
    "tests: follow-up PR on recurring-task-creator is a future action item, not a gap in this PR"
  ]
}

superseded by new automated review

[ben-s-pull-request-reviewer]

Based on the diff I've gathered, let me do the consolidated review analysis.

Review findings:

Changed files verified:

• CHANGELOG.md (+4 lines) — unreleased entry for new rule
• docs/go-composition.md (+82 lines) — new RULE block with Bad/Good examples
• rules/index.json (+13 lines) — new rule entry

Step 4 early exit applies: no Go/Python files changed — only docs, markdown, and JSON metadata.

Concern analysis:

Concern 1 — rules/index.json judgment enforcement: ✅ Correct. The rule is intentionally enforcement_type: "judgment" because distinguishing business logic from trivial helpers requires reading the function body. The trigger "**/*.go" is broad but acceptable for judgment-tier rules — the trigger field is not used for mechanical scanning, only for LLM adjudication scoping.

Concern 2 — Bad example accuracy: ✅ The buildFrontmatter example is drawn from bborbe/recurring-task-creator#16 as documented. The code shows a private helper doing non-trivial transformation (loop + conditional + template rendering + provenance injection) — matches the "Concrete trigger" criteria accurately.

Concern 3 — Good example mock path: ✅ ../../mocks/publisher-frontmatter-formatter.go is plausible for a structure where frontmatter.go sits at e.g. pkg/publisher/frontmatter.go → mocks at mocks/publisher-frontmatter-formatter.go. Counterfeiter's -o flag accepts relative paths from the generating file's location.

Concern 4 — applies_when prose vs doc "Concrete trigger": ⚠️ The JSON applies_when and enforcement fields are long prose paragraphs that largely duplicate the doc's "Concrete trigger" section. Per CLAUDE.md "the doc is authoritative; the JSON field should be a summary, not a different statement." Here the JSON is close to a copy-paste of the doc. However, since this is a judgment rule (no mechanical funnel), the JSON fields are for human readability in the index only — not used by any automated tool. Low risk, but could be tightened to a 1-line summary.

Other observations:

CHANGELOG.md: Entry correctly notes the judgment-tier nature and the broken-windows fallback rationale.

JSON validity: Cannot fully verify programmatically (jq/python blocked), but the diff shows well-formed JSON with correct comma/brace placement and all 8 required fields present.

Missing ast-grep YAML: Intentionally omitted — this is a judgment-only rule per its design rationale. Correct.

---

Now producing the final JSON verdict:

{
  "verdict": "approve",
  "summary": "Docs-only PR adding a judgment-tier rule. The rule is well-structured with clear trigger criteria, accurate real-world Bad example, and plausible Good example. JSON index entry is complete with all required fields. Minor note: the `applies_when`/`enforcement` prose in index.json is verbose (duplicates doc content) butharmless for a judgment-only rule.",
  "comments": [
    {
      "file": "rules/index.json",
      "line": 334,
      "severity": "minor",
      "message": "The `applies_when` and `enforcement` fields are lengthy prose paragraphs that largely duplicate the doc's \"Concrete trigger\" section. For judgment-only rules the JSON fields are human-readability only (no mechanical consumer), so this is not a blocker — but consider a 1-line summary here to avoid drift between doc (authoritative) and JSON."
    }
  ],
  "concerns_addressed": [
    "correctness: rules/index.json — new rule correctly uses 'judgment' enforcement; ast-grep cannot auto-detect; relies on human review. Trigger criteria (4 concrete conditions) in doc are clear enough for consistent application.",
    "docs: docs/go-composition.md — Bad example (buildFrontmatter) drawn from bborbe/recurring-task-creator#16, accurately reflects the problematic pattern described.",
    "docs: docs/go-composition.md — Good example shows counterfeiter usage with mock path '../../mocks/publisher-frontmatter-formatter.go' — realistic for described project structure.",
    "correctness: rules/index.json — 'applies_when' description is a long prose paragraph; it does not conflict with the doc's \"Concrete trigger\" section (same message, more detailed in doc). Doc is authoritative. JSON is acceptable for judgment-only rules."
  ]
}