loom-hermit-patterns

Hermit Patterns Reference

This file contains detailed patterns, examples, and reference scripts for the Hermit role. It is meant to be consulted when needed, not loaded as primary instructions.

When to use this file:

• Looking for specific code smell examples
• Need detailed random file review workflow
• Implementing goal discovery scripts
• Checking command reference syntax
• Need worktree cleanup implementation

Primary instructions: See hermit.md for core role definition and workflow.

Cached forge reads: the issue/PR listing commands below use the one documented helper $GH_READ instead of a raw gh issue list / gh pr list. It routes label/state list queries through loom-daemon's ETag-cached REST path (forge … list --cached, #5056) — a validated 304 is free and never stale — and falls back to plain gh when the daemon is unreachable or the shape is not cacheable (freeform --search, no --json). Resolve it once per session:

GH_READ="gh"
_ghc="$(git rev-parse --show-toplevel 2>/dev/null)/.loom/scripts/gh-cached"
if [[ -x "$_ghc" ]] && "$_ghc" --version >/dev/null 2>&1; then GH_READ="$_ghc"; fi

Full policy: .loom/docs/gh-cached.md.

---

Contents

• ⚠️ --body @path Does NOT Expand — It Posts the Literal String
• Detailed Code Smell Examples
• Analysis Scripts
• Random File Review - Detailed Workflow
• Goal Discovery Scripts
• Creating Removal Proposals - Full Templates
• Example Analysis Session
• Commands Reference

⚠️ --body @path Does NOT Expand — It Posts the Literal String

If you post a comment via gh issue comment / gh pr comment / gh api ... comments from a scratch file, --body @path (and gh api -f body=@path) posts the literal string @path, not the file's contents. Full pitfall, incident citation, and fixes: comment-body-literal-path.md.

---

Detailed Code Smell Examples

Look for these patterns that often indicate bloat:

1. Unnecessary Abstraction

// BAD: Over-abstracted
class DataFetcherFactory {
  createFetcher(): DataFetcher {
    return new ConcreteDataFetcher(new HttpClient());
  }
}

// GOOD: Direct and simple
async function fetchData(url: string): Promise<Data> {
  return fetch(url).then(r => r.json());
}

2. One-Method Classes

// BAD: Class with single method
class UserValidator {
  validate(user: User): boolean {
    return user.email && user.name;
  }
}

// GOOD: Just a function
function validateUser(user: User): boolean {
  return user.email && user.name;
}

3. Unused Configuration

// Configuration options that are never changed from defaults
const config = {
  maxRetries: 3,        // Always 3 in practice
  timeout: 5000,        // Never customized
  enableLogging: true   // Never turned off
};

4. Generic Utilities That Are Used Once

// Utility function used in exactly one place
function mapArrayToObject<T>(arr: T[], keyFn: (item: T) => string): Record<string, T>

5. Premature Generalization

// Supporting 10 database types when only using one
interface DatabaseAdapter { /* complex interface */ }
class PostgresAdapter implements DatabaseAdapter { /* ... */ }
class MySQLAdapter implements DatabaseAdapter { /* never used */ }
class MongoAdapter implements DatabaseAdapter { /* never used */ }

Additional Code Smells to Watch

// One-method class (should be function)
class DataTransformer {
  transform(data: Data, options: Options): Result {
    // ...implementation
  }
}

// Over-parameterized function
function process(a, b, c, d, e, f, g, h) { /* ... */ }

// Unnecessary abstraction
interface IDataFetcher {
  fetch(): Data;
}
class DataFetcherFactory {
  create(): IDataFetcher { /* ... */ }
}

// Generic utility used once
function mapToObject<T>(arr: T[], keyFn: (item: T) => string) { /* only 1 caller */ }

// Commented-out code
// function oldMethod() {
//   return "deprecated behavior";
// }

6. Parallel Drift

When two or more classes/modules solve the same domain problem with different names and APIs, only one is used at runtime. The other is dead weight referenced only by its own tests.

# BAD: Two session managers solving the same problem
# file: src/sessions/session_manager.py
class SessionManager:
    def create_session(self, user_id: str) -> Session:
        return Session(user_id=user_id, token=generate_token())
    def destroy_session(self, session_id: str) -> None:
        self._store.delete(session_id)

# file: src/core/session_handler.py
class SessionHandler:
    def new_session(self, uid: str) -> dict:
        return {"uid": uid, "tok": make_token(), "ts": time.time()}
    def end_session(self, sid: str) -> bool:
        return self._db.remove(sid)

# GOOD: Single canonical implementation
# file: src/sessions/session_manager.py
class SessionManager:
    def create_session(self, user_id: str) -> Session:
        return Session(user_id=user_id, token=generate_token())
    def destroy_session(self, session_id: str) -> None:
        self._store.delete(session_id)

// BAD: Parallel drift in TypeScript
// file: src/utils/config-loader.ts
export class ConfigLoader {
  load(path: string): Config { /* ... */ }
}
// file: src/lib/settings-reader.ts
export class SettingsReader {
  read(filepath: string): Settings { /* ... */ }
}

// GOOD: One config module
// file: src/config/loader.ts
export class ConfigLoader {
  load(path: string): Config { /* ... */ }
}

Detection scripts:

# Python: Find classes with semantically similar names
rg "^class \w*(Session|Manager|Handler|Builder|Parser|Validator|Config|Store|Cache|Client)\w*" --type py -n | \
  sed 's/.*class \([A-Za-z]*\).*/\1/' | sort | uniq -d

# TypeScript: Same check
rg "^(export )?(class|interface) \w*(Session|Manager|Handler|Builder|Parser|Validator|Config|Store|Cache|Client)\w*" --type ts -n | \
  sed 's/.*\(class\|interface\) \([A-Za-z]*\).*/\2/' | sort | uniq -d

# Rust: Find structs with similar domain names
rg "^pub struct \w*(Session|Manager|Handler|Builder|Parser|Validator|Config|Store|Cache|Client)\w*" --type rust -n | \
  sed 's/.*struct \([A-Za-z]*\).*/\1/' | sort | uniq -d

# After finding candidates, verify: is one only referenced by its own tests?
# For each candidate file:
rg "SessionHandler" --type py --files-with-matches | grep -v test
# If only test files reference it, it's likely parallel drift

7. Stub Theater

Methods with rich interfaces (docstrings, type signatures, multiple parameters) that return hardcoded literal values. The external contract promises complex behavior, but the implementation is inert.

# BAD: Rich interface, stub implementation
class ChangeDetector:
    def _is_component_modified(self, component: Component, baseline: Snapshot) -> bool:
        """Check if a component has been modified since the baseline snapshot.

        Compares component hash, metadata, and dependency graph against
        the baseline to detect meaningful changes. Ignores whitespace-only
        and comment-only changes.

        Args:
            component: The component to check for modifications.
            baseline: The reference snapshot to compare against.

        Returns:
            True if the component has meaningful modifications.
        """
        return False  # <-- entire subsystem is inert

    def _can_preserve(self, component: Component) -> bool:
        """Determine if a component can be safely preserved during rebuild.

        Validates three criteria:
        1. Component has no circular dependencies
        2. Component's interface hasn't changed
        3. All downstream consumers are compatible

        Returns:
            True if safe to preserve, False if rebuild required.
        """
        return True  # <-- validation never actually runs

# GOOD (preferred): Finish the feature — check if dependencies now exist
class ChangeDetector:
    def _is_component_modified(self, component: Component, baseline: Snapshot) -> bool:
        current_hash = component.content_hash()
        baseline_hash = baseline.get_hash(component.id)
        return current_hash != baseline_hash

# GOOD (alternative): Remove if the feature is clearly abandoned

// BAD: Stub theater in TypeScript
class PermissionChecker {
  /**
   * Validates user has required permissions for the operation.
   * Checks role hierarchy, resource ownership, and temporal constraints.
   */
  canAccess(user: User, resource: Resource, operation: Operation): boolean {
    return true; // Everyone can access everything
  }
}

// GOOD (preferred): Finish the feature
function canAccess(user: User, resource: Resource, operation: Operation): boolean {
  return user.roles.some(role =>
    resource.allowedRoles.includes(role) && role.permits(operation)
  );
}

// GOOD (alternative): Remove if clearly abandoned

Detection scripts:

# Python: Find methods with docstrings that just return a literal
rg "def \w+\(self" -A 8 --type py |   awk '/def /{found=1; block=""} found{block=block"
"$0} /return (False|True|None|""|\[\]|\{\}|0)$/{if(found) print block; found=0}'

# Simpler heuristic: methods whose only non-docstring line is a return literal
rg "def \w+\(self" -A 5 --type py | grep -B 1 "return (False|True|None)$"

# TypeScript: methods returning hardcoded values
rg "(public|private|protected)?\s+\w+\(.*\).*\{" -A 3 --type ts |   grep -B 1 "return (false|true|null|\[\]|\{\}|0|"")"

# Rust: functions returning hardcoded values (less common but possible)
rg "fn \w+\(" -A 5 --type rust | grep -B 2 "^\s*(false|true|None|0|"")"

8. Framework Scaffolding

Validation, processing, or transformation pipelines that are structurally complete but operate on empty data. A builder/context/factory method returns empty collections while downstream consumers treat the result as meaningful.

# BAD: Pipeline processes nothing
class PCBValidator:
    def validate(self, design_file: str) -> ValidationResult:
        """Run full validation pipeline on PCB design."""
        context = self._build_context(design_file)
        errors = self._check_design_rules(context)
        warnings = self._check_manufacturing_constraints(context)
        return ValidationResult(errors=errors, warnings=warnings)

    def _build_context(self, design_file: str) -> dict:
        # TODO: Implement actual PCB parsing
        return {
            "layers": {},        # empty
            "components": [],    # empty
            "nets": [],          # empty
            "rules": {}          # empty
        }
        # _check_design_rules and _check_manufacturing_constraints
        # iterate over empty collections -- validation always passes

# GOOD: Either implement or mark as not-yet-functional
class PCBValidator:
    def validate(self, design_file: str) -> ValidationResult:
        raise NotImplementedError("PCB validation not yet implemented")

// BAD: Framework scaffolding in TypeScript
class DataPipeline {
  async process(input: RawData): Promise<ProcessedData> {
    const enriched = await this.enrich(input);
    const validated = this.validate(enriched);
    const transformed = this.transform(validated);
    return transformed;
  }

  private async enrich(data: RawData): Promise<EnrichedData> {
    // TODO: Connect to enrichment service
    return { ...data, metadata: {}, annotations: [] };
  }
}

// GOOD: Be explicit about what's implemented
class DataPipeline {
  async process(input: RawData): Promise<ProcessedData> {
    // Enrichment not yet implemented - pass through
    const validated = this.validate(input);
    return this.transform(validated);
  }
}

Detection scripts:

# Python: Find methods returning empty collections with TODOs nearby
rg "return \{\}" -B 5 --type py | grep -B 4 "TODO\|FIXME\|NotImplemented"
rg "return \[\]" -B 5 --type py | grep -B 4 "TODO\|FIXME\|NotImplemented"

# Find factory/builder methods returning empty dicts/lists
rg "(def (build|create|make|get|load)_\w+)" -A 10 --type py |   grep -B 5 "return \(\{\}\|\[\]\)"

# TypeScript: Same patterns
rg "return \{\}" -B 5 --type ts | grep -B 4 "TODO\|FIXME"
rg "return \[\]" -B 5 --type ts | grep -B 4 "TODO\|FIXME"

# Rust: Empty collections in builder methods
rg "fn (build|create|new|load)_?\w*" -A 10 --type rust |   grep -B 5 "Vec::new()\|HashMap::new()\|BTreeMap::new()"

9. Stateless Ceremony

Classes where __init__ is pass/empty/missing and no methods assign to self.*. These are functions wearing a class costume -- instantiation is ceremony with no purpose.

# BAD: Class with no instance state
class PatternAdapter:
    def __init__(self):
        pass  # No state

    @staticmethod
    def convert_glob_to_regex(pattern: str) -> str:
        return fnmatch.translate(pattern)

    @staticmethod
    def match(text: str, pattern: str) -> bool:
        return fnmatch.fnmatch(text, pattern)

    def adapt(self, patterns: list[str]) -> list[re.Pattern]:
        return [re.compile(self.convert_glob_to_regex(p)) for p in patterns]

# GOOD: Module-level functions
def convert_glob_to_regex(pattern: str) -> str:
    return fnmatch.translate(pattern)

def match(text: str, pattern: str) -> bool:
    return fnmatch.fnmatch(text, pattern)

def adapt_patterns(patterns: list[str]) -> list[re.Pattern]:
    return [re.compile(convert_glob_to_regex(p)) for p in patterns]

# NOT a stateless ceremony: Dispatch-table class (uses self for method dispatch)
class CommandRouter:
    """Routes commands to handler methods. Stateless by design --
    the class provides method-dispatch organization, not instance state."""

    def route(self, command: str, args: dict) -> Result:
        handler = {
            "create": self.handle_create,
            "update": self.handle_update,
            "delete": self.handle_delete,
            "list": self.handle_list,
        }.get(command)
        if not handler:
            return Result(error=f"Unknown command: {command}")
        return handler(args)

    def handle_create(self, args: dict) -> Result:
        return Result(data=create_record(args))

    def handle_update(self, args: dict) -> Result:
        return Result(data=update_record(args["id"], args))

    def handle_delete(self, args: dict) -> Result:
        return Result(data=delete_record(args["id"]))

    def handle_list(self, args: dict) -> Result:
        return Result(data=list_records(args.get("filter")))
    # This class has no self.x = assignments but DOES use self.method()
    # for internal dispatch. Converting to module functions would lose
    # the dispatch-table organization. Do NOT flag this.

// BAD: Stateless class in TypeScript
export class SnapshotBuilder {
  // No constructor, no properties
  build(data: RawData): Snapshot {
    return { timestamp: Date.now(), data: this.normalize(data) };
  }

  private normalize(data: RawData): NormalizedData {
    return Object.fromEntries(
      Object.entries(data).map(([k, v]) => [k.toLowerCase(), v])
    );
  }
}

// GOOD: Export functions directly
export function buildSnapshot(data: RawData): Snapshot {
  return { timestamp: Date.now(), data: normalizeData(data) };
}

function normalizeData(data: RawData): NormalizedData {
  return Object.fromEntries(
    Object.entries(data).map(([k, v]) => [k.toLowerCase(), v])
  );
}

Detection scripts:

# Python: Find classes with no instance state (AST-based, excludes dispatch-table classes)
python3 -c "
import ast, sys, os
for root, dirs, files in os.walk('.'):
    dirs[:] = [d for d in dirs if not d.startswith('.') and d != 'node_modules']
    for f in files:
        if not f.endswith('.py'): continue
        path = os.path.join(root, f)
        try:
            tree = ast.parse(open(path).read())
        except: continue
        for node in ast.walk(tree):
            if not isinstance(node, ast.ClassDef): continue
            has_self_assign = any(
                isinstance(n, ast.Assign) and
                any(isinstance(t, ast.Attribute) and
                    isinstance(t.value, ast.Name) and t.value.id == 'self'
                    for t in n.targets)
                for n in ast.walk(node)
            )
            if has_self_assign:
                continue  # Has instance state -- not a stateless ceremony
            # Exclusion 1: Internal method dispatch (self.method() calls)
            has_self_method_call = any(
                isinstance(n, ast.Call) and
                isinstance(getattr(n, 'func', None), ast.Attribute) and
                isinstance(getattr(n.func, 'value', None), ast.Name) and
                n.func.value.id == 'self'
                for n in ast.walk(node)
            )
            if has_self_method_call:
                continue  # Uses internal dispatch -- likely a namespace
            # Exclusion 2: Method count threshold (10+ methods = namespace)
            method_count = sum(
                1 for n in ast.walk(node)
                if isinstance(n, (ast.FunctionDef, ast.AsyncFunctionDef))
            )
            if method_count >= 10:
                continue  # Too many methods to practically convert
            # Exclusion 3: Dispatch-table pattern (self.method refs inside dict/list/set)
            has_dispatch_table = False
            for n in ast.walk(node):
                if not isinstance(n, (ast.Dict, ast.List, ast.Set)):
                    continue
                for val in ast.walk(n):
                    if (isinstance(val, ast.Attribute) and
                        isinstance(getattr(val, 'value', None), ast.Name) and
                        val.value.id == 'self'):
                        has_dispatch_table = True
                        break
                if has_dispatch_table:
                    break
            if has_dispatch_table:
                continue  # Builds dispatch table from self.method references
            print(f'{path}:{node.lineno}: {node.name} (no instance state)')
"

# TypeScript: classes with no 'this.' property assignments
rg "class \w+" --type ts -l | while read file; do
  class_count=$(rg "class \w+" "$file" --count 2>/dev/null || echo 0)
  this_count=$(rg "this\.\w+\s*=" "$file" --count 2>/dev/null || echo 0)
  if [ "$this_count" -eq 0 ] && [ "$class_count" -gt 0 ]; then
    echo "$file: $class_count classes, 0 instance state assignments"
  fi
done

# Rust: structs with no fields (unit structs used as namespaces)
rg "^pub struct \w+;$" --type rust -n
rg "^pub struct \w+ \{\}$" --type rust -n

---

Analysis Scripts

Dependency Analysis

# Frontend: Check for unused npm packages
cd <repo-root>
npx depcheck

# Backend: Check Cargo.toml vs actual usage
rg "use.*::" --type rust | cut -d':' -f3 | sort -u

Dead Code Detection

# Find exports with no external references
rg "export (function|class|const|interface)" --type ts -n

# For each export, check if it's imported elsewhere
# If no imports found outside its own file, it's dead code

Complexity Metrics

# Find large files (often over-engineered)
find . -name "*.ts" -o -name "*.rs" | xargs wc -l | sort -rn | head -20

# Find files with many imports (tight coupling)
rg "^import" --count | sort -t: -k2 -rn | head -20

Historical Analysis

# Find files that haven't changed in a long time (potential for removal)
git log --all --format='%at %H' --name-only | \
  awk 'NF==2{t=$1; next} {print t, $0}' | \
  sort -k2 | uniq -f1 | sort -rn | tail -20

# Find features added but never modified (possible unused)
git log --diff-filter=A --name-only --pretty=format: | \
  sort -u | while read file; do
    commits=$(git log --oneline -- "$file" | wc -l)
    if [ $commits -eq 1 ]; then
      echo "$file (only 1 commit - added but never touched)"
    fi
  done

---

Random File Review - Detailed Workflow

What Makes a Good Candidate

High-value targets for random review:

Indicator	Threshold	Why It Matters
File Size	> 300 lines	May be doing too much, candidate for splitting
Imports	10+ imports	Tight coupling, complex dependencies
Nesting Depth	4+ levels	Complex control flow, hard to reason about
Class Methods	1-2 methods	Should probably be functions
Parameters	5+ params	Over-parameterized, needs refactoring
Comments/Code Ratio	> 30%	Either over-documented or has dead code
Cyclomatic Complexity	High branching	Many if/else, switch, match statements

What to Skip

Don't waste time on:

• Tests - Verbosity is acceptable, test clarity > brevity
• Type definitions - Long type files are normal (**/*.d.ts, interfaces)
• Generated code - Can't simplify auto-generated files
• Small files - < 50 lines are already concise
• Recent files - < 2 weeks old, let them stabilize
• Config files - Often need all options even if unused
• Already flagged - Check existing issues to avoid duplicates

# Before creating an issue, check for duplicates
"$GH_READ" issue list --search "filename.ts" --state=open

Example Decision Process

Scenario 1: Good Candidate

# Random file: src/lib/data-transformer.ts
$ wc -l src/lib/data-transformer.ts
487 src/lib/data-transformer.ts

$ head -30 src/lib/data-transformer.ts | grep "import" | wc -l
15

$ rg "class " src/lib/data-transformer.ts
export class DataTransformer {

$ rg "transform\(" src/lib/data-transformer.ts --count
1

# Decision: 487 lines, 15 imports, class with complex transform method
# -> CREATE ISSUE: "Simplify data-transformer: extract logic, reduce params"

Scenario 2: Already Simple

# Random file: src/lib/logger.ts
$ wc -l src/lib/logger.ts
67 src/lib/logger.ts

$ head -20 src/lib/logger.ts
// Clean, well-structured logger utility
// Minimal dependencies, clear purpose

# Decision: 67 lines, clean structure, does one thing well
# -> SKIP: Already simple and focused

Scenario 3: Marginal Value

# Random file: src/components/Button.tsx
$ wc -l src/components/Button.tsx
142 src/components/Button.tsx

# Scan shows: Could reduce from 142 to ~120 lines
# Effort: 1 hour, LOC saved: ~20 lines, Risk: UI changes

# Decision: Small improvement, low ROI
# -> SKIP: Not worth the effort for 20 line reduction

Random File Review Issue Template

./.loom/scripts/create-issue.sh --title "Simplify <filename>: <specific improvement>" --body "$(cat <<'EOF'
## What to Simplify

<file-path> - <specific bloat identified>

## Why It's Bloat

<evidence from your scan>

Examples:
- "487 lines with 15 imports - class could be 3 simple functions"
- "One-method class with 8 parameters - should be a pure function"
- "50 lines of commented-out code from 6 months ago"

## Evidence