Skip to content

feat(rules): bootstrap 3 Go doc families (mod-replace, glog, concurrency)#22

Merged
bborbe merged 2 commits into
masterfrom
feat/bootstrap-go3
Jun 2, 2026
Merged

feat(rules): bootstrap 3 Go doc families (mod-replace, glog, concurrency)#22
bborbe merged 2 commits into
masterfrom
feat/bootstrap-go3

Conversation

@bborbe

@bborbe bborbe commented Jun 2, 2026

Copy link
Copy Markdown
Owner

Summary

Third multi-doc batch (after PRs #17, #18, #21). 3 small Go docs (84-148 lines), 4 rules.

`rules/index.json`: 80 → 84 entries (3 new families).

Rules added

family / id level owner
`go-mod-replace/no-cross-repo-replace` MUST go-quality-assistant
`go-glog/use-v-for-debug-not-info` MUST go-quality-assistant
`go-concurrency/no-raw-go-func` MUST go-architecture-assistant
`go-concurrency/channel-closed-by-sender-only` MUST go-architecture-assistant

Only 4 rules this round — go-mod-replace and go-glog are narrow-scope docs with one strong MUST each; padding either with a forced second rule would dilute signal.

Walker validation caught a typo pre-push

First attempt named the glog rule `use-V-for-debug-not-info` (with a capital V). `make build-index` rejected it because `scripts/build-index.py` enforces `[a-z0-9-]+/[a-z0-9-]+` (lowercase + digits + hyphens only) on rule IDs. Renamed to `use-v-for-debug-not-info`. Validation chain working as designed — would have been a noisy bot finding otherwise.

Pre-emptive checks

  • `grep -nEi "personal/|/users/bborbe|~/documents/obsidian"` → clean
  • Trading-term grep → clean
  • All 4 rule IDs unique against 80 existing entries
  • Both owner agents exist
  • CLAUDE.md doc-agent table updated with 3 new rows
  • `make build-index` regenerated; `check-index` passes

Test plan

  • `make precommit` clean
  • 4 entries in index with consistent schema
  • All rule IDs pass walker validation
  • Bot review

…ncy)

Third multi-doc batch shape (after PRs #17, #18, #21). 3 small Go docs
(84-148 lines each), 4 rules. Bot review on one cycle.

Rules added (rules/index.json: 80 -> 84):

go-mod-replace/* (owner: go-quality-assistant)
- no-cross-repo-replace (MUST) — replace directives must not point
  outside the current repo's working tree. Off-repo replaces only work
  on the author's machine; CI breaks; module graph becomes
  non-reproducible. Same-repo monorepo replaces ARE correct and remain
  exempt.

go-glog/* (owner: go-quality-assistant)
- use-v-for-debug-not-info (MUST) — V0 (bare glog.Info) is the always-on
  production level; debug/trace/internal-state logs use glog.V(1+).
  Inverting the levels produces gigabytes of noise in production and
  requires a deploy to change verbosity for troubleshooting.
  (Rule ID is all-lowercase per build-index.py regex
  '[a-z0-9-]+/[a-z0-9-]+' — initial attempt with capital V was rejected;
  walker caught it before push.)

go-concurrency/* (owner: go-architecture-assistant)
- no-raw-go-func (MUST) — never use 'go func()' outside main.go entry
  points; use github.com/bborbe/run strategies (CancelOnFirstErrorWait,
  All, Sequential). Raw goroutines leak, race, and require hand-rolled
  sync.WaitGroup that drift toward deadlocks.
- channel-closed-by-sender-only (MUST) — only the producer closes a
  channel. Closing from the receiver side panics on still-pending
  sends; receivers use 'for v := range ch' or comma-ok idiom.

CLAUDE.md doc-agent table updated.

Generic examples throughout. No personal vault paths, no trading-
domain terms. Pre-emptive grep clean.

make build-index regenerated; check-index passes.

@ben-s-pull-request-reviewer ben-s-pull-request-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All checks complete. Here is the consolidated report:


PR Review: feat(rules): bootstrap 3 Go doc families (mod-replace, glog, concurrency)

Branch: feat/bootstrap-go3 → master
Files changed: 5 (CLAUDE.md, docs/go-concurrency-patterns.md, docs/go-glog-guide.md, docs/go-mod-replace-guide.md, rules/index.json)


Automated Checks

Check Result
LICENSE file ✅ Present
rules/index.json JSON validity ✅ Valid (Python json.load confirmed parseable)
Ast-grep YAML for new rules ❌ None exist for any of the 4 new rules

Must Fix (Critical)

None — no security vulnerabilities, no broken builds, no data correctness issues.


Should Fix (Important)

1. go-glog-guide.md is missing from README.md and llms.txt (discoverability)

  • Only go-logging-guide.md appears in both indexes. go-glog-guide.md is referenced by go-cli-guide.md:120 and go-k8s-binary-conventions.md:216, but readers browsing the plugin will not find it in the index tables.
  • rules/index.json:284 correctly registers the rule, but the guide itself is not listed in the main documentation index.

2. go-glog-guide.md — rule scope contradicts heading placement

  • The ### RULE go-glog/use-v-for-debug-not-info block (line 39) is nested under the H2 heading "Info V0 — Production Default" (line 37), yet its Applies when field covers "debug, trace, internal-state" logs at all V levels. Readers scanning the heading structure will assume the rule applies only to V0.
  • Fix: Move the rule block to a top-level position (e.g., before the level-by-level sections) or add a preamble noting its scope covers all V levels.

3. go-glog-guide.md — V(2) framing conflicts with go-logging-guide.md

  • go-glog-guide.md line 96 describes V(2) as "External Communication / Developer Default".
  • go-logging-guide.md line 30 describes V(2) as a "heartbeat" requiring sampling.
  • These are not strictly contradictory (external calls can be heartbeats) but will confuse readers following both guides.

4. go-concurrency-patterns.md — orphan rule 6 ("Capture loop variables by value")

  • The trailing "Rules" list (lines 211–216) contains 6 items but only 2 have ### RULE blocks. Item 6 (Capture loop variables by value: x := x) has no RULE block, no Why, and no Bad/Good example.
  • Per CLAUDE.md, enforceable guides need full rule blocks for all mandatory rules.

5. go-concurrency-patterns.md — missing ## Antipatterns section

  • The guide is enforceable (has 2 RULE blocks) but has no dedicated antipatterns section. CLAUDE.md Writing Docs standard: "End with antipatterns section."

6. All 4 new rules cite "ast-grep partial" but no YAML files exist in rules/go/

  • go-concurrency/no-raw-go-func, go-concurrency/channel-closed-by-sender-only, go-glog/use-v-for-debug-not-info, go-mod-replace/no-cross-repo-replace — all claim partial ast-grep enforcement but rules/go/ contains no corresponding YAML files.
  • If mechanical enforcement is not yet built, the Enforcement fields should say "judgment only" to avoid implying tooling that doesn't exist.

7. go-glog-guide.md — duplicate V-level table in agents/go-quality-assistant.md

  • agents/go-quality-assistant.md lines 129–135 repeat the V-level summary verbatim. Per CLAUDE.md ("rules live in docs, not duplicated in agents"), the agent should reference the doc as source of truth.

Nice to Have (Optional)

8. go-mod-replace-guide.md — prose/RULE duplication

  • The rule's substance is repeated 4×: RULE block, "Why" prose, "Correct patterns" prose, and "Antipatterns" section. The canonical source should be the RULE block; prose should add unique value only (workflow steps, exceptions).

9. go-concurrency-patterns.md — extra blank line at line 150–151


Concerns Addressed

Concern Status
go-concurrency/no-raw-go-func: partial ast-grep only Raised — no YAML files exist; enforcement should be "judgment only" until built
go-concurrency/channel-closed-by-sender-only: producer/consumer misclassification risk Acknowledged — judgment-only enforcement; risk is in the doc not the implementation
go-glog/use-v-for-debug-not-info: ast-grep partial cannot judge "production-default-worthy" Raised — no YAML files exist
go-mod-replace/no-cross-repo-replace: ast-grep misses same-repo escapes Acknowledged — "judgment only" scope in Enforcement field is appropriate
No test files changed Not a review finding — this is a doc/rule bootstrap, not implementation
CLAUDE.md table kept in sync ✅ Correctly updated with 3 new entries

Next Steps

  1. Add go-glog-guide.md to README.md (Go — Code Quality table) and llms.txt
  2. Move the ### RULE block in go-glog-guide.md out from under the V0 H2 to a top-level position
  3. Add ## Antipatterns section to go-concurrency-patterns.md; promote or remove orphan rule 6
  4. Update Enforcement fields to "judgment only" for all 4 rules until ast-grep YAML files are built
  5. Remove duplicate V-level table from agents/go-quality-assistant.md; reference go-glog-guide.md instead
  6. Resolve V(2) framing conflict between go-glog-guide.md and go-logging-guide.md

{
  "verdict": "request-changes",
  "summary": "The 3 new Go doc families (mod-replace, glog, concurrency) are well-structured with correct RULE block schemas and proper CLAUDE.md/rules/index.json integration. However, go-glog-guide.md is missing from README.md and llms.txt (critical discoverability gap), the go-glog rule block placement misleads readers about its scope, go-concurrency-patterns.md lacks a required antipatterns section and has an orphan rule without enforcement, and all 4 new rules claim ast-grep partial enforcement that has no corresponding YAML files in rules/go/.",
  "comments": [
    {
      "file": "docs/go-glog-guide.md",
      "line": 39,
      "severity": "major",
      "message": "go-glog/use-v-for-debug-not-info: rule block is nested under H2 'Info V0' but its Applies when covers ALL V levels — move to top-level position so readers understand the full scope"
    },
    {
      "file": "README.md",
      "line": 105,
      "severity": "major",
      "message": "go-glog-guide.md is missing from README.md — only go-logging-guide.md appears in the Go Code Quality table, making the new guide undiscoverable to plugin users"
    },
    {
      "file": "llms.txt",
      "line": 51,
      "severity": "major",
      "message": "go-glog-guide.md is missing from llms.txt — same discoverability gap as README.md"
    },
    {
      "file": "docs/go-concurrency-patterns.md",
      "line": 216,
      "severity": "major",
      "message": "Rule 6 in the trailing list ('Capture loop variables by value: x := x') has no RULE block, no Why, and no Bad/Good example — orphan rule that must be promoted to a full ### RULE or removed"
    },
    {
      "file": "docs/go-concurrency-patterns.md",
      "line": 209,
      "severity": "major",
      "message": "Enforceable guide missing a dedicated ## Antipatterns section — CLAUDE.md Writing Docs standard requires one for all enforceable guides"
    },
    {
      "file": "rules/index.json",
      "line": 119,
      "severity": "major",
      "message": "go-concurrency/channel-closed-by-sender-only cites 'ast-grep partial' but no YAML file exists in rules/go/ — update Enforcement to 'judgment only' until mechanical detection is built"
    },
    {
      "file": "rules/index.json",
      "line": 129,
      "severity": "major",
      "message": "go-concurrency/no-raw-go-func cites 'ast-grep partial' but no YAML file exists in rules/go/ — update Enforcement to 'judgment only' until mechanical detection is built"
    },
    {
      "file": "rules/index.json",
      "line": 278,
      "severity": "major",
      "message": "go-glog/use-v-for-debug-not-info cites 'ast-grep partial' but no YAML file exists in rules/go/ — update Enforcement to 'judgment only' until mechanical detection is built"
    },
    {
      "file": "rules/index.json",
      "line": 417,
      "severity": "major",
      "message": "go-mod-replace/no-cross-repo-replace cites 'ast-grep partial' but no YAML file exists in rules/go/ — update Enforcement to 'judgment only' until mechanical detection is built"
    },
    {
      "file": "agents/go-quality-assistant.md",
      "line": 129,
      "severity": "major",
      "message": "Duplicate V-level summary (lines 129-135) verbatim from go-glog-guide.md — per CLAUDE.md rules live in docs, not duplicated in agents; replace with a reference to go-glog-guide.md"
    },
    {
      "file": "docs/go-glog-guide.md",
      "line": 96,
      "severity": "minor",
      "message": "V(2) described as 'Developer Default / External Communication' here but as 'heartbeat (use sampling!)' in go-logging-guide.md line 30 — conflicting guidance for the same level"
    },
    {
      "file": "docs/go-mod-replace-guide.md",
      "line": 41,
      "severity": "nit",
      "message": "Prose 'Why' section duplicates content already in the RULE block (4x total repetition across RULE, Why, Correct patterns, Antipatterns) — condense prose to only workflow steps and exceptions not covered by the rule block"
    },
    {
      "file": "docs/go-concurrency-patterns.md",
      "line": 150,
      "severity": "nit",
      "message": "Extra blank line at line 150-151 (cosmetic)"
    }
  ],
  "concerns_addressed": [
    "correctness: go-concurrency/no-raw-go-func enforcement 'ast-grep partial' is misleading — no YAML files exist in rules/go/; raised as major finding",
    "correctness: go-concurrency/channel-closed-by-sender-only enforcement 'judgment (ast-grep partial)' — ast-grep files don't exist; raised as major finding",
    "correctness: go-glog/use-v-for-debug-not-info enforcement 'ast-grep partial' — no YAML files; raised as major finding",
    "correctness: go-mod-replace/no-cross-repo-replace enforcement 'ast-grep partial' — no YAML files; raised as major finding",
    "tests: no test files — not a review finding; this is a doc/rule bootstrap PR",
    "docs: CLAUDE.md table correctly updated with 3 new entries — confirmed correct"
  ]
}

Bot's deep review caught 10 real issues across 3 docs + agent file + README/llms.txt + index.json.

MAJOR (all fixed):

1. go-glog rule placement: was nested under '## Info V0' which made the scope
   look V0-only. Moved to top-level (before all '## Error/Warning/V0..V3+'
   sections) so the cross-V-level scope is obvious. Tightened Applies-when
   to explicitly state 'across ALL V levels' and removed the duplicate copy
   under Info V0.

2. README.md missing go-glog-guide.md — added entry under Go Code Quality
   table next to go-logging-guide.md.

3. llms.txt missing go-glog-guide.md — added entry next to logging.

4. go-concurrency orphan rule 6 'Capture loop variables by value: x := x'
   in the Rules numbered list. Outdated for Go 1.22+ which fixed loop-variable
   capture semantics (per-iteration scoping). Removed from the list and
   added a note explaining the Go 1.22+ fix + when x := x is still needed
   (pre-1.22 projects).

5. go-concurrency missing '## Antipatterns' section — added one
   cross-referencing the canonical #### Bad blocks under each ### RULE,
   plus 4 summary bullets covering raw go-func / consumer-close /
   unbounded-with-defer-close / hidden-Results-channel.

6-9. All 4 new rules' Enforcement field said 'ast-grep partial' but no
   YAML files exist in rules/go/. Reworded to 'judgment (ast-grep follow-up:
   ...)' matching the established phrasing across the repo's other
   judgment-tier rules. The pattern stays the same: the agent enforces
   judgment now, ast-grep YAML lands in a focused follow-up PR.

10. agents/go-quality-assistant.md duplicated the 6-line glog V-level
   table verbatim from go-glog-guide.md. Per CLAUDE.md 'rules live in
   docs, not duplicated in agents', replaced with a one-line reference
   pointing at the doc + the canonical rule ID. Future drift impossible.

MINOR (deferred, pre-existing): V(2) is described as 'Developer Default'
in go-glog-guide.md but as 'heartbeat (use sampling!)' in
go-logging-guide.md. Inter-doc inconsistency that predates this PR; should
be resolved in a focused follow-up that decides the canonical semantic
for V(2). Out of scope for this PR's rule blocks.

NITs skipped: prose duplication in go-mod-replace (RULE block + 'Why'
section overlap is intentional — they serve different readers); blank-
line cosmetic at concurrency line 150.

make build-index regenerated; check-index passes.
@ben-s-pull-request-reviewer ben-s-pull-request-reviewer Bot dismissed their stale review June 2, 2026 15:24

superseded by new automated review

@ben-s-pull-request-reviewer ben-s-pull-request-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worktree cleaned up. Here is the consolidated report:


PR Review: feat(rules): bootstrap 3 Go doc families (mod-replace, glog, concurrency)

PR #22 — base: master ← head: feat/bootstrap-go3 (commit 1e89c07 — fix commit addressing PR #22 review)

Scope: 8 files, +187/−14 lines. All changes are docs/agent/rule-index bootstrap — no production Go code.


Automated Checks

Check Result
LICENSE file ✅ Present
make precommit ✅ Equivalent checks pass (link validation, JSON validation, index check — confirmed by agent review)

Must Fix (Critical)

None.


Should Fix (Important)

None — the fix commit 1e89c07 resolved all 10 MAJOR issues from the original PR review.


Nice to Have (Optional)

  1. docs/go-concurrency-patterns.md line ~150 — Double blank line: After the channel-closed-by-sender-only Good/Bad code blocks, two blank lines precede the "Caller creates and owns the channel..." prose. Cosmetic only; already noted as deferred in the fix commit message.

  2. Pre-existing glog V-level inconsistency: go-glog-guide.md calls V(2) "Developer Default" while go-logging-guide.md calls V(2) "heartbeat (use sampling!)". Cross-doc inconsistency; explicitly deferred in fix commit. Should be resolved in a follow-up to prevent reviewer confusion when consulting both docs.

  3. go-glog-guide.md — Error/Warning sections lack ### RULE blocks: Only V(0) misuse (use-v-for-debug-not-info) has a rule block. Error and Warning levels are prose-only. Not a current gap, but a structural inconsistency worth tracking if future rule coverage expands.


Plan Concerns — Resolution Status

Concern Status
rules/index.json: 4 new entries (80→84), JSON schema consistency ✅ Addressed — all 84 entries share same 7-key schema, IDs match doc headings exactly
go-mod-replace-guide.md: rule ID go-mod-replace/no-cross-repo-replace correctly referenced ✅ Addressed
go-glog-guide.md: rule ID go-glog/use-v-for-debug-not-info (lowercase v — fixed from prior capital V typo) ✅ Addressed
go-concurrency-patterns.md: 2 rule IDs no-raw-go-func and channel-closed-by-sender-only ✅ Addressed
agents/go-quality-assistant.md: new rules assigned to correct owners ✅ Addressed
CI validation (make precommit clean) ✅ Addressed

{
  "verdict": "approve",
  "summary": "PR #22 bootstrap commit is clean. Four new MUST rules across three Go doc families (mod-replace, glog, concurrency) are correctly structured with matching rules/index.json entries, proper doc↔agent alignment in CLAUDE.md, and consistent JSON schema. The fix commit (1e89c07) resolved all 10 MAJOR findings from the original review. No production code changed — only docs, agent guidance, and the rule index.",
  "comments": [
    {
      "file": "docs/go-concurrency-patterns.md",
      "line": 150,
      "severity": "nit",
      "message": "Double blank line after the channel-closed-by-sender-only Good/Bad code blocks before the 'Caller creates and owns the channel' prose. Cosmetic only — already noted as deferred in the fix commit message."
    },
    {
      "file": "docs/go-glog-guide.md",
      "line": 98,
      "severity": "nit",
      "message": "Pre-existing cross-doc inconsistency: V(2) is described as 'Developer Default' here, but go-logging-guide.md describes it as 'heartbeat (use sampling!)'. Explicitly deferred in fix commit — worth a follow-up cleanup."
    },
    {
      "file": "docs/go-glog-guide.md",
      "line": 4,
      "severity": "nit",
      "message": "The Error and Warning level sections lack ### RULE blocks (only the V(0) misuse rule has one). Structural inconsistency worth tracking if rule coverage expands."
    }
  ],
  "concerns_addressed": [
    "correctness: rules/index.json grew 80→84 entries with consistent 7-key schema and matching rule IDs across doc headers and JSON index",
    "correctness: go-mod-replace-guide.md rule ID 'go-mod-replace/no-cross-repo-replace' correctly referenced in both doc and index",
    "correctness: go-glog-guide.md rule ID 'go-glog/use-v-for-debug-not-info' correctly uses lowercase v (prior PR had capital V typo, now fixed)",
    "correctness: go-concurrency-patterns.md has two correctly referenced rule IDs 'no-raw-go-func' and 'channel-closed-by-sender-only'",
    "correctness: agents/go-quality-assistant.md new rules assigned to correct owners (go-quality-assistant and go-architecture-assistant per CLAUDE.md)",
    "tests: CI validation via make precommit passes — link validation, JSON syntax, and index check all confirmed clean"
  ]
}

@bborbe bborbe merged commit 21e470f into master Jun 2, 2026
1 check passed
@bborbe bborbe deleted the feat/bootstrap-go3 branch June 2, 2026 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant