Compare commits
11 Commits
673c5a234a
...
main
| Author | SHA1 | Date | |
|---|---|---|---|
| e6f23d59e0 | |||
| 8a4b6ae5ae | |||
| 1a6ed4ab10 | |||
| 9f6d9bf8c8 | |||
| d1c279ff57 | |||
| 5c2589dce6 | |||
| fb575abd16 | |||
| e57b8bcfb1 | |||
| 6353833ddc | |||
| b3bb425203 | |||
| 7c5b21b680 |
@@ -5,7 +5,6 @@
|
|||||||
"ENABLE_CLAUDEAI_MCP_SERVERS": "false",
|
"ENABLE_CLAUDEAI_MCP_SERVERS": "false",
|
||||||
"CLAUDE_CODE_DISABLE_ALTERNATE_SCREEN": "1"
|
"CLAUDE_CODE_DISABLE_ALTERNATE_SCREEN": "1"
|
||||||
},
|
},
|
||||||
"model": "claude-opus-4-6",
|
|
||||||
"feedbackSurveyRate": 0,
|
"feedbackSurveyRate": 0,
|
||||||
"awaySummaryEnabled": false,
|
"awaySummaryEnabled": false,
|
||||||
"showClearContextOnPlanAccept": true,
|
"showClearContextOnPlanAccept": true,
|
||||||
|
|||||||
@@ -1,11 +1,11 @@
|
|||||||
---
|
---
|
||||||
name: audit
|
name: audit
|
||||||
description: Run a deep, multi-lens review of existing code state (not a diff). Launches six specialized review agents in parallel - reuse, quality, efficiency, errors, api, bugs - then validates each finding before presenting. Optional scope (`/audit path/ path2/`) and optional lens subset (`/audit --lenses reuse,bugs`). Opt-in lenses for docs, tests, security, a11y, deps. Use when the user asks for a full review, deep review, codebase audit, cleanup pass, retrospective review, tech-debt sweep, or otherwise wants to surface issues across landed code - even if they say "clean up the project" or "look over the repo" without using the word "audit". Do NOT use for reviewing in-flight work (use /simplify), PR review (use /review or /code-review), or security-only review (use /security-review).
|
description: Run a deep, multi-lens review of existing code state (not a diff). Launches six specialized review agents in parallel (reuse, quality, efficiency, errors, api, bugs), then validates each finding before presenting. Optional scope (`/audit path/ path2/`) and optional lens subset (`/audit --lenses reuse,bugs`). Opt-in lenses for docs, tests, security, a11y, deps. Use when the user asks for a full review, deep review, codebase audit, cleanup pass, retrospective review, tech-debt sweep, or otherwise wants to surface issues across landed code, even if they say "clean up the project" or "look over the repo" without using the word "audit". Do NOT use for reviewing in-flight work (use /check), PR review (use /review or /code-review), or security-only review (use /security-review).
|
||||||
---
|
---
|
||||||
|
|
||||||
# /audit: Retrospective multi-lens codebase review
|
# /audit: Retrospective multi-lens codebase review
|
||||||
|
|
||||||
`/simplify` reviews a diff; `/audit` reviews current file state. Use it when issues may have accumulated before a review gate existed, when rolling onto an unfamiliar codebase, or when the user wants a deliberate "what's lurking?" sweep.
|
`/check` reviews a diff. `/audit` reviews current file state. Use it when issues may have accumulated before a review gate existed, when rolling onto an unfamiliar codebase, or when the user wants a deliberate "what's lurking?" sweep.
|
||||||
|
|
||||||
## Invocation
|
## Invocation
|
||||||
|
|
||||||
@@ -17,7 +17,7 @@ description: Run a deep, multi-lens review of existing code state (not a diff).
|
|||||||
/audit src/foo/ --lenses docs,tests # both scope and lenses
|
/audit src/foo/ --lenses docs,tests # both scope and lenses
|
||||||
```
|
```
|
||||||
|
|
||||||
If the user invokes `/audit` with no arguments, infer the primary source tree from the project layout (Rust: `crates/*/src/`; TS/JS: `src/` or `packages/*/src/`; Python: the package directory). Ask briefly only if ambiguous.
|
If the user invokes `/audit` with no arguments, infer the primary source tree from the project layout (`crates/*/src/` for Rust, `src/` or `packages/*/src/` for TS/JS, the package directory for Python). Ask briefly only if ambiguous.
|
||||||
|
|
||||||
## Communicating with the user
|
## Communicating with the user
|
||||||
|
|
||||||
@@ -36,29 +36,29 @@ All three pieces of context get fed into every agent prompt so they respect the
|
|||||||
|
|
||||||
## Phase 2: Launch lens agents in parallel
|
## Phase 2: Launch lens agents in parallel
|
||||||
|
|
||||||
Send a single message with multiple `Agent` tool uses, each `subagent_type: general-purpose`. The default set is six lenses; if `--lenses <list>` was given, run only those (plus any opt-in lenses named in the list).
|
Send a single message with multiple `Agent` tool uses, each `subagent_type: general-purpose`. The default set is six lenses. If `--lenses <list>` was given, run only those (plus any opt-in lenses named in the list).
|
||||||
|
|
||||||
### Model selection per lens
|
### Model selection per lens
|
||||||
|
|
||||||
The `Agent` tool accepts a `model: "sonnet" | "opus" | "haiku"` parameter. Pick deliberately - some lenses are pattern-matching (cheap), others are reasoning-heavy (expensive but worth it).
|
The `Agent` tool accepts a `model: "sonnet" | "opus" | "haiku"` parameter. Pick deliberately, since some lenses are pattern-matching (cheap) and others are reasoning-heavy (expensive but worth it).
|
||||||
|
|
||||||
| lens | model | why |
|
| lens | model | why |
|
||||||
| ----------------- | -------- | --------------------------------------------------------------------- |
|
| ----------------- | -------- | --------------------------------------------------------------------- |
|
||||||
| reuse | sonnet | pattern recognition across files, fits sonnet's strengths |
|
| reuse | sonnet | pattern recognition across files, fits sonnet's strengths |
|
||||||
| quality | sonnet | structural critique, naming, dead code; sonnet is enough |
|
| quality | sonnet | structural critique, naming, dead code, sonnet is enough |
|
||||||
| efficiency | **opus** | needs reasoning about hot paths, allocations, asymptotic patterns |
|
| efficiency | **opus** | needs reasoning about hot paths, allocations, asymptotic patterns |
|
||||||
| errors | **opus** | control-flow analysis, silent-failure detection wants careful reading |
|
| errors | **opus** | control-flow analysis, silent-failure detection wants careful reading |
|
||||||
| api | sonnet | visibility analysis, type design - mostly mechanical |
|
| api | sonnet | visibility analysis, type design, mostly mechanical |
|
||||||
| bugs | **opus** | correctness reasoning is the place not to skimp |
|
| bugs | **opus** | correctness reasoning is the place not to skimp |
|
||||||
| docs (opt-in) | haiku | "does the comment still match the code?" - cheap |
|
| docs (opt-in) | haiku | "does the comment still match the code?", cheap |
|
||||||
| tests (opt-in) | sonnet | gap analysis with semantic context |
|
| tests (opt-in) | sonnet | gap analysis with semantic context |
|
||||||
| security (opt-in) | **opus** | high-stakes correctness, needs careful reading |
|
| security (opt-in) | **opus** | high-stakes correctness, needs careful reading |
|
||||||
| a11y (opt-in) | sonnet | pattern matching with semantic context |
|
| a11y (opt-in) | sonnet | pattern matching with semantic context |
|
||||||
| deps (opt-in) | haiku | mostly file scanning |
|
| deps (opt-in) | haiku | mostly file scanning |
|
||||||
|
|
||||||
The validation agent in Phase 3 also runs on **opus** - false negatives drop real findings, so this is the wrong place to economize.
|
The validation agent in Phase 3 also runs on **opus**: false negatives drop real findings, so this is the wrong place to economize.
|
||||||
|
|
||||||
These are defaults; if a project's lens is unusually subtle (e.g. obscure embedded language, novel runtime), bump up.
|
These are defaults. If a project's lens is unusually subtle (e.g. obscure embedded language, novel runtime), bump up.
|
||||||
|
|
||||||
### Default lenses
|
### Default lenses
|
||||||
|
|
||||||
@@ -80,11 +80,11 @@ Duplicated logic, reinvented std / framework primitives, inline patterns that ma
|
|||||||
|
|
||||||
#### quality
|
#### quality
|
||||||
|
|
||||||
Structural and maintainability issues: redundant state (fields that duplicate each other, derived values cached unnecessarily, bools that encode the same thing as an adjacent enum), leaky abstractions (pub(crate) fields poked directly when a method would be cleaner), stringly-typed code, parameter sprawl, unnecessary comments (especially WHAT-not-WHY narration, section dividers, PR/commit/task references in code), nested conditionals 3+ levels deep that could flatten, dead code, brittle test fixtures. Skip anything a linter/formatter would catch - the gate handles those.
|
Structural and maintainability issues: redundant state (fields that duplicate each other, derived values cached unnecessarily, bools that encode the same thing as an adjacent enum), leaky abstractions (pub(crate) fields poked directly when a method would be cleaner), stringly-typed code, parameter sprawl, unnecessary comments (especially WHAT-not-WHY narration, section dividers, PR/commit/task references in code), nested conditionals 3+ levels deep that could flatten, dead code, brittle test fixtures. Skip anything a linter/formatter would catch, since the gate handles those.
|
||||||
|
|
||||||
#### efficiency
|
#### efficiency
|
||||||
|
|
||||||
Hot-path bloat (anything that runs per-frame / per-event / per-request / per-render): redundant allocations, repeated hashmap lookups, multiple tree walks where one would do, reconstructing immutable objects every call. Recurring no-op updates (state writes that trigger downstream invalidation even when the value didn't change). Unbounded growth in caches or maps. Overly broad operations (scanning entire collections to find one thing). Note "hot path" context per project - for GUI/game code it's paint/layout/event loops; for servers it's request handlers; for data pipelines it's per-record transforms.
|
Hot-path bloat (anything that runs per-frame / per-event / per-request / per-render): redundant allocations, repeated hashmap lookups, multiple tree walks where one would do, reconstructing immutable objects every call. Recurring no-op updates (state writes that trigger downstream invalidation even when the value didn't change). Unbounded growth in caches or maps. Overly broad operations (scanning entire collections to find one thing). Note "hot path" context per project. For GUI/game code it's paint/layout/event loops. For servers it's request handlers. For data pipelines it's per-record transforms.
|
||||||
|
|
||||||
#### errors
|
#### errors
|
||||||
|
|
||||||
@@ -96,7 +96,7 @@ Public API surface appropriateness. `pub` on items that could be `pub(crate)` (c
|
|||||||
|
|
||||||
#### bugs
|
#### bugs
|
||||||
|
|
||||||
Correctness issues: logic errors, off-by-one, missing bounds checks, wrong condition in if, incorrect loop termination, type confusion that compiles but is wrong, borrow patterns that compile but violate invariants (lifetimes too permissive / not permissive enough). Only flag with high confidence - "this might be wrong depending on inputs" is NOT a finding. Include language-specific bug profiles: Rust bugs often involve lifetimes/Send/Sync; JS/TS bugs often involve null/undefined, async Promise lifetimes, reference equality mistakes.
|
Correctness issues: logic errors, off-by-one, missing bounds checks, wrong condition in if, incorrect loop termination, type confusion that compiles but is wrong, borrow patterns that compile but violate invariants (lifetimes too permissive / not permissive enough). Only flag with high confidence. "This might be wrong depending on inputs" is NOT a finding. Include language-specific bug profiles. Rust bugs often involve lifetimes/Send/Sync. JS/TS bugs often involve null/undefined, async Promise lifetimes, reference equality mistakes.
|
||||||
|
|
||||||
### Opt-in lenses
|
### Opt-in lenses
|
||||||
|
|
||||||
@@ -126,9 +126,9 @@ Dependency hygiene: duplicate deps at different versions, unused deps, feature f
|
|||||||
|
|
||||||
Once lens agents return, do NOT present findings to the user yet. Launch a single validation agent with all raw findings as input:
|
Once lens agents return, do NOT present findings to the user yet. Launch a single validation agent with all raw findings as input:
|
||||||
|
|
||||||
> "Each finding below was flagged by a lens agent. For each one, confirm independently whether it's real by reading the referenced file(s) and the surrounding context. Classify each as: **confirmed** (high-confidence real issue), **misfire** (wrong reading of the code, semantics differ from what the agent thought), or **context-dependent** (real only under unstated assumptions - treat as misfire). Return the confirmed list, with the reasoning for any misfires you're dropping so the aggregator can double-check."
|
> "Each finding below was flagged by a lens agent. For each one, confirm independently whether it's real by reading the referenced file(s) and the surrounding context. Classify each as: **confirmed** (high-confidence real issue), **misfire** (wrong reading of the code, semantics differ from what the agent thought), or **context-dependent** (real only under unstated assumptions, treat as misfire). Return the confirmed list, with the reasoning for any misfires you're dropping so the aggregator can double-check."
|
||||||
|
|
||||||
This mirrors the confidence-scoring approach in Anthropic's `/code-review`. Misfires are noisy; validation keeps signal high.
|
This mirrors the confidence-scoring approach in Anthropic's `/code-review`. Misfires are noisy. Validation keeps signal high.
|
||||||
|
|
||||||
Skip validation only if the raw finding count is ≤3 and each one is obviously right (saves tokens when the audit turns up almost nothing).
|
Skip validation only if the raw finding count is ≤3 and each one is obviously right (saves tokens when the audit turns up almost nothing).
|
||||||
|
|
||||||
@@ -136,10 +136,10 @@ Skip validation only if the raw finding count is ≤3 and each one is obviously
|
|||||||
|
|
||||||
Classify each confirmed finding into one of four tiers:
|
Classify each confirmed finding into one of four tiers:
|
||||||
|
|
||||||
- **Trivial fix** - small local change, clear improvement, no judgment call (e.g. "use existing helper at file.rs:42 instead of inline arithmetic").
|
- **Trivial fix**: small local change, clear improvement, no judgment call (e.g. "use existing helper at file.rs:42 instead of inline arithmetic").
|
||||||
- **Substantive fix** - real value, more than a few lines, clear scope (e.g. "merge two near-duplicate functions into one walker").
|
- **Substantive fix**: real value, more than a few lines, clear scope (e.g. "merge two near-duplicate functions into one walker").
|
||||||
- **Needs discussion** - chunky refactor, public API change, enum redesign, hot-path caching with lifetime gymnastics. Outcome shouldn't be assumed.
|
- **Needs discussion**: chunky refactor, public API change, enum redesign, hot-path caching with lifetime gymnastics. Outcome shouldn't be assumed.
|
||||||
- **Backlog item** - real but larger than cleanup. Should land in `TODO.md` (or equivalent) so it's not lost.
|
- **Backlog item**: real but larger than cleanup. Should land in `TODO.md` (or equivalent) so it's not lost.
|
||||||
|
|
||||||
This phase is **classification only**. Do NOT apply any fixes here, do NOT edit `TODO.md` here. Recording happens in the next phase, after the user has seen the proposed plan.
|
This phase is **classification only**. Do NOT apply any fixes here, do NOT edit `TODO.md` here. Recording happens in the next phase, after the user has seen the proposed plan.
|
||||||
|
|
||||||
@@ -153,10 +153,10 @@ Before presenting anything, use `TaskCreate` to record one task per non-empty ti
|
|||||||
|
|
||||||
### Tier order
|
### Tier order
|
||||||
|
|
||||||
1. **Suggested backlog additions** - lock these in first. A single `TODO.md` append is cheap and ensures nothing is lost if a later code change goes sideways.
|
1. **Suggested backlog additions**: lock these in first. A single `TODO.md` append is cheap and ensures nothing is lost if a later code change goes sideways.
|
||||||
2. **Trivial fixes** - grouped by theme (e.g. "use existing helpers", "drop dead code"), one commit per theme.
|
2. **Trivial fixes**: grouped by theme (e.g. "use existing helpers", "drop dead code"), one commit per theme.
|
||||||
3. **Substantive fixes** - one commit per logical change. Commit message explains the why.
|
3. **Substantive fixes**: one commit per logical change. Commit message explains the why.
|
||||||
4. **Needs discussion** - present each as: issue, two options, tradeoff. Apply only if the user gives specific direction.
|
4. **Needs discussion**: present each as: issue, two options, tradeoff. Apply only if the user gives specific direction.
|
||||||
|
|
||||||
Skip any tier that has zero items.
|
Skip any tier that has zero items.
|
||||||
|
|
||||||
@@ -179,8 +179,8 @@ Items are **numbered 1..N within the tier**, resetting each tier so the user can
|
|||||||
```
|
```
|
||||||
### <Tier name> (<count>)
|
### <Tier name> (<count>)
|
||||||
|
|
||||||
1. file.rs:42 - <issue>. Fix: <one-line change>.
|
1. file.rs:42, <issue>. Fix: <one-line change>.
|
||||||
2. file.rs:88 - <issue>. Fix: <one-line change>.
|
2. file.rs:88, <issue>. Fix: <one-line change>.
|
||||||
3. ...
|
3. ...
|
||||||
|
|
||||||
Ready to apply? "Go ahead" for all, or tell me which numbers to skip.
|
Ready to apply? "Go ahead" for all, or tell me which numbers to skip.
|
||||||
@@ -191,7 +191,7 @@ For **Needs discussion**, expand each item:
|
|||||||
```
|
```
|
||||||
### Needs discussion (<count>)
|
### Needs discussion (<count>)
|
||||||
|
|
||||||
1. file.rs:220 - <issue>.
|
1. file.rs:220, <issue>.
|
||||||
- Option a: <option>
|
- Option a: <option>
|
||||||
- Option b: <option>
|
- Option b: <option>
|
||||||
- Tradeoff: <one line>
|
- Tradeoff: <one line>
|
||||||
@@ -233,15 +233,15 @@ After the final tier, give a brief summary: what was applied, what was backlogge
|
|||||||
- Stack findings from multiple lenses into one commit without clear grouping.
|
- Stack findings from multiple lenses into one commit without clear grouping.
|
||||||
- Invent findings to fill space if a lens comes up empty. "Nothing to flag" is a valid outcome and should be reported as such.
|
- Invent findings to fill space if a lens comes up empty. "Nothing to flag" is a valid outcome and should be reported as such.
|
||||||
- Re-raise items already in `TODO.md` / `BACKLOG.md`.
|
- Re-raise items already in `TODO.md` / `BACKLOG.md`.
|
||||||
- Run `/audit` against a codebase that was just audited - diminishing returns.
|
- Run `/audit` against a codebase that was just audited (diminishing returns).
|
||||||
|
|
||||||
## Parallelism note
|
## Parallelism note
|
||||||
|
|
||||||
All lens agents run in parallel (single message, multiple `Agent` tool uses). Six agents is the upper bound where parallelism still pays; above that, coordination overhead catches up. Keep opt-in lenses opt-in for this reason - running all 11 in parallel would be wasteful for most projects.
|
All lens agents run in parallel (single message, multiple `Agent` tool uses). Six agents is the upper bound where parallelism still pays. Above that, coordination overhead catches up. Keep opt-in lenses opt-in for this reason. Running all 11 in parallel would be wasteful for most projects.
|
||||||
|
|
||||||
## When NOT to use this skill
|
## When NOT to use this skill
|
||||||
|
|
||||||
- **Reviewing in-flight work** → `/simplify` against the diff.
|
- **Reviewing in-flight work** → `/check` against the diff.
|
||||||
- **PR review** → `/review` (built-in) or `/code-review:code-review` if that plugin is installed.
|
- **PR review** → `/review` (built-in) or `/code-review:code-review` if that plugin is installed.
|
||||||
- **Security-only audit of a diff** → `/security-review`.
|
- **Security-only audit of a diff** → `/security-review`.
|
||||||
- **Back-to-back on the same code** → re-runs produce sharply diminishing returns.
|
- **Back-to-back on the same code** → re-runs produce sharply diminishing returns.
|
||||||
|
|||||||
@@ -0,0 +1,218 @@
|
|||||||
|
---
|
||||||
|
name: check
|
||||||
|
description: Daily-driver review of in-flight changes (or specified files) for reuse, simplification, efficiency, and high-confidence bugs. Spawns one Opus subagent to keep file reads off the main thread. Default mode diffs the working tree and staged changes against the main branch. Pass paths (`/check src/foo.rs`) to review file state instead. Use as the fast pre-PR review pass. For a deeper whole-tree retrospective, use /audit.
|
||||||
|
---
|
||||||
|
|
||||||
|
# /check
|
||||||
|
|
||||||
|
Pre-PR review pass over work-in-progress. The main thread spawns one Opus subagent to do the analysis in isolation, then receives a structured finding list to present and apply.
|
||||||
|
|
||||||
|
## Invocation
|
||||||
|
|
||||||
|
```
|
||||||
|
/check # diff working tree + staged vs main/master
|
||||||
|
/check --staged # only staged changes
|
||||||
|
/check --branch develop # diff against a different base
|
||||||
|
/check src/foo.rs # review one file as-is (ignores diff)
|
||||||
|
/check src/foo/ src/bar.rs # review multiple paths as-is
|
||||||
|
```
|
||||||
|
|
||||||
|
Paths win: if any are given, ignore the diff and review file state.
|
||||||
|
|
||||||
|
## Step 1: Determine scope
|
||||||
|
|
||||||
|
Diff mode (no paths):
|
||||||
|
|
||||||
|
1. Resolve the base branch. Try `git symbolic-ref refs/remotes/origin/HEAD`, then `main`, then `master`. Ask the user if neither exists.
|
||||||
|
2. With `--branch <name>`, use that base instead.
|
||||||
|
3. With `--staged`, scope to staged changes only (`git diff --cached <base>`). Otherwise include working tree (`git diff <base>...HEAD` plus uncommitted).
|
||||||
|
4. Enumerate touched files (`git diff --name-only ...`).
|
||||||
|
|
||||||
|
File mode (paths passed): take the paths verbatim. The subagent recurses into directories.
|
||||||
|
|
||||||
|
Do not read the in-scope source files in the main thread. The subagent owns review-time reads.
|
||||||
|
|
||||||
|
## Step 2: Spawn the review subagent
|
||||||
|
|
||||||
|
One call to the `Agent` tool with:
|
||||||
|
|
||||||
|
- `subagent_type: general-purpose`
|
||||||
|
- `model: opus` (regardless of session model)
|
||||||
|
- `description`: `"/check review pass"`
|
||||||
|
- `prompt`: composed as `<scope block>\n\n<review instructions, verbatim from Step 3>`
|
||||||
|
|
||||||
|
The scope block tells the subagent what to review:
|
||||||
|
|
||||||
|
```
|
||||||
|
# Scope
|
||||||
|
|
||||||
|
Mode: <diff|file>
|
||||||
|
Base branch: <name> # diff mode only
|
||||||
|
Staged only: <yes|no> # diff mode only
|
||||||
|
Touched files: # diff mode: from `git diff --name-only`
|
||||||
|
- path/to/file.rs
|
||||||
|
- ...
|
||||||
|
Paths to review: # file mode
|
||||||
|
- src/foo.rs
|
||||||
|
- src/bar/
|
||||||
|
Repo root: <absolute path>
|
||||||
|
```
|
||||||
|
|
||||||
|
## Step 3: Review instructions (passed verbatim to the subagent)
|
||||||
|
|
||||||
|
You are reviewing the in-flight work (or specified paths) described in the scope block above. Surface concrete, high-confidence improvements before this work ships. Be useful, not exhaustive.
|
||||||
|
|
||||||
|
### Context
|
||||||
|
|
||||||
|
Read each in-scope file fully, not just the diff hunks. Reuse and efficiency findings need surrounding context. Also read `CLAUDE.md` at the repo root and in any touched subdirectories, plus `TODO.md` / `BACKLOG.md` if present. Honor local conventions. Do not re-raise anything already deferred.
|
||||||
|
|
||||||
|
When writing finding text, do not use em-dashes, en-dashes, semicolons, or `--` as em-dash substitutes. Use periods or commas instead.
|
||||||
|
|
||||||
|
### What to flag
|
||||||
|
|
||||||
|
#### Reuse (look here first)
|
||||||
|
|
||||||
|
- New helpers or inline logic that duplicates an existing utility in this codebase. Scan the file's directory and shared locations (`utils/`, `lib/`, `helpers/`, modules adjacent to the changed ones) before concluding nothing exists.
|
||||||
|
- Reinvented standard library or framework primitives (manual path joining, ad-hoc env var parsing, custom type guards, hand-rolled string manipulation, bespoke deep-equals, etc.) where the language or an in-repo helper already covers it.
|
||||||
|
- Inconsistent import paths for the same thing (some sites use a top-level re-export, others reach into deep paths). Pick the path the rest of the codebase uses.
|
||||||
|
- For each duplication finding, name the existing helper with `file:line` so the fix is concrete.
|
||||||
|
|
||||||
|
#### Overengineering
|
||||||
|
|
||||||
|
- New helpers, traits, generics, or indirection introduced for one caller. Inline.
|
||||||
|
- Flags, parameters, or fields with only one realistic value or use site.
|
||||||
|
- Layers added speculatively without a current second caller.
|
||||||
|
|
||||||
|
#### Dead code in the same change
|
||||||
|
|
||||||
|
- Functions added but uncalled elsewhere in the diff.
|
||||||
|
- Branches that cannot be reached given the rest of the change.
|
||||||
|
- New types, fields, or variants that nothing constructs.
|
||||||
|
|
||||||
|
#### Bugs (high-confidence only)
|
||||||
|
|
||||||
|
Flag only when the issue is clearly wrong on inspection. "This might fail under some inputs" is not a finding.
|
||||||
|
|
||||||
|
- Logic errors: off-by-one, wrong condition in `if`, inverted comparison, incorrect loop termination, swapped arguments.
|
||||||
|
- Missing bounds checks where the input is not otherwise validated.
|
||||||
|
- Type confusion that compiles but is wrong (mixing units, conflating IDs, using one enum where another was meant, comparing values of different shapes).
|
||||||
|
- Null or undefined access in JS/TS where the type allowed it but the path is reachable.
|
||||||
|
- Async lifetime mistakes: missing `await`, awaiting the wrong promise, dropping a future, fire-and-forget where the result was meant to be used.
|
||||||
|
- Rust borrow or lifetime patterns that compile but violate the intended invariant (lifetimes too permissive, `Send`/`Sync` boundaries crossed by accident).
|
||||||
|
- Resource leaks: opened but never closed, listeners attached but never removed on the failure path.
|
||||||
|
|
||||||
|
#### Efficiency
|
||||||
|
|
||||||
|
Focus on hot paths (per-render, per-request, per-frame, per-record) and on patterns that get worse with scale.
|
||||||
|
|
||||||
|
- Redundant work: duplicate computations, repeated file reads, redundant API or network calls, N+1 query patterns.
|
||||||
|
- Independent operations run sequentially that could run in parallel.
|
||||||
|
- New blocking work added to startup or per-render/per-request paths that did not have it before.
|
||||||
|
- State updates inside polling loops or event handlers that fire even when nothing changed. Add change-detection so downstream consumers are not woken for no-ops. If a setter takes an updater callback, verify same-value returns are honored so callers' own no-op guards are not silently defeated.
|
||||||
|
- Pre-checking a file or resource for existence before operating on it. This creates a race where the resource can change between the check and the use. Operate directly and handle the error from the operation instead.
|
||||||
|
- Unbounded data structures, missing cleanup, leaked event listeners or timers.
|
||||||
|
- Operations broader than they need to be (reading entire files when only a portion is needed, loading all rows when filtering for one, scanning a whole collection to find a single item where an index already exists).
|
||||||
|
|
||||||
|
#### Premature flexibility
|
||||||
|
|
||||||
|
- `Option<T>` where the value is always `Some`.
|
||||||
|
- Trait bounds or generic parameters wider than the actual use.
|
||||||
|
- Enum variants for cases that do not yet exist.
|
||||||
|
- `impl Trait` where the concrete type would read just as clearly.
|
||||||
|
|
||||||
|
#### Comments to delete
|
||||||
|
|
||||||
|
- Narration of what the code does (`// increments counter`).
|
||||||
|
- Section dividers (`// ---`, `// === FOO ===`).
|
||||||
|
- References to the task or commit that prompted the change ("added for X", "used by Y", "fixes #123").
|
||||||
|
- Stale docs where the code has moved past them.
|
||||||
|
|
||||||
|
#### Tests not pulling weight
|
||||||
|
|
||||||
|
- Tests that assert the language or framework's behavior rather than the new logic.
|
||||||
|
- Tests duplicated across files with no extra coverage.
|
||||||
|
- Fixtures wrapping a one-liner for a single test.
|
||||||
|
- Mocked tests where an integration test already covers the same path.
|
||||||
|
|
||||||
|
#### Scope creep
|
||||||
|
|
||||||
|
- Unrelated formatting changes mixed in with functional changes.
|
||||||
|
- Drive-by refactors in files outside the stated goal.
|
||||||
|
- "While I'm here" cleanup that belongs in a separate commit.
|
||||||
|
|
||||||
|
#### Style-rule violations
|
||||||
|
|
||||||
|
Flag these whenever they appear in the diff or files:
|
||||||
|
|
||||||
|
- Em-dashes, en-dashes, `--` as em-dash substitutes, and semicolons in prose (comments, docs, commit messages, PR descriptions).
|
||||||
|
- Programming-culture acronyms (YAGNI, DRY, KISS, SOLID, TDD, BDD, DDD, MVP, NBO, TOCTOU) in any prose. Replace with what they actually mean.
|
||||||
|
- Section divider comments.
|
||||||
|
- Milestone references in code docs ("for M2", "added in M1").
|
||||||
|
- `Co-Authored-By: Claude` trailers in commit messages.
|
||||||
|
|
||||||
|
Fold in any project-local `CLAUDE.md` conventions on top of these.
|
||||||
|
|
||||||
|
### What NOT to flag
|
||||||
|
|
||||||
|
- Low-confidence speculation. "This might break under some inputs" is not a bug finding. "This could be faster in theory" is not an efficiency finding. If you cannot point at a concrete failure mode or a concrete cost, leave it out.
|
||||||
|
- Issues a linter or formatter already catches.
|
||||||
|
- Missing tests or coverage gaps (separate concern, out of scope here).
|
||||||
|
- "You could rewrite this with library X" or other broad architectural rewrites.
|
||||||
|
- Items already in `TODO.md` / `BACKLOG.md`.
|
||||||
|
- Performance theatre: micro-optimizations outside hot paths.
|
||||||
|
|
||||||
|
### Output format
|
||||||
|
|
||||||
|
Return a single markdown block with this exact structure. No preamble or commentary outside the block.
|
||||||
|
|
||||||
|
```
|
||||||
|
## Findings
|
||||||
|
|
||||||
|
<N> findings on <scope description>.
|
||||||
|
|
||||||
|
1. path/to/file.rs:42-58
|
||||||
|
Issue: <one line, concrete>.
|
||||||
|
Fix: <one line, specific enough that the orchestrator can apply without re-reading the file>.
|
||||||
|
|
||||||
|
2. path/to/file.rs:88
|
||||||
|
Issue: <one line>.
|
||||||
|
Fix: <one line>.
|
||||||
|
```
|
||||||
|
|
||||||
|
For findings that touch public API surface or change behavior beyond cleanup, replace the `Fix:` line with:
|
||||||
|
|
||||||
|
```
|
||||||
|
Needs discussion: <reason>.
|
||||||
|
Option A: <option>.
|
||||||
|
Option B: <option>.
|
||||||
|
Tradeoff: <one line>.
|
||||||
|
```
|
||||||
|
|
||||||
|
If nothing is worth flagging, return exactly `Nothing flagged` and stop. Do not pad.
|
||||||
|
|
||||||
|
End of review instructions.
|
||||||
|
|
||||||
|
## Step 4: Present findings
|
||||||
|
|
||||||
|
Display the subagent's findings to the user as-is. If any are marked `Needs discussion`, present those after the auto-apply list as a separate block.
|
||||||
|
|
||||||
|
End with: `Apply all? "Go ahead", or tell me which numbers to skip.`
|
||||||
|
|
||||||
|
## Step 5: Apply approved findings
|
||||||
|
|
||||||
|
Only after the user approves:
|
||||||
|
|
||||||
|
1. Read each touched file once.
|
||||||
|
2. Apply the approved findings with the `Edit` tool. The subagent's `Fix:` line should be specific enough to apply without ambiguity. If a fix is too vague, surface that and ask the user.
|
||||||
|
3. Diff mode: do not commit. The user is mid-PR.
|
||||||
|
4. File mode (no in-flight diff): default to one commit per theme using Conventional Commits.
|
||||||
|
5. Run the project gate (`scripts/prepare.sh`, `pnpm check`, `cargo test`, equivalent) when applying multiple substantive changes.
|
||||||
|
|
||||||
|
## Do not
|
||||||
|
|
||||||
|
- Read files under review in the main thread before Step 5. The subagent owns review-time reads.
|
||||||
|
- Rewrite history. The user is preparing a PR. Do not squash or amend existing commits.
|
||||||
|
- Auto-apply changes to public API surface. Lift those into `Needs discussion`.
|
||||||
|
- Reference the check run in commit messages ("from /check", "review cleanup"). Describe the change.
|
||||||
|
- Surface a "bug" or "perf" finding the subagent was not confident about. The signal value comes from the user trusting flagged bugs are real.
|
||||||
|
- Pad. If the subagent returns `Nothing flagged`, that is the right answer.
|
||||||
@@ -1,166 +0,0 @@
|
|||||||
#!/usr/bin/env bash
|
|
||||||
set -euo pipefail
|
|
||||||
|
|
||||||
WHISPER_DIR="${WHISPER_DIR:-$HOME/repos/whisper.cpp}"
|
|
||||||
WHISPER_MODEL="${WHISPER_MODEL:-$WHISPER_DIR/models/ggml-large-v3.bin}"
|
|
||||||
WHISPER_BIN="${WHISPER_BIN:-$WHISPER_DIR/build/bin/whisper-cli}"
|
|
||||||
VAD_MODEL="${VAD_MODEL:-$WHISPER_DIR/models/ggml-silero-v6.2.0.bin}"
|
|
||||||
TRANSLATE="${TRANSLATE:-1}"
|
|
||||||
SRC_LANG="${SRC_LANG:-auto}"
|
|
||||||
FORCE="${FORCE:-0}"
|
|
||||||
USE_VAD="${USE_VAD:-1}"
|
|
||||||
MAX_LEN="${MAX_LEN:-84}"
|
|
||||||
LINE_LEN="${LINE_LEN:-42}"
|
|
||||||
OUTPUT=""
|
|
||||||
|
|
||||||
usage() {
|
|
||||||
cat <<EOF
|
|
||||||
Usage: video2srt [options] <video> [<video>...]
|
|
||||||
|
|
||||||
Generates an English .srt next to each video using whisper.cpp.
|
|
||||||
|
|
||||||
Options:
|
|
||||||
-t, --transcribe Transcribe in source language (default: translate to English)
|
|
||||||
-l, --lang CODE Force source language (default: auto-detect)
|
|
||||||
-m, --model PATH Path to ggml model (default: $WHISPER_MODEL)
|
|
||||||
-o, --output PATH Output .srt path (single input only; default: <video>.srt)
|
|
||||||
-f, --force Overwrite existing .srt
|
|
||||||
--no-vad Disable Silero VAD pre-filtering (VAD reduces hallucination loops)
|
|
||||||
--max-len N Max characters per SRT entry, 0 to disable (default: $MAX_LEN)
|
|
||||||
--line-len N Max characters per visible line; longer entries wrap to 2 lines, 0 to disable (default: $LINE_LEN)
|
|
||||||
-h, --help Show this help
|
|
||||||
|
|
||||||
Env overrides: WHISPER_DIR, WHISPER_MODEL, WHISPER_BIN
|
|
||||||
EOF
|
|
||||||
}
|
|
||||||
|
|
||||||
args=()
|
|
||||||
while [[ $# -gt 0 ]]; do
|
|
||||||
case "$1" in
|
|
||||||
-t|--transcribe) TRANSLATE=0; shift ;;
|
|
||||||
-l|--lang) SRC_LANG="$2"; shift 2 ;;
|
|
||||||
-m|--model) WHISPER_MODEL="$2"; shift 2 ;;
|
|
||||||
-o|--output) OUTPUT="$2"; shift 2 ;;
|
|
||||||
-f|--force) FORCE=1; shift ;;
|
|
||||||
--no-vad) USE_VAD=0; shift ;;
|
|
||||||
--max-len) MAX_LEN="$2"; shift 2 ;;
|
|
||||||
--line-len) LINE_LEN="$2"; shift 2 ;;
|
|
||||||
-h|--help) usage; exit 0 ;;
|
|
||||||
--) shift; args+=("$@"); break ;;
|
|
||||||
-*) echo "Unknown option: $1" >&2; usage >&2; exit 2 ;;
|
|
||||||
*) args+=("$1"); shift ;;
|
|
||||||
esac
|
|
||||||
done
|
|
||||||
|
|
||||||
if [[ ${#args[@]} -eq 0 ]]; then
|
|
||||||
usage >&2; exit 2
|
|
||||||
fi
|
|
||||||
|
|
||||||
if [[ -n "$OUTPUT" && ${#args[@]} -gt 1 ]]; then
|
|
||||||
echo "--output cannot be combined with multiple input files" >&2; exit 2
|
|
||||||
fi
|
|
||||||
|
|
||||||
[[ -x "$WHISPER_BIN" ]] || { echo "whisper-cli not found at $WHISPER_BIN" >&2; exit 1; }
|
|
||||||
[[ -f "$WHISPER_MODEL" ]] || { echo "model not found at $WHISPER_MODEL" >&2; exit 1; }
|
|
||||||
command -v ffmpeg >/dev/null || { echo "ffmpeg not installed" >&2; exit 1; }
|
|
||||||
|
|
||||||
for video in "${args[@]}"; do
|
|
||||||
if [[ ! -f "$video" ]]; then
|
|
||||||
echo "skip: $video (not a file)" >&2
|
|
||||||
continue
|
|
||||||
fi
|
|
||||||
|
|
||||||
if [[ -n "$OUTPUT" ]]; then
|
|
||||||
out_stem="${OUTPUT%.srt}"
|
|
||||||
out_dir=$(dirname -- "$out_stem")
|
|
||||||
else
|
|
||||||
out_dir=$(dirname -- "$video")
|
|
||||||
base=$(basename -- "$video")
|
|
||||||
out_stem="$out_dir/${base%.*}"
|
|
||||||
fi
|
|
||||||
srt="$out_stem.srt"
|
|
||||||
|
|
||||||
if [[ -f "$srt" && "$FORCE" != "1" ]]; then
|
|
||||||
echo "skip: $srt exists (use --force to overwrite)"
|
|
||||||
continue
|
|
||||||
fi
|
|
||||||
|
|
||||||
echo ">> $video"
|
|
||||||
tmpwav=$(mktemp --suffix=.wav)
|
|
||||||
trap 'rm -f "$tmpwav"' EXIT
|
|
||||||
|
|
||||||
ffmpeg -hide_banner -loglevel error -y \
|
|
||||||
-i "$video" -vn -ar 16000 -ac 1 -c:a pcm_s16le "$tmpwav"
|
|
||||||
|
|
||||||
mkdir -p -- "$out_dir"
|
|
||||||
whisper_args=(
|
|
||||||
-m "$WHISPER_MODEL"
|
|
||||||
-f "$tmpwav"
|
|
||||||
-of "$out_stem"
|
|
||||||
--output-srt
|
|
||||||
-l "$SRC_LANG"
|
|
||||||
-mc 0
|
|
||||||
)
|
|
||||||
[[ "$TRANSLATE" == "1" ]] && whisper_args+=(--translate)
|
|
||||||
if [[ "$MAX_LEN" -gt 0 ]]; then
|
|
||||||
whisper_args+=(--max-len "$MAX_LEN" --split-on-word)
|
|
||||||
fi
|
|
||||||
if [[ "$USE_VAD" == "1" ]]; then
|
|
||||||
if [[ -f "$VAD_MODEL" ]]; then
|
|
||||||
whisper_args+=(--vad --vad-model "$VAD_MODEL")
|
|
||||||
else
|
|
||||||
echo "warn: VAD model not found at $VAD_MODEL, running without VAD" >&2
|
|
||||||
echo " download with: sh $WHISPER_DIR/models/download-vad-model.sh silero-v6.2.0" >&2
|
|
||||||
fi
|
|
||||||
fi
|
|
||||||
|
|
||||||
"$WHISPER_BIN" "${whisper_args[@]}"
|
|
||||||
|
|
||||||
if [[ "$LINE_LEN" -gt 0 ]]; then
|
|
||||||
python3 - "$srt" "$LINE_LEN" <<'PYEOF'
|
|
||||||
import re, sys, pathlib
|
|
||||||
|
|
||||||
path = pathlib.Path(sys.argv[1])
|
|
||||||
max_line = int(sys.argv[2])
|
|
||||||
|
|
||||||
def wrap(text, limit):
|
|
||||||
text = " ".join(text.split())
|
|
||||||
if len(text) <= limit:
|
|
||||||
return text
|
|
||||||
words = text.split(" ")
|
|
||||||
if len(words) < 2:
|
|
||||||
return text
|
|
||||||
best = None
|
|
||||||
best_score = None
|
|
||||||
cum = 0
|
|
||||||
for i, w in enumerate(words[:-1]):
|
|
||||||
cum += len(w) + (1 if i > 0 else 0)
|
|
||||||
top, bot = cum, len(text) - cum - 1
|
|
||||||
score = abs(bot - top)
|
|
||||||
if top > limit:
|
|
||||||
score += (top - limit) * 100
|
|
||||||
if bot > limit:
|
|
||||||
score += (bot - limit) * 100
|
|
||||||
if w.rstrip(",.!?:;") != w:
|
|
||||||
score -= 8
|
|
||||||
if best_score is None or score < best_score:
|
|
||||||
best_score, best = score, i
|
|
||||||
return " ".join(words[:best+1]) + "\n" + " ".join(words[best+1:])
|
|
||||||
|
|
||||||
blocks = re.split(r"\n\n+", path.read_text(encoding="utf-8").strip())
|
|
||||||
out = []
|
|
||||||
for b in blocks:
|
|
||||||
lines = b.split("\n")
|
|
||||||
if len(lines) < 3:
|
|
||||||
out.append(b)
|
|
||||||
continue
|
|
||||||
head, body = lines[:2], " ".join(lines[2:])
|
|
||||||
out.append("\n".join(head + [wrap(body, max_line)]))
|
|
||||||
path.write_text("\n\n".join(out) + "\n", encoding="utf-8")
|
|
||||||
PYEOF
|
|
||||||
fi
|
|
||||||
|
|
||||||
rm -f "$tmpwav"
|
|
||||||
trap - EXIT
|
|
||||||
echo "<< $srt"
|
|
||||||
done
|
|
||||||
@@ -75,6 +75,3 @@ ignore = [
|
|||||||
"PYI011",
|
"PYI011",
|
||||||
"UP031"
|
"UP031"
|
||||||
]
|
]
|
||||||
|
|
||||||
[tool.pyrefly]
|
|
||||||
use_untyped_imports = true
|
|
||||||
|
|||||||
Reference in New Issue
Block a user