Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 17 additions & 16 deletions .claude/skills/ci-prep/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,20 @@ name: ci-prep
description: Prepares the current branch for CI by running the exact same steps locally and fixing issues. If CI is already failing, fetches the GH Actions logs first to diagnose. Use before pushing, when CI is red, or when the user says "fix ci".
argument-hint: "[--failing] [optional job name to focus on]"
---
<!-- agent-pmo:d58c330 -->
<!-- agent-pmo:29b9dcf -->

# CI Prep

Prepare the current state for CI. If CI is already failing, fetch and analyze the logs first.

## Arguments

- `--failing` -- Indicates a GitHub Actions run is already failing. When present, you MUST execute **Step 1** before doing anything else.
- `--failing` Indicates a GitHub Actions run is already failing. When present, you MUST execute **Step 1** before doing anything else.
- Any other argument is treated as a job name to focus on (but all failures are still reported).

If `--failing` is NOT passed, skip directly to **Step 2**.

## Step 1 -- Fetch failed CI logs (only when `--failing`)
## Step 1 Fetch failed CI logs (only when `--failing`)

You MUST do this before any other work.

Expand All @@ -40,23 +40,23 @@ gh run view "$RUN_ID" --log-failed

Read **every line** of `--log-failed` output. For each failure note the exact file, line, and error message. If a job name argument was provided, prioritize that job but still report all failures.

## Step 2 -- Analyze the CI workflow
## Step 2 Analyze the CI workflow

1. Find the CI workflow file. Look in `.github/workflows/` for `ci.yml`, `build.yml`, `test.yml`, `checks.yml`, `main.yml`, `pull_request.yml`, or any workflow triggered on `pull_request` or `push`.
2. Read the workflow file completely. Parse every job and every step.
3. Extract the ordered list of commands the CI actually runs (e.g., `make lint`, `make fmt-check`, `make test`, `make coverage-check`, `make build`, or whatever the workflow specifies -- it may use `npm`, `cargo`, `dotnet`, raw shell commands, or anything else).
3. Extract the ordered list of commands the CI actually runs (e.g., `make lint`, `make fmt-check`, `make test`, `make coverage-check`, `make build`, or whatever the workflow specifies it may use `npm`, `cargo`, `dotnet`, raw shell commands, or anything else).
4. Note any environment variables, matrix strategies, or conditional steps that affect execution.

**Do NOT assume the steps are `make lint`, `make test`, `make coverage-check`, `make build`.** The actual CI may run different commands, in a different order, with different targets. Extract what the CI *actually does*.

## Step 3 -- Run each CI step locally, in order
## Step 3 Run each CI step locally, in order

Work through failures in this priority order:

1. **Formatting** -- run auto-formatters first to clear noise
2. **Compilation errors** -- must compile before lint/test
3. **Lint violations** -- fix the code pattern
4. **Runtime / test failures** -- fix source code to satisfy the test
1. **Formatting** run auto-formatters first to clear noise
2. **Compilation errors** must compile before lint/test
3. **Lint violations** fix the code pattern
4. **Runtime / test failures** fix source code to satisfy the test

For each command extracted from the CI workflow:

Expand All @@ -67,20 +67,21 @@ For each command extracted from the CI workflow:

### Hard constraints

- **NEVER modify test files** -- fix the source code, not the tests
- **NEVER add suppressions** (`#pragma warning disable`, `// eslint-disable`, `#[allow(...)]`)
- **NEVER modify test files** — fix the source code, not the tests
- **NEVER add suppressions** (`#[allow(...)]`, `// eslint-disable`, `#pragma warning disable`)
- **NEVER use `any` in TypeScript** to silence type errors
- **NEVER delete or ignore failing tests**
- **NEVER remove assertions**

If stuck on the same failure after 5 attempts, ask the user for help.

## Step 4 -- Report
## Step 4 Report

- List every step that was run and its result (pass/fail/fixed).
- If any step could not be fixed, report what failed and why.
- Confirm whether the branch is ready to push.

## Step 5 -- Commit/Push (only when `--failing`)
## Step 5 Commit/Push (only when `--failing`)

Once all CI steps pass locally:

Expand All @@ -96,10 +97,10 @@ Once all CI steps pass locally:
- Fix issues found in each step before moving to the next
- Never skip steps or suppress errors
- If the CI workflow has multiple jobs, run all of them (respecting dependency order)
- Skip steps that are CI-infrastructure-only (checkout, setup-node/python/rust actions, cache steps, artifact uploads) -- focus on the actual build/test/lint commands
- Skip steps that are CI-infrastructure-only (checkout, setup-node/python/rust actions, cache steps, artifact uploads) focus on the actual build/test/lint commands

## Success criteria

- Every command that CI runs has been executed locally and passed
- All fixes are applied to the working tree
- The CI passes successfully (if you are correcting an existing failure)
- The CI passes successfully (if you are correcting and existing failure)
84 changes: 51 additions & 33 deletions .claude/skills/code-dedup/SKILL.md
Original file line number Diff line number Diff line change
@@ -1,20 +1,23 @@
---
name: code-dedup
description: Searches for duplicate code, duplicate tests, and dead code, then safely merges or removes them. Use when the user says "deduplicate", "find duplicates", "remove dead code", "DRY up", or "code dedup". Requires test coverage -- refuses to touch untested code.
description: Searches for duplicate code, duplicate tests, and dead code, then safely merges or removes them. Use when the user says "deduplicate", "find duplicates", "remove dead code", "DRY up", or "code dedup". Requires test coverage refuses to touch untested code.
---
<!-- agent-pmo:d58c330 -->
<!-- agent-pmo:29b9dcf -->

# Code Dedup

Carefully search for duplicate code, duplicate tests, and dead code across the repo. Merge duplicates and delete dead code -- but only when test coverage proves the change is safe.
Carefully search for duplicate code, duplicate tests, and dead code across the repo. Merge duplicates and delete dead code but only when test coverage proves the change is safe.

## Prerequisites -- hard gate
## Prerequisites hard gate

Before touching ANY code, verify these conditions. If any fail, stop and report why.

1. Run `make test` -- all tests must pass. If tests fail, stop. Do not dedup a broken codebase.
2. Run `make coverage-check` -- coverage must meet the repo's threshold. If it doesn't, stop.
3. This is a C# repo with static typing -- proceed.
1. Run `make test` — all tests must pass. If tests fail, stop. Do not dedup a broken codebase.
2. Run `make coverage-check` — coverage must meet the repo's threshold. If it doesn't, stop.
3. Verify the project uses **static typing**. Check for:
- C#: typed by default — proceed
- Python: must have type annotations AND a type checker configured (pyright, mypy, or Basilisk in pyproject.toml / Makefile) — proceed
- **Untyped Python: STOP. Refuse to dedup.** Print: "This codebase has no static type checking. Deduplication without types is reckless — too high a risk of silent breakage. Add type checking first."

## Steps

Expand All @@ -30,65 +33,80 @@ Dedup Progress:
- [ ] Step 6: Verification passed (tests green, coverage stable)
```

### Step 1 -- Inventory test coverage
### Step 1 — Inventory test coverage

Before deciding what to touch, understand what is tested.

1. Run `make test` and `make coverage-check` to confirm green baseline
2. Note the current coverage percentage -- this is the floor. It must not drop.
2. Note the current coverage percentage this is the floor. It must not drop.
3. Identify which files/modules have coverage and which do not. Only files WITH coverage are candidates for dedup.

### Step 2 -- Scan for dead code
### Step 2 — Scan for dead code

Search for code that is never called, never imported, never referenced.

1. Look for unused exports, unused functions, unused classes, unused variables
2. C# analyzer warnings for unused members -- check `make lint` output
3. For each candidate: **grep the entire codebase** for references. Only mark as dead if truly zero references.
2. Use language-appropriate tools where available:
- C#: analyzer warnings for unused members
- Python: look for functions/classes with zero imports across the codebase
3. For each candidate: **grep the entire codebase** for references (including tests, scripts, configs). Only mark as dead if truly zero references.
4. List all dead code found with file paths and line numbers. Do NOT delete yet.

### Step 3 -- Scan for duplicate code
### Step 3 — Scan for duplicate code

Search for code blocks that do the same thing in multiple places.

1. Look for functions/methods with identical or near-identical logic
2. Look for copy-pasted blocks (same structure, maybe different variable names)
3. Look for multiple implementations of the same algorithm or pattern
4. Check across module boundaries -- duplicates often hide in different projects
5. For each duplicate pair: note both locations, what they do, and how they differ
4. Check across module boundaries duplicates often hide in different packages/projects
5. For each duplicate pair: note both locations, what they do, and how they differ (if at all)
6. List all duplicates found. Do NOT merge yet.

### Step 4 -- Scan for duplicate tests
### Step 4 — Scan for duplicate tests

Search for tests that verify the same behavior.

1. Look for test functions with identical assertions against the same code paths
2. Look for test fixtures/helpers that are duplicated across test files
3. Look for integration tests that fully cover what a unit test also covers (keep the integration test)
3. Look for integration tests that fully cover what a unit test also covers (keep the integration test, mark the unit test as redundant per CLAUDE.md rules)
4. List all duplicate tests found. Do NOT delete yet.

### Step 5 -- Apply changes (one at a time)
### Step 5 Apply changes (one at a time)

For each change: **change -> test -> verify coverage -> continue or revert**.
For each change, follow this cycle: **change test verify coverage continue or revert**.

#### 5a. Remove dead code
- Delete dead code identified in Step 2
- After each deletion: run `make test` and `make coverage-check`
- If tests fail or coverage drops: **revert immediately**
- If tests fail or coverage drops: **revert immediately** and investigate
- Dead code removal should never break tests or drop coverage

#### 5b. Merge duplicate code
- Extract shared logic into a single function/module, update all call sites
- For each duplicate pair: extract the shared logic into a single function/module
- Update all call sites to use the shared version
- After each merge: run `make test` and `make coverage-check`
- If tests fail: **revert immediately**
- If tests fail: **revert immediately**. The duplicates may have subtle differences you missed.
- If coverage drops: the shared code must have equivalent test coverage. Add tests if needed before proceeding.

#### 5c. Remove duplicate tests
- Delete the redundant test (keep the more thorough one)
- After each deletion: run `make coverage-check`
- If coverage drops: **revert immediately**
- If coverage drops: **revert immediately**. The "duplicate" test was covering something the other wasn't.

### Step 6 -- Final verification
### Step 6 Final verification

1. Run `make test` -- all tests must still pass
2. Run `make coverage-check` -- coverage must be >= the baseline from Step 1
3. Run `make lint` and `make fmt-check` -- code must be clean
1. Run `make test` all tests must still pass
2. Run `make coverage-check` coverage must be >= the baseline from Step 1
3. Run `make lint` and `make fmt-check` code must be clean
4. Report: what was removed, what was merged, final coverage vs baseline

## Rules

- **No test coverage = do not touch.** If a file has no tests covering it, leave it alone entirely.
- **Coverage must not drop.** The coverage floor from Step 1 is sacred.
- **One change at a time.** Never batch multiple dedup changes before testing.
- **When in doubt, leave it.**
- **Preserve public API surface.**
- **Three similar lines is fine.** Only dedup when shared logic is substantial (>10 lines) or 3+ copies.
- **No test coverage = do not touch.** If a file has no tests covering it, leave it alone entirely. You cannot safely dedup what you cannot verify.
- **Coverage must not drop.** If removing or merging code causes coverage to decrease, revert and investigate. The coverage floor from Step 1 is sacred.
- **Untyped code = refuse to dedup.** Untyped Python is too dangerous. Types are the safety net that catches breakage at compile time. Without them, silent runtime errors are near-certain.
- **One change at a time.** Make one dedup change, run tests, verify coverage. Never batch multiple dedup changes before testing.
- **When in doubt, leave it.** If two code blocks look similar but you're not 100% sure they're functionally identical, leave both. False dedup is worse than duplication.
- **Preserve public API surface.** Do not change function signatures, class names, or module exports that external code depends on. Internal refactoring only.
- **Three similar lines is fine.** Do not create abstractions for trivial duplication. The cure must not be worse than the disease. Only dedup when the shared logic is substantial (>10 lines) or when there are 3+ copies.
Loading
Loading