fix(rules): stop false-positive counterfeiter and Parse*Default findings

CHANGELOG.md

@@ -8,6 +8,10 @@ Please choose versions by [Semantic Versioning](http://semver.org/).
 * MINOR version when you add functionality in a backwards-compatible manner, and
 * PATCH version when you make backwards-compatible bug fixes.
 
+## Unreleased
+
+- fix: stop false-positive counterfeiter and Parse*Default findings (counterfeiter-directive-on-interface is now an over-inclusive mechanical anchor + judgment-tier adjudication; new-prefix-constructor-naming exempts Parse<X>Default)
+
 ## v0.45.0
 
 - feat: promote 1 m3 finding, three-model-agreed (m3+m2.7+deepseek all flagged `tts-mcp#13` `CHANGELOG.md` conventional-prefix violation) and deepseek-verified with runnable evidence, into the golden set (`golden-curated-2` → `golden-curated-3`, 157 → 158 entries). Of the 16 three-way agreement locations surfaced by [[Triangulate the Golden Set Across Models]], only 1 was genuinely new and verified — 3 others were re-statements of existing entries (caught by the aliasing guard, correctly dropped), 12 were rejected on defensible grounds (DRY, documented contract, pre-existing). Net effect on scores vs `golden-curated-2` (re-scored against the enlarged 158-entry set): m3 recall 0.066 → **0.072** (+1 hit, m3's own finding now counts), deepseek 0.109 → 0.109 (no change), opus 0.832 → **0.826** (−1 hit, the new entry is a finding opus missed — the self-match tautology loosening, as expected when the set grows). The triangulation principle's doing its job: agreement across models is necessary but not sufficient; aliasing checks remain essential to avoid silent double-counting

docs/go-architecture-patterns.md

@@ -16,8 +16,8 @@ The standard pattern for Go packages in the Services follows this structure:
 ### RULE go-architecture/counterfeiter-directive-on-interface (MUST)
 
 **Owner**: go-architecture-assistant
-**Applies when**: an exported `interface` declaration in a non-`main` Go service package (i.e. likely substituted via mocks in tests) has no preceding `//counterfeiter:generate` line. Concrete trigger: any package with `*_test.go` files that import a `mocks` package, plus every exported interface in that package.
-**Enforcement**: `rules/go/counterfeiter-directive-on-interface.yml`
+**Applies when**: an exported `interface` declaration in a non-`main` Go service package (i.e. likely substituted via mocks in tests). The ast-grep anchor fires on every exported interface — over-inclusive by design, because the `//counterfeiter:generate` directive commonly sits above the interface's doc comment (a block of several lines), which ast-grep's immediate-sibling relations cannot see. The agent adjudicates: reads the comment block directly above the interface and reports only when no `//counterfeiter:generate` directive is present.
+**Enforcement**: `rules/go/counterfeiter-directive-on-interface.yml` (mechanical first-pass — flags every exported interface declaration in production code) + judgment-tier LLM adjudication for whether a `//counterfeiter:generate` directive exists above the interface (the directive may sit above the interface's doc comment, which ast-grep's immediate-sibling relations cannot see)
 **Why**: Hand-written mocks drift silently — when the interface gains a method, the mock keeps satisfying the old surface and tests pass against a stale contract. The `//counterfeiter:generate` directive forces `go generate ./...` to regenerate the fake, so any drift surfaces immediately at code-gen time. Missing the directive means the fake isn't regenerated, the test doesn't exercise the new method, and the bug ships.
 
 #### Bad
@@ -65,7 +65,7 @@ type UserService interface {
 
 **Owner**: go-architecture-assistant
 **Applies when**: a Go function outside `pkg/factory/**` returns an exported interface or struct type and is intended to be the canonical construction site, but the function name does not start with `New`.
-**Enforcement**: `rules/go/new-prefix-constructor-naming.yml` (mechanical first-pass flags exported functions returning exported types without `New` prefix; `pkg/factory/**` excluded) + judgment-tier LLM adjudication to rule out `Create*` factories, helper functions, and non-constructor uses.
+**Enforcement**: `rules/go/new-prefix-constructor-naming.yml` (mechanical first-pass flags exported functions returning exported types without `New` prefix; `pkg/factory/**` excluded, and `Parse<X>Default` paired-default functions exempted so this rule cannot contradict go-parse/paired-parse-and-parsedefault) + judgment-tier LLM adjudication to rule out `Create*` factories, helper functions, and non-constructor uses.
 **Why**: `New*` is the universal Go signal for "this is the constructor — give it deps, get back a ready-to-use object". Without it, consumers can't tell `UserService(...)` from a regular function call; tooling (IDE search, godoc grouping, godoc renderers) treats it as ordinary; refactors don't surface the construction site. The convention is cheap; ignoring it costs every consumer a second of "wait, is this the constructor?". The `Create*` factory prefix is a deliberate exception scoped to `pkg/factory/**` — that's the factory pattern's home, not the service-construction site this rule covers.
 
 #### Bad

rule-tests/__snapshots__/go-architecture/counterfeiter-directive-on-interface-snapshot.yml

@@ -0,0 +1,108 @@
+id: go-architecture/counterfeiter-directive-on-interface
+snapshots:
+ ? |
+ package pkg
+ import "context"
+ // UserService is the user-facing surface.
+ type Service interface {
+ Create(ctx context.Context, user User) error
+ Get(ctx context.Context, id string) (*User, error)
+ }
+ : labels:
+ - source: |-
+ type Service interface {
+ Create(ctx context.Context, user User) error
+ Get(ctx context.Context, id string) (*User, error)
+ }
+ style: primary
+ start: 72
+ end: 198
+ - source: |-
+ interface {
+ Create(ctx context.Context, user User) error
+ Get(ctx context.Context, id string) (*User, error)
+ }
+ style: secondary
+ start: 85
+ end: 198
+ - source: Service
+ style: secondary
+ start: 77
+ end: 84
+ - source: |-
+ Service interface {
+ Create(ctx context.Context, user User) error
+ Get(ctx context.Context, id string) (*User, error)
+ }
+ style: secondary
+ start: 77
+ end: 198
+ ? |
+ package pkg
+ import "context"
+ //counterfeiter:generate -o mocks/service.go --fake-name Service . Service
+
+ // Service handles the user lifecycle.
+ // Implementations MUST be concurrency-safe.
+ type Service interface {
+ Create(ctx context.Context, user User) error
+ }
+ : labels:
+ - source: |-
+ type Service interface {
+ Create(ctx context.Context, user User) error
+ }
+ style: primary
+ start: 189
+ end: 262
+ - source: |-
+ interface {
+ Create(ctx context.Context, user User) error
+ }
+ style: secondary
+ start: 202
+ end: 262
+ - source: Service
+ style: secondary
+ start: 194
+ end: 201
+ - source: |-
+ Service interface {
+ Create(ctx context.Context, user User) error
+ }
+ style: secondary
+ start: 194
+ end: 262
+ ? |
+ package pkg
+ import "context"
+ //counterfeiter:generate -o mocks/service.go --fake-name Service . Service
+ type Service interface {
+ Create(ctx context.Context, user User) error
+ }
+ : labels:
+ - source: |-
+ type Service interface {
+ Create(ctx context.Context, user User) error
+ }
+ style: primary
+ start: 104
+ end: 177
+ - source: |-
+ interface {
+ Create(ctx context.Context, user User) error
+ }
+ style: secondary
+ start: 117
+ end: 177
+ - source: Service
+ style: secondary
+ start: 109
+ end: 116
+ - source: |-
+ Service interface {
+ Create(ctx context.Context, user User) error
+ }
+ style: secondary
+ start: 109
+ end: 177

rule-tests/counterfeiter-directive-on-interface-test.yml

@@ -0,0 +1,46 @@
+id: go-architecture/counterfeiter-directive-on-interface
+valid:
+ # Struct type — not an interface, not flagged
+ - |
+ package pkg
+ type UserService struct { db *DB }
+ # Unexported interface — no mockability obligation
+ - |
+ package pkg
+ type userService interface {
+ Get(ctx context.Context, id string) (User, error)
+ }
+invalid:
+ # Exported interface without directive — flagged; agent verifies no
+ # //counterfeiter:generate sits above it (through its doc comment)
+ - |
+ package pkg
+ import "context"
+ // UserService is the user-facing surface.
+ type Service interface {
+ Create(ctx context.Context, user User) error
+ Get(ctx context.Context, id string) (*User, error)
+ }
+ # Exported interface whose directive sits one line above — still anchored
+ # (over-inclusive first pass); the agent reads the block above and confirms
+ # the directive exists, so it is NOT reported as a finding.
+ - |
+ package pkg
+ import "context"
+ //counterfeiter:generate -o mocks/service.go --fake-name Service . Service
+ type Service interface {
+ Create(ctx context.Context, user User) error
+ }
+ # Exported interface with directive above its multi-line doc comment —
+ # still anchored (over-inclusive first pass); the agent confirms the
+ # directive exists above the doc block, so it is NOT reported.
+ - |
+ package pkg
+ import "context"
+ //counterfeiter:generate -o mocks/service.go --fake-name Service . Service
+
+ // Service handles the user lifecycle.
+ // Implementations MUST be concurrency-safe.
+ type Service interface {
+ Create(ctx context.Context, user User) error
+ }

rule-tests/new-prefix-constructor-naming-test.yml

@@ -11,6 +11,12 @@ valid:
  package p
  type userService struct{}
  func newUserService() *userService { return nil }
+ # Correct: Parse<X>Default paired-default function — exempt, naming mandated
+ # by paired-parse-and-parsedefault
+ - |
+ package p
+ type Config struct{ Host string }
+ func ParseConfigDefault(ctx context.Context, value string, def Config) Config { return def }
 invalid:
  # Make* prefix
  - |

rules/go/counterfeiter-directive-on-interface.yml

@@ -2,26 +2,36 @@ id: go-architecture/counterfeiter-directive-on-interface
 language: go
 severity: error
 message: |
- Every Go interface in a service package needs a //counterfeiter:generate
- directive so go generate regenerates the fake when the interface drifts.
- Without it, hand-written mocks drift silently — tests pass against a
- stale contract while the production code adds methods.
+ An exported Go interface in a service package was found. Each interface
+ substituted via mocks in tests needs a //counterfeiter:generate directive
+ so go generate regenerates the fake when the interface drifts. Without
+ it, hand-written mocks drift silently — tests pass against a stale
+ contract while the production code adds methods.
+ EXEMPT: interfaces that already carry a //counterfeiter:generate
+ directive above them (possibly separated from the interface by its doc
+ comment), and unexported/internal interfaces not used across package
+ boundaries. The agent checks the comment block directly above the
+ interface before reporting.
  See docs/go-architecture-patterns.md (RULE go-architecture/counterfeiter-directive-on-interface).
 rule:
- # ast-grep 0.43.0 shape: match a type_declaration that wraps a type_spec
- # wrapping an interface_type, with the type name starting with an
- # uppercase letter (Go exported convention), AND is NOT preceded
- # (follows in tree-sitter 'previous-sibling' direction) by a comment
- # containing 'counterfeiter:generate'.
+ # Anchor: an exported interface type declaration in production code.
+ # The agent adjudicates: reads the comment block directly above the
+ # interface and confirms whether a //counterfeiter:generate directive is
+ # present. The directive commonly sits above the interface's doc comment,
+ # and that block can be several lines long — ast-grep's `follows`/`precedes`
+ # relations only reach the IMMEDIATE previous sibling, so a doc comment
+ # between the directive and the interface makes the directive invisible to
+ # a mechanical `not.follows` check. The rule is therefore intentionally
+ # over-inclusive (flags every exported interface) and delegates the
+ # present-or-missing verdict to the adjudicator, matching the design of the
+ # sibling rule go-testing/counterfeiter-mocks-required and of the other
+ # judgment-backed detectors (main-test-with-compiles, external-call-logs-
+ # response, secret-fields-need-display-length, slog-not-glog-in-new-projects).
  #
- # Verified the relation direction during smoke: 'not.follows' means
- # "this node does NOT come after the matched sibling" — which for a
- # type_declaration whose comment is the previous sibling is what we want.
- #
- # Exportedness filter (added after PR #31 review): the rule only
- # applies to exported interfaces (those substituted via mocks in
- # tests). Unexported / internal interfaces (lowercase names) are
- # package-private and don't carry mockability obligations.
+ # Exportedness filter (kept from PR #31): the rule only anchors exported
+ # interfaces (uppercase name) — those substituted via mocks in tests.
+ # Unexported / internal interfaces are package-private and don't carry
+ # mockability obligations.
  kind: type_declaration
  has:
  kind: type_spec
@@ -31,10 +41,6 @@ rule:
  - has:
  kind: type_identifier
  regex: '^[A-Z]'
- not:
- follows:
- kind: comment
- regex: 'counterfeiter:generate'
 ignores:
  - "main.go"
  - "**/main.go"

rules/go/new-prefix-constructor-naming.yml

@@ -5,10 +5,15 @@ message: |
  Exported function returns an exported type but name does not start with 'New'.
  'New*' is the universal Go signal for "this is the constructor". Without it consumers
  cannot tell UserService(...) from a regular function; tooling cannot surface it as a constructor.
+ EXEMPT: Parse<X>Default paired-default functions — go-parse/paired-parse-and-parsedefault
+ mandates that exact name; a New prefix would contradict it. Also exempt helper functions
+ that happen to return an exported type, and test helpers.
  See docs/go-architecture-patterns.md (RULE go-architecture/new-prefix-constructor-naming).
 rule:
  # Anchor: exported function_declaration whose name does NOT start with "New" but whose result
- # is an exported type_identifier (interface or struct type alias).
+ # is an exported type_identifier (interface or struct type alias), AND is not a
+ # Parse<X>Default paired-default function (that naming is mandated by the
+ # paired-parse-and-parsedefault rule — firing on it here would contradict it).
  # The LLM adjudicates: rules out Create* factories in pkg/factory/** (covered by a different rule),
  # helper functions that happen to return an exported type, and test helpers.
  kind: function_declaration
@@ -18,7 +23,9 @@ rule:
  kind: identifier
  regex: '^[A-Z]'
  not:
- regex: '^New'
+ any:
+ - regex: '^New'
+ - regex: '^Parse[A-Z].*Default$'
  - has:
  field: result
  kind: type_identifier

rules/index.json

@@ -241,9 +241,9 @@
  },
  {
  "anchor": "go-architecture/counterfeiter-directive-on-interface",
- "applies_when": "an exported `interface` declaration in a non-`main` Go service package (i.e. likely substituted via mocks in tests) has no preceding `//counterfeiter:generate` line. Concrete trigger: any package with `*_test.go` files that import a `mocks` package, plus every exported interface in that package.",
+ "applies_when": "an exported `interface` declaration in a non-`main` Go service package (i.e. likely substituted via mocks in tests). The ast-grep anchor fires on every exported interface — over-inclusive by design, because the `//counterfeiter:generate` directive commonly sits above the interface's doc comment (a block of several lines), which ast-grep's immediate-sibling relations cannot see. The agent adjudicates: reads the comment block directly above the interface and reports only when no `//counterfeiter:generate` directive is present.",
  "doc_path": "docs/go-architecture-patterns.md",
- "enforcement": "`rules/go/counterfeiter-directive-on-interface.yml`",
+ "enforcement": "`rules/go/counterfeiter-directive-on-interface.yml` (mechanical first-pass — flags every exported interface declaration in production code) + judgment-tier LLM adjudication for whether a `//counterfeiter:generate` directive exists above the interface (the directive may sit above the interface's doc comment, which ast-grep's immediate-sibling relations cannot see)",
  "enforcement_type": "mechanical",
  "id": "go-architecture/counterfeiter-directive-on-interface",
  "level": "MUST",
@@ -253,7 +253,7 @@
  "anchor": "go-architecture/new-prefix-constructor-naming",
  "applies_when": "a Go function outside `pkg/factory/**` returns an exported interface or struct type and is intended to be the canonical construction site, but the function name does not start with `New`.",
  "doc_path": "docs/go-architecture-patterns.md",
- "enforcement": "`rules/go/new-prefix-constructor-naming.yml` (mechanical first-pass flags exported functions returning exported types without `New` prefix; `pkg/factory/**` excluded) + judgment-tier LLM adjudication to rule out `Create*` factories, helper functions, and non-constructor uses.",
+ "enforcement": "`rules/go/new-prefix-constructor-naming.yml` (mechanical first-pass flags exported functions returning exported types without `New` prefix; `pkg/factory/**` excluded, and `Parse<X>Default` paired-default functions exempted so this rule cannot contradict go-parse/paired-parse-and-parsedefault) + judgment-tier LLM adjudication to rule out `Create*` factories, helper functions, and non-constructor uses.",
  "enforcement_type": "mechanical",
  "id": "go-architecture/new-prefix-constructor-naming",
  "level": "MUST",