Visibility Pattern Guardian Skill

Purpose

Prevent 20-40% coverage loss from using private methods by enforcing the visibility pattern documented in project style guides.

Core Principle: "The skill DETECTS and ENFORCES. The style guides are the single source of truth."

---

Single Source of Truth

Authoritative Documentation:

• .claude/styles/scala-conventions.md (lines 111-160): Visibility Pattern for Testability
• .claude/styles/testing-standards.md (lines 81-154): Testability Pattern

This Skill's Role:

1. DETECT violations (private methods, inline lambdas)
2. REFERENCE style guides (point to exact line numbers)
3. ENFORCE by blocking during code review
4. AUTO-REFACTOR with user approval

NO DUPLICATION: All pattern guidance lives in style guides. This skill references them.

---

Activation Strategy

Layer 1: Light Warning During Writing (Real-time)

Trigger: User writes private def in Scala file

Response:

⚠️ "private def detected - may violate testability pattern.

📖 See authoritative guides:
   .claude/styles/scala-conventions.md (lines 111-160)
   .claude/styles/testing-standards.md (lines 81-154)

From the guide:
'This is a peer review rejection point - all new code must
follow this pattern unless specifically justified.'

(Full enforcement during code review phase)"

Purpose: Gentle education, point to where to learn

---

Layer 2: BLOCK During scala-ninja Review (Phase 2)

Trigger: scala-ninja code review detects visibility violations

Response:

❌ PEER REVIEW REJECTION POINT

Visibility pattern violations detected in GuardianActor.scala

📖 Reference: .claude/styles/scala-conventions.md (lines 111-160)
"Visibility Pattern for Testability (REQUIRED)"

From the guide:
"Keep methods public, apply visibility restriction at class/object level"

Violations found:
  ❌ Line 45: private def receiveBehavior(...): Behavior[Command]
     Cannot unit test this method directly

  ❌ Line 78: private def handleInitialize(...): Behavior[Command]
     Cannot unit test this method directly

  ❌ Line 102: private def handleGetQueueActor(...): Behavior[Command]
     Cannot unit test this method directly

Impact:
- 8 methods untestable → ~180 lines of code without regression protection
- Estimated coverage loss: 30-40%
- This was proven in GuardianActor: 45% → 85% coverage after fix

Pattern from guide (line 119):
  ✅ Use private[core] on object
  ✅ Keep methods public (no private keyword)

Auto-refactoring to match style guide pattern...

Purpose: Hard enforcement at quality gate, comprehensive explanation

---

Detection Patterns

Pattern 1: Private Methods in Objects/Classes

Detect:

// VIOLATION
object GuardianActor {
  private def receiveBehavior(...): Behavior[Command] = { ... }
  private def handleInitialize(...): Behavior[Command] = { ... }
}

Reference Guide:

📖 .claude/styles/scala-conventions.md (line 119)

From the guide:
"CORRECT: Methods public, object scoped to module
private[core] object TestExecutionActor {
  def setupBehavior(...): Behavior[Command] = { ... }
}"

Auto-Refactor:

// REFACTORED TO MATCH GUIDE
private[core] object GuardianActor {
  def receiveBehavior(...): Behavior[Command] = { ... }
  def handleInitialize(...): Behavior[Command] = { ... }
}

---

Pattern 2: Private Business Logic Methods

Detect:

// VIOLATION
class MyService {
  private def calculateDiscount(price: Double): Double = { ... }
}

Reference Guide:

📖 .claude/styles/testing-standards.md (lines 121-127)

From the guide:
"All helper methods should have direct unit tests.
Do NOT test helper methods only indirectly through integration tests."

Auto-Refactor:

// REFACTORED TO MATCH GUIDE
private[services] class MyService {
  def calculateDiscount(price: Double): Double = { ... }
}

---

Pattern 3: Inline Lambdas with Business Logic

Detect:

// VIOLATION: Business logic buried in lambda
List("a", "b", "c").filter(v => v.equals(outerScopedString))

Reference Guide:

📖 .claude/styles/testing-standards.md (testability pattern)

From discussions:
"Extract business logic into named functions ('laws' - pure functions).
One can be tested, one cannot without testing the List operations."

Suggest Refactor:

// REFACTORED: Business logic extracted to testable law
val matchesOuterString: String => Boolean =
  v => v.equals(outerScopedString)

List("a", "b", "c").filter(matchesOuterString)

Explanation:

💡 "Testability Pattern: Laws vs Unit Tests

BEFORE (untestable):
  List("a", "b").filter(v => v.equals(outerScopedString))
  - Business logic (equals check) is inline
  - Can only test by testing entire List operation
  - Logic is not discoverable or named

AFTER (testable law):
  val matchesOuterString: String => Boolean = v => v.equals(outerScopedString)
  List("a", "b").filter(matchesOuterString)
  - matchesOuterString is a 'law' (pure function)
  - Can be unit tested independently
  - Business logic is discoverable and named
  - Filter operation uses the law

From style guide philosophy:
'Make code discoverable. Extract business logic into named functions.'"

---

Auto-Refactoring Protocol

Request Approval

Template:

❌ Visibility violations detected in GuardianActor.scala

I can automatically refactor to match the pattern from:
📖 .claude/styles/scala-conventions.md (lines 111-160)

BEFORE (violates guide):
  object GuardianActor {
    private def receiveBehavior(...): Behavior[Command] = { ... }
    private def handleInitialize(...): Behavior[Command] = { ... }
    private def handleGetQueueActor(...): Behavior[Command] = { ... }
  }

AFTER (matches guide):
  private[core] object GuardianActor {
    def receiveBehavior(...): Behavior[Command] = { ... }
    def handleInitialize(...): Behavior[Command] = { ... }
    def handleGetQueueActor(...): Behavior[Command] = { ... }
  }

Changes:
  - Add private[core] to object (line 119 of guide)
  - Remove private from 8 methods (lines 120-131 of guide)
  - Makes all methods unit testable

Benefits (from guide):
  ✅ Comprehensive unit testing: All helper methods directly testable
  ✅ Module encapsulation: private[core] prevents external access
  ✅ Test coverage: Enables 85%+ coverage without hacky workarounds
  ✅ Refactoring safety: Helper methods have regression protection

Approve auto-refactor? (yes/no)

Apply Refactor

If user approves ("yes"):

"✅ Applying visibility pattern from style guide...

Changes made:
  ✓ GuardianActor.scala: Added private[core] to object
  ✓ GuardianActor.scala: Removed private from 8 methods

Recompiling to verify...

  mvn compile -pl test-probe-core -q

✅ Code compiles successfully after refactoring

Pattern applied matches:
  .claude/styles/scala-conventions.md (line 119)

Violations resolved. Ready to proceed to testing phase."

If user defers ("later", "not now"):

✅ Noted. Continuing for now.

⚠️ WARNING: This WILL block at Phase 2 (scala-ninja code review)

From the style guide:
"This is a peer review rejection point - all new code must
follow this pattern unless specifically justified."

scala-ninja will require this fix before proceeding to Phase 3.

If user rejects ("no"):

⚠️ Visibility pattern violation remains.

This WILL block at Phase 2 (scala-ninja code review).

From the style guide (line 116):
"This is a peer review rejection point - all new code must
follow this pattern unless specifically justified."

Do you have a justification for the exception?
Valid exceptions:
1. Internal state that genuinely must be hidden
2. Temporary/intermediate methods during refactoring
3. Security-sensitive operations

If yes, I'll document the exception for peer review.
If no, this will block during code review.

---

Scope Guidelines (From Style Guide)

Reference: .claude/styles/scala-conventions.md (lines 150-154)

Scope selection:
- private[core]       → core module actors/services
- private[common]     → common module utilities
- private[services]   → services module implementations
- private[glue]       → Cucumber step definitions
- private[actors]     → limit to actors subpackage only

Auto-detect correct scope:

File: test-probe-core/src/main/scala/.../GuardianActor.scala
Module: test-probe-core
Suggested scope: private[core]

File: test-probe-common/src/main/scala/.../Utils.scala
Module: test-probe-common
Suggested scope: private[common]

---

Integration with scala-ninja

Phase 2: Code Review Coordination

scala-ninja review:
  "Reviewing code against project standards...

  References:
  - .claude/styles/scala-conventions.md
  - .claude/styles/testing-standards.md
  - .claude/styles/akka-patterns.md

  Checking visibility pattern (scala-conventions.md lines 111-160)...

  ❌ Violations found: GuardianActor.scala (8 private methods)

  Triggering visibility-pattern-guardian for enforcement..."

visibility-pattern-guardian activates:
  [Blocks with detailed violation report]
  [References style guide sections]
  [Offers auto-refactoring]

User approves refactor:
  [Applies changes]
  [Recompiles]
  [Marks violations resolved]

scala-ninja continues:
  "✅ Visibility pattern violations resolved.
   Continuing review..."

---

Communication Templates

Detection (Layer 1 - Light Warning)

⚠️ Testability pattern suggestion

Detected: private def in actor code

📖 Pattern reference:
   .claude/styles/scala-conventions.md (lines 111-160)
   "Visibility Pattern for Testability (REQUIRED)"

From the guide:
"Keep methods public, apply visibility restriction at class/object level"

Quick example (from guide, line 119):
  ✅ private[core] object MyActor {
       def myMethod() = { ... }  // Public method, testable
     }

This will be enforced during code review phase (Phase 2).

Enforcement (Layer 2 - BLOCK)

❌ PEER REVIEW REJECTION POINT

File: GuardianActor.scala
Issue: 8 private methods violate testability pattern

📖 Authoritative source:
   .claude/styles/scala-conventions.md (lines 111-160)

From the guide (line 116):
"This is a peer review rejection point - all new code must
follow this pattern unless specifically justified."

Coverage impact (from guide):
  ❌ GuardianActor with private methods: 45% coverage (methods untestable)
  ✅ GuardianActor with visibility pattern: 85%+ coverage (all testable)

Impact: -40% coverage from this one violation

Pattern required (from guide):
  private[core] object GuardianActor {
    def receiveBehavior(...) = { ... }  // ✅ Testable
    def handleInitialize(...) = { ... } // ✅ Testable
  }

Auto-refactoring to match guide pattern...

Lambda Extraction Guidance

💡 Testability Pattern: Extract Business Logic to Named Functions

Detected:
  users.filter(u => u.age > 18 && u.isActive && !u.isBlocked)

Issue:
  Business logic (eligibility rules) is inline and untestable

📖 Principle (from discussions + testing-standards.md):
"Extract business logic into named functions ('laws').
One can be tested, one cannot without testing List operations."

Suggested refactor:
  val isEligibleUser: User => Boolean =
    u => u.age > 18 && u.isActive && !u.isBlocked

  users.filter(isEligibleUser)

Benefits:
  ✅ isEligibleUser is a 'law' (pure function)
  ✅ Can be unit tested independently:
     isEligibleUser(User(age=17, ...)) shouldBe false
  ✅ Business logic is discoverable and named
  ✅ Reusable across codebase

Shall I refactor this? (yes/no)

---

Exceptions (Require Justification)

From guide (.claude/styles/scala-conventions.md line 156):

Exceptions requiring justification in PR:
- Internal state that genuinely must be hidden
- Temporary/intermediate methods during refactoring
- Security-sensitive operations

If user claims exception:

"⚠️ Exception to visibility pattern requested

You indicated this private method is an exception.

From the style guide (line 156), valid exceptions are:
1. Internal state that genuinely must be hidden
2. Temporary/intermediate methods during refactoring
3. Security-sensitive operations

Which exception applies to this case?

Please note: This exception will need justification in code review/PR.
The default is to follow the pattern unless there's a compelling reason."

---

Key Insights

Laws vs Unit Tests

Law (Pure function - testable independently):

val isEligible: User => Boolean = u => u.age > 18
// Can test: isEligible(User(age=17)) shouldBe false

Unit Test (Testing implementation detail):

users.filter(u => u.age > 18)
// Must test entire List operation to verify filter logic

Principle: Extract business logic into laws (named functions) that can be tested independently.

---

Why This Matters (From Guide)

Coverage Impact:

GuardianActor with private methods: 45% coverage
GuardianActor with visibility pattern: 85%+ coverage

Difference: 40% more regression protection

Proven: This pattern was implemented in actual project actors (GuardianActor, TestExecutionActor) with measurable results.

---

Exit Criteria

Violations Resolved When:

✅ All private methods refactored to public with module scoping
✅ All objects/classes have appropriate private[module] scope
✅ Complex inline lambdas extracted to named functions
✅ Code recompiles successfully
✅ Pattern matches examples from style guides

Only Then: Proceed to Phase 3 (Specifications)

---

Success Indicators

Skill is working when:

• ✅ No private method violations ship
• ✅ Coverage consistently achieves 85%+ for actors
• ✅ Developers learn to read style guides (education)
• ✅ "Laws" pattern adopted for business logic
• ✅ Peer review rejections eliminated

---

Style Guide References

Quick Links:

• Visibility Pattern: .claude/styles/scala-conventions.md (lines 111-160)
• Testability Pattern: .claude/styles/testing-standards.md (lines 81-154)
• Helper Method Testing: .claude/styles/testing-standards.md (lines 137-154)

Always reference these - they are the single source of truth.

---

Version: 1.0 Last Updated: 2025-10-21 Based on: working/skills-suite/visibility-pattern-guardian-design.md