From eec22879246283e13800a10fa7d3528efcd5fe34 Mon Sep 17 00:00:00 2001
From: Gonzalo Serrano <boikot@gmail.com>
Date: Thu, 7 May 2026 17:00:29 +0200
Subject: [PATCH] docs(gopilot): resolve self-contradiction in error-handling
 section
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The "Wrap with context" guidance and the "Handle Errors Once" section
disagreed:

- "Wrap with context" said use low-cardinality strings AND showed a
  Good example with slog.ErrorContext(...) at the wrap site followed by
  return fmt.Errorf(...). That's exactly the "log AND return" pattern
  the next section labels Bad.
- "Handle Errors Once"'s Good example used fmt.Errorf("connect to %s",
  addr, err) — a high-cardinality wrap, contradicting the previous rule.

Real Go code is mostly intermediate (wrap-and-return), not boundary
code. Mixing boundary-level logging into wrap-site advice is what
created the contradiction. Resolution:

1. Wrap sites only wrap (low-cardinality).
2. Variable data (IDs, names, paths) is logged once at the application
   boundary — HTTP middleware, gRPC interceptor, top-level main —
   from ctx-scoped fields or from a single structured slog.Error call.
3. Caveat acknowledging the rule presumes a boundary that surfaces
   variable data structurally; if it doesn't, embedding the variable
   in the wrap is the lesser evil. Treat low-cardinality as an
   optimization, not a correctness rule.

Bump to 1.0.34.
---
 .claude-plugin/plugin.json |  2 +-
 skills/gopilot/SKILL.md    | 13 +++++++------
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json
index 4d9081d..093507e 100644
--- a/.claude-plugin/plugin.json
+++ b/.claude-plugin/plugin.json
@@ -1,6 +1,6 @@
 {
   "name": "gopilot",
-  "version": "1.0.33",
+  "version": "1.0.34",
   "description": "gopilot is your Go copilot - a skill for writing idiomatic Go code, covering design patterns, error handling, testing, concurrency, generics, and stdlib patterns up to Go 1.26.",
   "author": {
     "name": "Gonzalo Serrano",
diff --git a/skills/gopilot/SKILL.md b/skills/gopilot/SKILL.md
index 3edd171..d69790f 100644
--- a/skills/gopilot/SKILL.md
+++ b/skills/gopilot/SKILL.md
@@ -58,13 +58,14 @@ Use these over inferring APIs. Hallucinated signatures fail at compile time; `go
 ## Error Handling
 
 - Errors are values. Design APIs around that.
-- Wrap with context: `fmt.Errorf("get config: %w", err)` -- use low-cardinality strings only (no IDs, names, or variable data in the format string; attach those as structured slog attributes so APM tools can group errors)
+- Wrap with context using low-cardinality strings: `fmt.Errorf("get config: %w", err)` rather than `fmt.Errorf("get config from %s: %w", path, err)`. APM tools that group by message text can collapse the same root cause across instances; high-cardinality wraps fragment the dashboard.
+- Variable data (IDs, names, paths) belongs in the structured log record at the **application boundary** (HTTP middleware, gRPC interceptor, top-level main), not at the wrap site. Intermediate layers wrap and return; the boundary observes the full chain and logs once with `slog.Error(..., "user_id", id, "error", err)`.
+- Caveat: if no boundary in your service preserves the variable as a structured field, embedding it in the wrap is the lesser evil — losing the data is worse than losing APM grouping. Treat low-cardinality as an optimization, not a correctness rule. Audit for `zap.String("user_id", id)` / `slog.Error(... "user_id", id ...)` near the call boundary before stripping the variable from a wrap.
   ```go
   // Bad: high-cardinality error string -- APM sees each user as a unique error
   return fmt.Errorf("fetch user %s: %w", userID, err)
 
-  // Good: stable error string + structured context
-  slog.ErrorContext(ctx, "fetch user", "user_id", userID, "error", err)
+  // Good: stable wrap; the boundary handler logs user_id structurally
   return fmt.Errorf("fetch user: %w", err)
   ```
 - Generic error matching: `errors.AsType[T]` (Go 1.26+) replaces the `var t *MyErr; errors.As(err, &t)` dance
@@ -108,13 +109,13 @@ if err != nil {
     return fmt.Errorf("connect: %w", err)
 }
 
-// Good: wrap and return; let the top-level caller log
+// Good: wrap (low-cardinality) and return; the boundary logs once
 if err != nil {
-    return fmt.Errorf("connect to %s: %w", addr, err)
+    return fmt.Errorf("connect: %w", err)
 }
 ```
 
-Wrap with context at each layer; log/handle only at the application boundary.
+Wrap with context at each layer; log/handle only at the application boundary. The boundary picks up variable data (`addr`, `user_id`, etc.) from `ctx`-scoped log fields or from a structured `slog.Error` call there — not from each wrap site.
 
 ### Context with Cause (Go 1.21+)