Pre Ship Review

Pre-Ship Review

Structured quality review before shipping code at any checkpoint: PRs, releases, milestones. Catches the failures that occur at integration boundaries -- where contracts, examples, constants, and tests must all agree.

Core thesis: AI-generated code excels at isolated components but fails systematically at boundaries between components. This skill systematically checks those boundaries.

Self-Evolving Skill: This skill improves through use. If instructions are wrong, parameters drifted, or a workaround was needed — fix this file immediately, don't defer. Only update for real, reproducible issues.

When to Use This Skill

Use before any significant code shipment:

• Pull requests with multiple new modules that wire together
• Releases combining work from multiple contributors or branches
• Milestones where quality gates must pass before proceeding
• Any checkpoint where code with examples, constants across files, or interface extensions needs validation

NOT needed for: single-file cosmetic changes, documentation-only updates, dependency bumps.

---

TodoWrite Task Templates

MANDATORY: Select and load the appropriate template before starting review.

Template A: New Feature Ship

1. Detect changed files and scope (git diff --name-only against base branch)
2. Run Phase 1 - External tool checks (Pyright, Vulture, import-linter, deptry, Semgrep, Griffe)
3. Run Phase 2 - cc-skills orchestration (code-hardcode-audit, dead-code-detector, pr-gfm-validator)
4. Run Phase 2 conditional checks based on file types changed
5. Phase 3 - Verify every function parameter has at least one caller passing it by name
6. Phase 3 - Verify every config/example parameter maps to an actual function kwarg
7. Phase 3 - Check for architecture boundary violations (hardcoded feature lists, cross-layer coupling)
8. Phase 3 - Verify domain constants and formulas are correct (cross-reference cited sources)
9. Phase 3 - Audit test quality - do tests test what they claim (not side effects)?
10. Phase 3 - Check for implicit dependencies between new components
11. Phase 3 - Look for O(n^2) patterns where O(n) suffices
12. Phase 3 - Verify error messages give actionable guidance
13. Phase 3 - Confirm examples reflect actual behavior, not aspirational behavior
14. Compile findings report with severity and suggested fixes

Template B: Bug Fix Ship

1. Verify the fix addresses root cause, not symptom
2. Verify the fix does not mask information flow
3. Check that new test reproduces the original bug (fails without fix)
4. Run Phase 1 - External tool checks on changed files
5. Run Phase 2 - cc-skills checks on changed files
6. Verify constants consistency if any values changed
7. Compile findings report

Template C: Refactoring Ship

1. Verify all callers updated to match new signatures
2. Run Phase 1 - External tool checks (especially Griffe for API drift)
3. Run Phase 2 - cc-skills checks (especially dead-code-detector)
4. Verify examples/docs updated to match new parameter names
5. Verify no dead imports from removed features
6. Check for introduced cross-boundary coupling
7. Compile findings report

---

Three-Phase Workflow

Phase 1: External Tool Checks (~15s, parallelizable)

Run static analysis tools on changed files. Skip any tool that is not installed (graceful degradation).

Detect scope:
  git diff --name-only $(git merge-base HEAD main)...HEAD

Run in parallel:
  pyright --outputjson <changed_py_files>          # Type contracts
  vulture <changed_py_files> --min-confidence 80   # Dead code / YAGNI
  lint-imports                                      # Architecture boundaries
  deptry .                                          # Dependency hygiene
  semgrep --config .semgrep/ <changed_files>        # Custom pattern rules
  griffe check --against main <package>             # API signature drift

What each tool catches:

Graceful degradation: If a tool is not installed, log a warning and skip it. Never fail the entire review because one optional tool is missing.

For detailed tool procedures, see Automated Checks Reference. For installation instructions, see Tool Install Guide.

Phase 2: cc-skills Orchestration (~30s, subagent-parallelizable)

Invoke existing cc-skills that complement external tools.

Always run:

• code-hardcode-audit -- Hardcoded values, magic numbers, leaked secrets
• dead-code-detector -- Polyglot dead code detection (Python, TypeScript, Rust)
• pr-gfm-validator -- PR description link validity (if creating a PR)

Run conditionally based on changed file types:

Phase 3: Human Judgment Review (Claude-assisted)

These checks require understanding intent, domain correctness, and architectural fitness. Go through each one manually.

Check 1: Architecture Boundaries

• Does new code in a "core" layer reference names from a "plugin" or "capability" layer?
• Are there hardcoded lists of feature/plugin names? (Boundary violation)
• Would adding another instance of this feature type require modifying core code?

Check 2: Domain Correctness

• Are mathematical formulas correct? Cross-reference with cited papers.
• Are constants labeled correctly? (e.g., a "daily" constant should use the daily value)
• Do units and time periods match? (annual vs daily rates, quarterly vs monthly lambdas)

Check 3: Test Quality

• Does each test exercise the specific function it claims to test?
• Or does it test a side-effect? (Function A tests function B which internally calls A)
• Are edge cases covered? (Empty input, NaN, single element, division by zero)

Check 4: Dependency Transparency

• If component A requires component B to run first, is this documented?
• Are ordering requirements explicit in interfaces, not just in examples?

Check 5: Performance

• Any nested loops over the same data? (Potential O(n^2))
• Any expanding-window operations that could be rolling or full-sample?
• Any per-element operations that could be vectorized?

Check 6: Error Message Quality

• Do errors tell users what to DO, not just what went wrong?
• Do validation errors reference the specific parameter/value that failed?

Check 7: Example Accuracy

• Do examples demonstrate features that actually work in the code?
• Are there parameters in examples that get silently absorbed by **kwargs or **_?

For detailed check procedures, see Judgment Checks Reference.

---

Universal Pre-Ship Checklist

Phase 1 (Tools):
- [ ] Pyright strict passes on changed files (no type errors)
- [ ] Vulture finds no unused code in new files (or allowlisted)
- [ ] import-linter passes (no architecture boundary violations)
- [ ] deptry passes (no unused/missing dependencies)
- [ ] Semgrep custom rules pass (no non-determinism, no silent param absorption)
- [ ] Griffe shows no unintended API breaking changes vs base branch

Phase 2 (cc-skills):
- [ ] code-hardcode-audit passes (no magic numbers or secrets)
- [ ] dead-code-detector passes (no unused code)
- [ ] PR description links valid (pr-gfm-validator)

Phase 3 (Judgment):
- [ ] No new cross-boundary coupling introduced
- [ ] Domain constants and formulas are mathematically correct
- [ ] Tests actually test what they claim (not side effects)
- [ ] Implicit dependencies between components are documented
- [ ] No O(n^2) where O(n) suffices
- [ ] Error messages give actionable guidance
- [ ] Examples reflect actual behavior, not aspirational behavior

---

Anti-Pattern Catalog

This skill is built on a taxonomy of 9 integration boundary anti-patterns. For the full catalog with examples, detection heuristics, and fix approaches, see Anti-Pattern Catalog.

---

Post-Change Checklist

After modifying THIS skill:

• [ ] Anti-pattern catalog reflects real-world findings
• [ ] Tool install guide has current versions and commands
• [ ] TodoWrite templates cover the three ship types
• [ ] Universal checklist is complete and non-redundant
• [ ] All references/ links resolve correctly
• [ ] Append changes to references/evolution-log.md

Troubleshooting

---

Reference Documentation

For detailed information, see:

• Automated Checks Reference -- Phase 1 external tool procedures
• Judgment Checks Reference -- Phase 3 human-judgment procedures
• Anti-Pattern Catalog -- Full 9-category taxonomy with examples
• Tool Install Guide -- Installation and setup for all external tools
• Evolution Log -- Change history

Post-Execution Reflection

After this skill completes, reflect before closing the task:

0. Locate yourself. — Find this SKILL.md's canonical path (Glob for this skill's name) before editing. All corrections target THIS file and its sibling references/ — never other documentation. 1. What failed? — Fix the instruction that caused it. If it could recur, add it as an anti-pattern. 2. What worked better than expected? — Promote it to recommended practice. Document why. 3. What drifted? — Any script, reference, or external dependency that no longer matches reality gets fixed now. 4. Log it. — Every change gets an evolution-log entry with trigger, fix, and evidence.

Do NOT defer. The next invocation inherits whatever you leave behind.

---

---

Skill: Pre-Ship Review

Anti-Pattern Catalog

Taxonomy of 9 integration boundary anti-patterns. Each entry includes the pattern, detection heuristic, fix approach, and a generalized example.

Origin: Derived from analysis of 15 real review issues, then generalized to be project-agnostic.

---

1. Interface Contract Violation

Pattern: Parameter names differ between caller and callee. The function expects column_x but the caller passes column1. With **kwargs or **_, this fails silently.

Detection:

• Pyright strict mode (catches type-level mismatches)
• Griffe (catches signature drift between branches)
• Manual: trace every parameter from caller to callee

Fix: Rename parameters to match. If renaming, update ALL callers and examples simultaneously.

Example:

# Caller (config/DSL)
params: { column_x: "price.close", column_y: "price.open" }

# Callee (WRONG - different names)
def my_function(*, column1: str, column2: str, **_): ...

# Callee (CORRECT - matching names)
def my_function(*, column_x: str, column_y: str, **_): ...

---

2. Misleading Examples

Pattern: Examples show parameters that the code silently ignores. Users expect the parameter to have an effect, but it's absorbed by **kwargs/**_.

Detection:

• Semgrep custom rule for **_ catch-all functions
• Manual: for each example parameter, verify it appears in the function signature as a named kwarg

Fix: Remove parameters from examples that code doesn't use, OR implement them.

Example:

# Example shows window parameter
- using: var_historical
  params:
    window: 252 # <- Code ignores this, uses all data
    confidence: 0.95 # <- This one actually works

---

3. Architecture Boundary Violation

Pattern: Core/framework code contains hardcoded references to specific plugin/feature names, breaking the separation between generic infrastructure and specific implementations.

Detection:

• import-linter (catches forbidden imports)
• Manual: search for hardcoded sets/lists of feature names in core code

Fix: Use registry lookups, metadata tags, or convention-based discovery instead of hardcoded names.

Example:

# WRONG - core code knows about specific plugins
RISK_PLUGINS = {"var_historical", "sortino_ratio", "beta"}
if plugin_name in RISK_PLUGINS:
    dependencies = get_risk_deps()

# CORRECT - core code uses generic logic
if has_model_nodes(dag):
    dependencies = get_model_deps()
else:
    dependencies = get_risk_deps()

---

4. Incorrect Domain Constants

Pattern: Constants have wrong values or are mislabeled. A constant named LAMBDA_DAILY contains the quarterly value. A frequency-scaled constant uses the wrong scaling rule.

Detection:

• Semgrep (flag constants with specific naming patterns for manual review)
• Manual: cross-reference values with cited academic sources

Fix: Correct the value and add an inline comment citing the source and showing the derivation.

Example:

# WRONG - labeled "daily" but contains quarterly value
HP_LAMBDA_DAILY = 1600  # This is the quarterly value!

# CORRECT - properly scaled with citation
HP_LAMBDA_DAILY = 129_600  # Ravn-Uhlig (2002): 1600 * 3^4 (monthly scaling)

---

5. Testing Gaps

Pattern: Tests test a side-effect of the target function rather than the function itself. If the target function breaks, the test may still pass because it's actually validating a different code path.

Detection:

• mutmut (mutation testing reveals weak assertions)
• Manual: trace each test's call chain to verify it reaches the target

Fix: Call the target function directly and assert on its specific output.

Example:

# WRONG - tests alpha by running beta (alpha is a side-effect)
def test_alpha_plugin():
    result = beta_plugin(panel)  # beta internally computes alpha too
    assert "eval.alpha" in result.columns  # Passes, but doesn't test standalone alpha

# CORRECT - tests alpha directly
def test_alpha_plugin():
    result = alpha_plugin(panel, beta_col="eval.beta")
    assert "eval.alpha_jensen" in result.columns

---

6. Non-Determinism

Pattern: Random operations without explicit seeds make results non-reproducible across runs.

Detection:

• Semgrep custom rule for random.*(), np.random.*(), torch.rand*()
• Manual: search for random operations and verify seed parameter exists

Fix: Add a seed parameter. Use np.random.default_rng(seed) instead of module-level random state.

Example:

# WRONG - non-reproducible
particles = np.random.normal(0, 1, size=(n_particles,))

# CORRECT - reproducible when seed is provided
rng = np.random.default_rng(seed)
particles = rng.normal(0, 1, size=(n_particles,))

---

7. YAGNI (You Aren't Gonna Need It)

Pattern: Constants, interfaces, or infrastructure defined for features that don't exist yet. This adds maintenance burden and confusion without current value.

Detection:

• Vulture (finds unused constants and functions)
• dead-code-detector (polyglot dead code detection)
• Manual: for each new constant/interface, verify at least one concrete usage exists

Fix: Remove unused definitions. Add them when the feature is actually implemented.

Example:

# WRONG - constants for features that don't exist
UKF_ALPHA = 1e-3        # No Unscented Kalman Filter plugin exists
UKF_BETA = 2.0          # No Unscented Kalman Filter plugin exists
FF3_SMB_COL = "smb"     # No Fama-French 3-Factor plugin exists

# CORRECT - only define what's used
BETA_MIN_OBSERVATIONS = 30  # Used by eval.beta plugin

---

8. Hidden Dependencies

Pattern: Component A requires component B to run first, but this is only discoverable at runtime when A crashes with a confusing error.

Detection:

• Manual: trace required inputs for each component and verify their origin
• Check error messages for missing dependencies

Fix: Document dependencies explicitly. Add actionable error messages when dependencies are missing.

Example:

# WRONG - crashes with KeyError when market.return doesn't exist
alpha = panel[return_col] - (risk_free + panel["market.return"] * excess)

# CORRECT - actionable error message
if market_col not in panel.columns:
    raise ValueError(
        f"Column '{market_col}' not found. "
        "Run eval.beta first (creates market.return), "
        "or provide an explicit market_col parameter."
    )

---

9. Performance Anti-Patterns

Pattern: O(n^2) or worse complexity when O(n) is achievable. Common in expanding-window operations and per-element matrix solves.

Detection:

• Manual: look for nested loops, expanding windows, per-element lstsq/solve calls

Fix: Replace with single full-sample computation, rolling windows, or vectorized operations.

Example:

# WRONG - O(n^2) expanding window OLS
for i in range(min_obs, len(y)):
    X_window = X[:i+1]
    y_window = y[:i+1]
    beta, _, _, _ = np.linalg.lstsq(X_window, y_window)
    result[i] = X[i] @ beta

# CORRECT - O(n) single full-sample OLS
beta, _, _, _ = np.linalg.lstsq(X, y)
result = X @ beta

Skill: Pre-Ship Review

Automated Checks Reference

Detailed procedures for Phase 1 external tool checks. Each check runs independently and can be parallelized.

---

Scope Detection

Before running any check, determine the changed files:

/usr/bin/env bash << 'SCOPE_EOF'
# Detect base branch (main or master)
BASE=$(git rev-parse --verify main 2>/dev/null && echo main || echo master)

# Get changed files relative to base
CHANGED=$(git diff --name-only "$(git merge-base HEAD "$BASE")...HEAD")

# Filter by language
PY_FILES=$(echo "$CHANGED" | grep '\.py$' || true)
YAML_FILES=$(echo "$CHANGED" | grep -E '\.(ya?ml)$' || true)
MD_FILES=$(echo "$CHANGED" | grep '\.md$' || true)

echo "Changed Python files: $(echo "$PY_FILES" | wc -l | tr -d ' ')"
echo "Changed YAML files: $(echo "$YAML_FILES" | wc -l | tr -d ' ')"
echo "Changed Markdown files: $(echo "$MD_FILES" | wc -l | tr -d ' ')"
SCOPE_EOF

---

Check 1: Pyright Strict Mode

Anti-patterns caught: Interface contract violations (#1), return type mismatches (#1)

What to run:

/usr/bin/env bash << 'PYRIGHT_EOF'
if ! command -v pyright &>/dev/null; then
  echo "SKIP: pyright not installed (pip install pyright)"
  exit 0
fi

# Run on changed Python files only
pyright --outputjson --pythonversion 3.14 $PY_FILES 2>/dev/null | \
  python3 -c "
import json, sys
data = json.load(sys.stdin)
diags = data.get('generalDiagnostics', [])
errors = [d for d in diags if d['severity'] == 'error']
print(f'Pyright: {len(errors)} errors, {len(diags) - len(errors)} warnings')
for e in errors:
    print(f\"  {e['file']}:{e['range']['start']['line']}: {e['message']}\")
"
PYRIGHT_EOF

Key diagnostics to watch for:

---

Check 2: Vulture (Dead Code / YAGNI)

Anti-patterns caught: YAGNI (#7), unused constants/imports

What to run:

/usr/bin/env bash << 'VULTURE_EOF'
if ! command -v vulture &>/dev/null; then
  echo "SKIP: vulture not installed (pip install vulture)"
  exit 0
fi

# Run on changed files with confidence threshold
vulture $PY_FILES --min-confidence 80

# To generate an allowlist for framework entry points:
# vulture . --make-whitelist > whitelist.py
# vulture $PY_FILES whitelist.py --min-confidence 80
VULTURE_EOF

Common false positives and how to handle:

---

Check 3: import-linter (Architecture Boundaries)

Anti-patterns caught: Architecture boundary violations (#3)

What to run:

/usr/bin/env bash << 'IMPORTLINT_EOF'
if ! command -v lint-imports &>/dev/null; then
  echo "SKIP: import-linter not installed (pip install import-linter)"
  exit 0
fi

lint-imports
IMPORTLINT_EOF

Configuration (in pyproject.toml):

[importlinter]
root_packages = your_core_package, your_capability_package

[importlinter:contract:core-independence]
name = Core does not import capabilities
type = forbidden
source_modules = your_core_package
forbidden_modules = your_capability_package

[importlinter:contract:no-circular]
name = No circular imports between packages
type = independence
modules =
    your_core_package
    your_capability_package

---

Check 4: deptry (Dependency Hygiene)

Anti-patterns caught: Unused dependencies, missing dependencies, transitive dependency usage

What to run:

/usr/bin/env bash << 'DEPTRY_EOF'
if ! command -v deptry &>/dev/null; then
  echo "SKIP: deptry not installed (pip install deptry)"
  exit 0
fi

# Run per package directory (if monorepo)
for pkg_dir in packages/*/; do
  if [ -f "$pkg_dir/pyproject.toml" ]; then
    echo "Checking $pkg_dir..."
    cd "$pkg_dir" && deptry . && cd - > /dev/null
  fi
done
DEPTRY_EOF

---

Check 5: Semgrep (Custom Pattern Rules)

Anti-patterns caught: Non-determinism (#6), misleading examples (#2), silent param absorption

Semgrep lets you define project-specific rules. Create a .semgrep/ directory with YAML rules.

Example rules:

# .semgrep/non-determinism.yaml
rules:
  - id: random-without-seed
    patterns:
      - pattern-either:
          - pattern: np.random.normal(...)
          - pattern: np.random.uniform(...)
          - pattern: np.random.randn(...)
          - pattern: random.random()
          - pattern: random.choice(...)
    message: "Random operation without explicit seed. Use np.random.default_rng(seed) for reproducibility."
    severity: WARNING
    languages: [python]

  - id: torch-random-without-seed
    pattern: torch.rand(...)
    message: "PyTorch random without manual_seed. Use torch.manual_seed(seed) for reproducibility."
    severity: WARNING
    languages: [python]

# .semgrep/kwargs-absorption.yaml
rules:
  - id: kwargs-star-underscore
    pattern: |
      def $FUNC(..., **_, ...):
          ...
    message: "Function uses **_ catch-all. Verify all callers pass correct parameter names -- misnamed params are silently absorbed."
    severity: INFO
    languages: [python]

What to run:

/usr/bin/env bash << 'SEMGREP_EOF'
if ! command -v semgrep &>/dev/null; then
  echo "SKIP: semgrep not installed (brew install semgrep)"
  exit 0
fi

# Run with project rules on changed files
if [ -d ".semgrep" ]; then
  semgrep --config .semgrep/ --include="*.py" $PY_FILES --json 2>/dev/null | \
    python3 -c "
import json, sys
data = json.load(sys.stdin)
results = data.get('results', [])
print(f'Semgrep: {len(results)} findings')
for r in results:
    print(f\"  {r['path']}:{r['start']['line']}: [{r['check_id']}] {r['extra']['message']}\")
"
else
  echo "SKIP: No .semgrep/ directory found (create project-specific rules)"
fi
SEMGREP_EOF

---

Check 6: Griffe (API Signature Drift)

Anti-patterns caught: Interface contract violations (#1), breaking changes

What to run:

/usr/bin/env bash << 'GRIFFE_EOF'
if ! python3 -c "import griffe" 2>/dev/null; then
  echo "SKIP: griffe not installed (pip install griffe)"
  exit 0
fi

BASE=$(git rev-parse --verify main 2>/dev/null && echo main || echo master)

# Check for breaking API changes vs base branch
griffe check --against "$BASE" your_package_name 2>&1 || true
GRIFFE_EOF

What Griffe detects:

---

Interpreting Results

Severity classification:

When to override a finding:

• Vulture reports plugin entry points as unused -> Add to allowlist
• Griffe reports intentional breaking change -> Document in changelog
• Semgrep **_ warning on a framework convention -> Add # nosemgrep comment
• import-linter violation for legitimate cross-cutting concern -> Update contract

Skill: Pre-Ship Review

Evolution Log

Reverse chronological record of changes to the pre-ship-review skill.

---

2026-02-09: Initial creation

• Created pre-ship-review skill based on analysis of 15 real review issues from alpha-forge PR #135
• 9 anti-pattern categories identified, 8 universal (project-agnostic)
• Three-phase structure: external tools (Pyright, Vulture, import-linter, deptry, Semgrep, Griffe) -> cc-skills orchestration (code-hardcode-audit, dead-code-detector, pr-gfm-validator) -> human judgment (7 checks)
• TodoWrite templates for 3 ship types: new feature, bug fix, refactoring
• Tool install guide with graceful degradation when tools are missing
• Anti-pattern catalog with detection heuristics and fix approaches

Skill: Pre-Ship Review

Judgment Checks Reference

Detailed procedures for Phase 3 human-judgment checks. These require understanding intent, domain correctness, and architectural fitness -- they cannot be fully automated.

---

Check 1: Architecture Boundaries

Anti-pattern: Core code references capability-specific names, creating coupling that violates architectural separation.

How to check:

1. Search for hardcoded lists of feature/plugin/module names in core code:

   Look for: sets, lists, or dicts containing specific plugin/feature names
   Example violation: RISK_PLUGINS = {"var_historical", "sortino_ratio", ...}

2. Apply the "add another" test:

• Would adding a new feature/plugin of this type require modifying core code?
• If yes, the boundary is violated.

3. Check for imports flowing in the wrong direction:

• Core should never import from capabilities/plugins
• Capabilities may import from core (dependency flows inward)

Fix approach: Replace hardcoded names with:

• Registry lookups
• Configuration/metadata on the plugins themselves
• Convention-based discovery (tags, naming patterns)

---

Check 2: Domain Correctness

Anti-pattern: Constants, formulas, or domain-specific values are mathematically incorrect or mislabeled.

How to check:

1. For each new constant or formula, verify against the cited source:

• Is the paper/textbook cited? If not, find the authoritative source.
• Does the implementation match the formula exactly?
• Are the units correct? (Annual vs daily, percent vs decimal)

2. Check label-value alignment:

• A constant named LAMBDA_DAILY should contain the daily-frequency value
• A constant named RISK_FREE_RATE should be in the expected units (daily/annual)

3. Common domain traps:

• Annualization factors: sqrt(252) for daily returns, sqrt(12) for monthly
• Frequency scaling: Power-law rules like Ravn-Uhlig (lambda \* ratio^4)
• Confidence levels: 95% VaR vs 97.5% CVaR (Basel III)

Fix approach: Add inline comments citing the source and showing the calculation.

---

Check 3: Test Quality

Anti-pattern: Tests test side-effects of the target function rather than the function itself.

How to check:

1. For each test, verify the call chain:

• Does test_alpha_plugin actually call the alpha function directly?
• Or does it call beta which internally computes alpha as a side-effect?

2. Apply the "break it" test:

• If you introduce a bug in the target function, does THIS test fail?
• If the bug only fails a different test, the test is indirect.

3. Check assertion quality:

• Are assertions specific? (assert result["col"] == expected_value)
• Or vague? (assert len(result) > 0, assert result is not None)

4. Check edge case coverage:

• Empty input / no data
• NaN values / missing data
• Single element / minimum viable input
• Boundary values (exact thresholds)

Fix approach: Each test should:

• Call the target function directly
• Assert on the specific output of that function
• Cover at least one happy path and one edge case

---

Check 4: Dependency Transparency

Anti-pattern: Component A requires component B to run first, but this dependency is undocumented and only discoverable at runtime.

How to check:

1. For each new component, trace its required inputs:

• What columns/data does it expect to exist?
• Where are those columns created?
• Is the creator always run before this component?

2. Check for "magic columns" -- columns that appear without explicit creation:

• A function expects market.return column but doesn't document where it comes from
• A plugin reads eval.beta column that only exists after another plugin runs

3. Verify that error messages for missing dependencies are actionable:

• BAD: KeyError: 'market.return'
• GOOD: "market.return column not found. Run eval.beta first or provide market_col parameter."

Fix approach:

• Document dependencies in function docstrings and decorator metadata
• Add actionable error messages when dependencies are missing
• Consider auto-creating missing dependencies when possible

---

Check 5: Performance

Anti-pattern: O(n^2) or worse algorithmic complexity when O(n) is possible.

How to check:

1. Look for nested loops over the same data:

   # O(n^2) - expanding window pattern
   for i in range(len(data)):
       window = data[:i+1]  # Growing window
       result[i] = some_computation(window)

2. Look for per-element operations on arrays:

   # O(n) per element = O(n^2) total
   for i in range(len(data)):
       result[i] = np.linalg.lstsq(X[:i+1], y[:i+1])  # Full solve each step

3. Check for opportunities to vectorize:

• Loop over rows -> pandas/numpy vectorized operation
• Repeated computation -> cache/memoize
• Expanding window -> rolling window or single full-sample computation

Fix approach: Replace with:

• Single full-sample computation (when mathematically equivalent)
• Rolling window operations (pandas.rolling, numpy.convolve)
• Vectorized operations (numpy broadcasting, pandas.apply)

---

Check 6: Error Message Quality

Anti-pattern: Error messages describe what went wrong but not what to do about it.

How to check:

For each raise statement or error log in new code, ask:

1. Does the message tell the user what action to take?

• BAD: "Column 'x' not found"
• GOOD: "Column 'x' not found in panel. Ensure data plugin outputs this column, or specify an alternative via the column_x parameter."

2. Does it reference the specific value that failed?

• BAD: "Invalid parameter"
• GOOD: "Parameter 'window' must be positive, got -5"

3. Does it provide context for debugging?

• BAD: "Insufficient data"
• GOOD: "Symbol BTCUSDT has 15 observations, minimum required is 30 for beta calculation"

Fix approach: Every error message should answer: "What should I do now?"

---

Check 7: Example Accuracy

Anti-pattern: Examples/documentation show parameters or behavior that the code silently ignores.

How to check:

1. For each example file (YAML, JSON, config, README code blocks):

• List every parameter name used
• Verify each parameter exists in the target function's signature
• Check it's NOT absorbed by **kwargs or **_ without effect

2. For each parameter shown in examples:

• Does changing the value actually change the output?
• If the function ignores it, the example is misleading

3. Common traps:

• window: 252 passed but function uses all available data
• periods_per_year: 365 passed but function hardcodes 252
• annualize: true passed but function doesn't have this parameter

Fix approach:

• Remove parameters from examples that the code doesn't use
• OR implement the parameter in the code
• Never leave aspirational parameters in examples

Skill: Pre-Ship Review

Tool Install Guide

Installation and setup for all external tools used by the pre-ship-review skill. All tools are optional -- the skill degrades gracefully if a tool is not installed.

---

Quick Install (All Tools)

/usr/bin/env bash << 'INSTALL_EOF'
# Tier 1: Always Run
pip install pyright vulture import-linter deptry

# Tier 2: Pattern Checks
brew install semgrep
pip install griffe

# Tier 3: Deep Checks (optional)
pip install mutmut
INSTALL_EOF

---

Tool Details

Pyright (Type Checker)

Configuration (pyproject.toml):

# SSoT-OK: pythonVersion should match your project's pyproject.toml python-requires
[tool.pyright]
pythonVersion = "<your-python-version>"
typeCheckingMode = "strict"
reportReturnType = true
reportArgumentType = true
reportCallIssue = true

Usage: pyright --outputjson <files>

---

Vulture (Dead Code Detector)

Usage:

# Basic scan
vulture <files> --min-confidence 80

# Generate allowlist for framework entry points
vulture . --make-whitelist > whitelist.py

# Scan with allowlist
vulture <files> whitelist.py --min-confidence 80

---

import-linter (Architecture Enforcer)

Configuration (pyproject.toml):

[importlinter]
root_packages = your_core, your_plugins

[importlinter:contract:core-independence]
name = Core does not import plugins
type = forbidden
source_modules = your_core
forbidden_modules = your_plugins

[importlinter:contract:layer-order]
name = Layers are independent
type = independence
modules = your_core, your_plugins

Usage: lint-imports

---

deptry (Dependency Linter)

Usage: deptry . (run from package root)

---

Semgrep (Custom Pattern Rules)

Setup:

1. Create .semgrep/ directory in project root 2. Add YAML rule files (see Automated Checks Reference for examples) 3. Run: semgrep --config .semgrep/ <files>

Example rule (.semgrep/non-determinism.yaml):

rules:
  - id: random-without-seed
    patterns:
      - pattern-either:
          - pattern: np.random.normal(...)
          - pattern: np.random.uniform(...)
          - pattern: random.random()
    message: "Random operation without explicit seed."
    severity: WARNING
    languages: [python]

---

Griffe (API Change Detector)

Usage:

# Compare current branch against main
griffe check --against main your_package

# Dump current API as JSON
griffe dump your_package --output json

---

mutmut (Mutation Testing) -- Optional

Usage:

# Run mutation testing on specific files
mutmut run --paths-to-mutate=<changed_files>

# View results
mutmut results

# Inspect a surviving mutant
mutmut show 42

Note: Mutation testing is slow (minutes, not seconds). Use as an optional deep check, not a blocking gate.

---

Checking Installation Status

/usr/bin/env bash << 'CHECK_EOF'
echo "=== Pre-Ship Review Tool Status ==="
for tool in pyright vulture lint-imports deptry semgrep griffe mutmut; do
  if command -v "$tool" &>/dev/null; then
    echo "  [OK] $tool"
  else
    echo "  [--] $tool (not installed)"
  fi
done
CHECK_EOF