feat(rules): bootstrap 3 Go doc families (enum-type, composition, build-args)#24
Merged
Conversation
…ld-args)
Fifth Go bootstrap batch. 3 docs (120-189 lines), 5 rules.
Rules added (rules/index.json: 90 -> 95):
go-enum-type/* (owner: go-architecture-assistant)
- typed-constants-with-collection (MUST) — enum constants must be
declared with explicit type (type X string + const Y X = ...) AND
paired with a 'var AvailableXs Xs' collection. Untyped constants
spread as bare strings; the AvailableXs collection makes the
closed-set discoverable.
- validate-against-available-collection (MUST) — Validate() ranges
over AvailableXs.Contains, never an inline switch. Collapses the
two declarations (const + switch) into one source of truth so
adding a new constant can't silently break validation.
go-composition/* (owner: go-architecture-assistant)
- no-package-function-calls-in-business-logic (MUST) — service methods
receive capabilities via injected interfaces, never via direct
package-function calls (prompt.X, git.Y). Hidden deps don't show up
at the constructor, can't be mocked, can't be swapped.
- small-interfaces-1-2-methods (SHOULD) — Rob Pike's 'the bigger the
interface, the weaker the abstraction'. 3+ method interfaces span
capabilities; split per capability and compose explicitly when
needed. SHOULD because composition cases legitimately bundle.
go-build-args/* (owner: go-quality-assistant)
- three-args-required (MUST) — every Docker build passes
BUILD_GIT_VERSION, BUILD_GIT_COMMIT, BUILD_DATE. Each answers a
different forensic question ('which release?', 'exact source?',
'when built?'). Missing any one removes the answer to its question;
the cost is three Makefile lines.
CLAUDE.md doc-agent table updated with all 3 new mappings.
Generic examples (Order, OrderStatus, ItemStore, Logger). No personal
vault paths, no trading-domain terms. Pre-emptive grep clean.
make build-index regenerated; check-index passes.
There was a problem hiding this comment.
PR Review Summary
Review of: feat(rules): bootstrap 3 Go doc families (enum-type, composition, build-args)
Branch: feat/bootstrap-go5 → master
Files Changed: CLAUDE.md, docs/go-build-args-guide.md, docs/go-composition.md, docs/go-enum-type-pattern.md, rules/index.json
Automated Checks
| Check | Result |
|---|---|
make precommit |
Has precommit target (check-links check-json check-index) |
| Project type | Plugin repo (Markdown + JSON), not Go library |
| License file | ✅ Present (BSD-2-Clause) |
| README License section | ✅ Present |
Findings
Must Fix
| File | Line | Issue |
|---|---|---|
docs/go-composition.md |
24 | Unused variable in Bad example — queued := prompt.ListQueued(...) is declared but never used. Go rejects this at compile time. The Bad example is meant to show hidden dependencies, not unused variables — fix by using _ = prompt.ListQueued(...) or passing queued to Execute. |
Should Fix
| File | Line | Issue |
|---|---|---|
docs/go-enum-type-pattern.md |
61 | Missing context.Context — the Process(status OrderStatus) error function signature in the Good example should be Process(ctx context.Context, status OrderStatus) error per project conventions. This is a Good example teaching the wrong signature. |
docs/go-composition.md |
135 | Missing context.Context — the Logger interface in the Good example has Log(msg string) but should accept ctx context.Context for trace propagation consistency with glog(ctx, ...). |
docs/go-enum-type-pattern.md |
42–118 | Exported identifiers lack doc comments — both the #### Good block and the ### Minimal Complete Implementation define OrderStatus, OrderStatuses, AvailableOrderStatuses, String(), Validate(), Contains() without GoDoc comments. This directly contradicts the go-doc/exported-item-must-have-comment (MUST) rule the doc itself teaches. Readers who copy these examples get non-compliant code. |
docs/go-composition.md |
34–53 | New "Good" example interfaces lack doc comments — PromptScanner, Releaser, and NewRunner in the diff-added Good block have no GoDoc comments, same issue as above. |
Nice to Have
| File | Line | Issue |
|---|---|---|
docs/go-enum-type-pattern.md |
105 | == false vs ! inconsistency — Minimal Complete Implementation uses if AvailableOrderStatuses.Contains(o) == false while the newer Good example (line 147) uses idiomatic if !AvailableOrderStatuses.Contains(o). The latter is canonical — update line 105 to match. |
docs/go-enum-type-pattern.md |
174 | Undefined Priority type referenced — the Order struct has a Priority field of type Priority that is never declared in the example, potentially confusing readers who try to compile it. |
docs/go-composition.md |
49 | Runner return type undeclared — NewRunner returns Runner but the interface is not declared in the code block. Self-contained examples should declare it. |
docs/go-build-args-guide.md |
19 | RULE block placed before its context — go-build-args/three-args-required RULE appears before the ## Layout section that formally introduces the three-file structure. A reader lands on the rule before seeing the implementation. |
docs/go-composition.md |
110 | Composed Service interface not addressed in examples — the small-interfaces-1-2-methods Good example shows a composed Service { Logger; ItemStore; Notifier } with 3+ methods, which could be naively flagged as a violation. The text resolves this but the example doesn't demonstrate the discriminator. |
Verdict
{
"verdict": "request-changes",
"summary": "The PR adds 5 new rule definitions across 3 Go doc families (enum-type, composition, build-args) with correct rules/index.json entries and CLAUDE.md mappings. However, the Go code examples embedded in the docs have two categories of issues: (1) a compile error in the Bad example at go-composition.md:24 (unused variable), and (2) consistent omission of GoDoc comments on exported identifiers in Good examples — directly contradicting the go-doc/exported-item-must-have-comment rule these docs teach. Additionally, two Good examples teach function signatures missing context.Context.",
"comments": [
{
"file": "docs/go-composition.md",
"line": 24,
"severity": "critical",
"message": "Must Fix [go-composition/no-package-function-calls-in-business-logic Bad example]: `queued` is declared and assigned but never used — Go rejects this at compile time. Fix: use `_ = prompt.ListQueued(ctx, r.dir)` or pass `queued` to `Execute`. The Bad example should demonstrate hidden dependencies, not unused variables."
},
{
"file": "docs/go-enum-type-pattern.md",
"line": 61,
"severity": "major",
"message": "Should Fix [go-enum-type/typed-constants-with-collection Good example]: `Process(status OrderStatus) error` is missing `context.Context` as first parameter. Per project conventions (and go-errors/no-context-background-in-business-logic), exported functions doing validation/business logic must accept ctx. Correct signature: `Process(ctx context.Context, status OrderStatus) error`."
},
{
"file": "docs/go-composition.md",
"line": 135,
"severity": "major",
"message": "Should Fix [go-composition/small-interfaces-1-2-methods Good example]: `Logger` interface defines `Log(msg string)` without context. For trace propagation consistency with glog(ctx, ...) conventions, correct shape is `Log(ctx context.Context, msg string)`."
},
{
"file": "docs/go-enum-type-pattern.md",
"line": 42,
"severity": "major",
"message": "Should Fix [go-enum-type/typed-constants-with-collection Good example + Minimal Complete Implementation]: All exported identifiers (OrderStatus, OrderStatuses, AvailableOrderStatuses, String(), Validate(), Contains()) lack GoDoc comments. This directly contradicts the go-doc/exported-item-must-have-comment (MUST) rule these docs teach — readers copying the example get non-compliant code. Add doc comments to all exported items per the rule."
},
{
"file": "docs/go-composition.md",
"line": 34,
"severity": "major",
"message": "Should Fix [go-composition/no-package-function-calls-in-business-logic Good example]: Exported identifiers PromptScanner, Releaser, and NewRunner lack GoDoc comments, contradicting go-doc/exported-item-must-have-comment (MUST)."
},
{
"file": "docs/go-enum-type-pattern.md",
"line": 105,
"severity": "nit",
"message": "Nice to Have [go-enum-type-pattern.md Minimal Complete Implementation]: Uses `if AvailableOrderStatuses.Contains(o) == false` while the newer Good example at line 147 uses idiomatic `if !AvailableOrderStatuses.Contains(o)`. Update line 105 to use `!` for internal consistency."
},
{
"file": "docs/go-enum-type-pattern.md",
"line": 174,
"severity": "nit",
"message": "Nice to Have [go-enum-type-pattern.md Usage Example]: `Order` struct references `Priority Priority` field but `Priority` type is never declared in the example. Either declare it or remove the field to avoid confusing readers who try to compile the snippet."
},
{
"file": "docs/go-composition.md",
"line": 49,
"severity": "nit",
"message": "Nice to Have [go-composition.md NewRunner Good example]: `NewRunner` returns `Runner` interface but `Runner` is not declared in the code block. For self-contained examples, add `type Runner interface { Run(ctx context.Context) error }` above the constructor."
},
{
"file": "docs/go-build-args-guide.md",
"line": 19,
"severity": "nit",
"message": "Nice to Have [go-build-args/three-args-required]: RULE block is placed before the ## Layout section that formally describes the three-file structure (Makefile.docker, Dockerfile, Argument struct). Readers hitting the rule first may lack context for understanding the examples. Consider moving the RULE after the Layout section, or adding a cross-reference."
},
{
"file": "docs/go-composition.md",
"line": 110,
"severity": "nit",
"message": "Nice to Have [go-composition/small-interfaces-1-2-methods]: The composed `Service` interface in the Good example has 3+ methods (Logger+ItemStore+Notifier), which could be naively flagged as a rule violation. The applies_when text correctly explains this is acceptable via composition, but the example itself doesn't call this out. Consider adding a comment in the example explaining the composed interface satisfies ISP."
}
],
"concerns_addressed": [
"correctness: rules/index.json rule definitions are syntactically valid and consistent with existing rule schema — confirmed, all 5 entries are well-formed",
"correctness: go-enum-type-pattern.md enum-type examples are idiomatic with one compile error (unused variable in Bad example at composition:24) and missing context.Context in Good examples",
"correctness: go-composition.md composition patterns follow Go idioms but have unused variable bug in Bad example",
"correctness: go-build-args-guide.md build args guidance is accurate for Go toolchain"
]
}Bot caught real issues, most are meta-violations: my Good examples
violated the very rules they document. Fixing keeps the examples
self-consistent with the rule set.
CRITICAL:
- go-composition no-package-function-calls-in-business-logic Bad
example had 'queued := prompt.ListQueued(...)' with queued never
used — Go compile error. Bot caught it. Fixed: use 'queued, _ :='
+ loop over queued to call r.executor.Execute(ctx, p). Example
now compiles AND demonstrates the hidden-dep anti-pattern more
realistically.
MAJOR — context.Context propagation (project convention):
- go-enum-type Process(status OrderStatus) missing ctx.Context. Per
go-errors/no-context-background-in-business-logic, exported functions
doing business logic take ctx first. Added 'ctx context.Context' to
signature.
- go-composition Logger.Log(msg string) missing ctx. Same convention
applied — Log(ctx context.Context, msg string).
MAJOR — meta-violations (examples violated the rules they teach):
- go-enum-type Good example had OrderStatus, AvailableOrderStatuses,
Process etc. with no GoDoc comments. Directly contradicts
go-doc/exported-item-must-have-comment (MUST) defined in the same
repo. Added GoDoc on every exported identifier in the example.
- go-composition Good examples had PromptScanner, Releaser, NewRunner,
Runner, Logger, ItemStore, Notifier, Service all without GoDoc.
Added GoDoc on every exported identifier. Also added the missing
'type Runner interface { Run(ctx context.Context) error }'
declaration the constructor returns (was referenced but never
declared).
NITs skipped (pre-existing or minor):
- Inline switch '== false' vs '!' style inconsistency: pre-existing
in the doc's earlier section, not introduced by this PR.
- Order struct's undeclared Priority field in Usage Example:
pre-existing.
- RULE block placement before Layout section in build-args:
judgment call; the Why section in the RULE block provides
sufficient context for the example.
make build-index regenerated; check-index passes.
superseded by new automated review
There was a problem hiding this comment.
PR Review Report: feat/ruled: bootstrap 3 Go doc families (enum-type, composition, build-args)
Summary
The PR adds 3 new Go documentation guides with 5 new rules, updates the CLAUDE.md agent table, and registers all rules in rules/index.json. All schema fields are consistent and complete.
Must Fix (Critical)
None.
Should Fix (Important)
None.
Nice to Have (Optional)
None — all cross-references verified, agents exist, JSON valid, README/llms.txt already had entries.
Concerns Addressed
- correctness:rules/index.json: 5 new rule entries verified — all have
anchor,applies_when,doc_path,enforcement,id,level,owner. No duplicate IDs. - correctness:CLAUDE.md: Agent table updated with 3 new rows.
go-enum-type-pattern.md → go-architecture-assistant✓,go-composition.md → go-architecture-assistant✓,go-build-args-guide.md → go-quality-assistant✓. All three agents exist inagents/. - correctness:README/llms.txt: Both already contained entries for the 3 new docs — no drift.
- docs:cross-refs:
go-build-args-guide.md:221referencesgo-makefile-commands.md— file exists.go-enum-type-pattern.md:214referencesgo-validation-framework-guide.md— file exists.
{
"verdict": "approve",
"summary": "PR adds 5 high-quality rules across 3 new Go doc families with consistent schema, correct agent assignments, and valid cross-references. No mechanical issues found in rules/index.json, CLAUDE.md, or doc cross-links.",
"comments": [],
"concerns_addressed": [
"correctness: rules/index.json — 5 new entries verified complete with all required fields, no duplicate IDs",
"correctness: CLAUDE.md — 3 new agent table rows correctly map docs to existing agents (go-architecture-assistant, go-quality-assistant)",
"docs: cross-references — go-makefile-commands.md and go-validation-framework-guide.md both exist",
"correctness: README.md and llms.txt already contained entries for all 3 new docs"
]
}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fifth Go bootstrap batch. 3 docs (120-189 lines), 5 rules.
`rules/index.json`: 90 → 95 entries (3 new families).
Rules added
5 rules across 3 docs — build-args's single MUST is the strong one for that doc; padding wouldn't add signal. Enum + composition have 2 rules each.
Pre-emptive checks
Test plan