Files
gstack/review/specialists/maintainability.md
T
Garry Tan a4a181ca92 feat: Review Army — parallel specialist reviewers for /review (v0.14.3.0) (#692)
* feat: extend gstack-diff-scope with SCOPE_MIGRATIONS, SCOPE_API, SCOPE_AUTH

Three new scope signals for Review Army specialist activation:
- SCOPE_MIGRATIONS: db/migrate/, prisma/migrations/, alembic/, *.sql
- SCOPE_API: *controller*, *route*, *endpoint*, *.graphql, openapi.*
- SCOPE_AUTH: *auth*, *session*, *jwt*, *oauth*, *permission*, *role*

* feat: add 7 specialist checklist files for Review Army

- testing.md (always-on): coverage gaps, flaky patterns, security enforcement
- maintainability.md (always-on): dead code, DRY, stale comments
- security.md (conditional): OWASP deep analysis, auth bypass, injection
- performance.md (conditional): N+1 queries, bundle impact, complexity
- data-migration.md (conditional): reversibility, lock duration, backfill
- api-contract.md (conditional): breaking changes, versioning, error format
- red-team.md (conditional): adversarial analysis, cross-cutting concerns

All use standard header with JSON output schema and NO FINDINGS fallback.

* feat: Review Army resolver — parallel specialist dispatch + merge

New resolver in review-army.ts generates template prose for:
- Stack detection and specialist selection
- Parallel Agent tool dispatch with learning-informed prompts
- JSON finding collection, fingerprint dedup, consensus highlighting
- PR quality score computation
- Red Team conditional dispatch

Registered as REVIEW_ARMY in resolvers/index.ts.

* refactor: restructure /review template for Review Army

- Replace Steps 4-4.75 with CRITICAL pass + {{REVIEW_ARMY}}
- Remove {{DESIGN_REVIEW_LITE}} and {{TEST_COVERAGE_AUDIT_REVIEW}}
  (subsumed into Design and Testing specialists respectively)
- Extract specialist-covered categories from checklist.md
- Keep CRITICAL + uncovered INFORMATIONAL in main agent pass

* test: Review Army — 14 diff-scope tests + 7 E2E tests

- test/diff-scope.test.ts: 14 tests for all 9 scope signals
- test/skill-e2e-review-army.test.ts: 7 E2E tests
  Gate: migration safety, N+1 detection, delivery audit,
        quality score, JSON findings
  Periodic: red team, consensus
- Updated gen-skill-docs tests for new review structure
- Added touchfile entries and tier classifications

* docs: update SELF_LEARNING_V0.md with Release 2 status + Release 2.5

Mark Release 2 (Review Army) as in-progress. Add Release 2.5 for
deferred expansions (E1 adaptive gating, E3 test stubs, E5 cross-review
dedup, E7 specialist tracking).

* chore: bump version and changelog (v0.14.3.0)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
2026-03-30 22:07:50 -06:00

2.2 KiB

Maintainability Specialist Review Checklist

Scope: Always-on (every review) Output: JSON objects, one finding per line. Schema: {"severity":"INFORMATIONAL","confidence":N,"path":"file","line":N,"category":"maintainability","summary":"...","fix":"...","fingerprint":"path:line:maintainability","specialist":"maintainability"} If no findings: output NO FINDINGS and nothing else.


Categories

Dead Code & Unused Imports

  • Variables assigned but never read in the changed files
  • Functions/methods defined but never called (check with Grep across the repo)
  • Imports/requires that are no longer referenced after the change
  • Commented-out code blocks (either remove or explain why they exist)

Magic Numbers & String Coupling

  • Bare numeric literals used in logic (thresholds, limits, retry counts) — should be named constants
  • Error message strings used as query filters or conditionals elsewhere
  • Hardcoded URLs, ports, or hostnames that should be config
  • Duplicated literal values across multiple files

Stale Comments & Docstrings

  • Comments that describe old behavior after the code was changed in this diff
  • TODO/FIXME comments that reference completed work
  • Docstrings with parameter lists that don't match the current function signature
  • ASCII diagrams in comments that no longer match the code flow

DRY Violations

  • Similar code blocks (3+ lines) appearing multiple times within the diff
  • Copy-paste patterns where a shared helper would be cleaner
  • Configuration or setup logic duplicated across test files
  • Repeated conditional chains that could be a lookup table or map

Conditional Side Effects

  • Code paths that branch on a condition but forget a side effect on one branch
  • Log messages that claim an action happened but the action was conditionally skipped
  • State transitions where one branch updates related records but the other doesn't
  • Event emissions that only fire on the happy path, missing error/edge paths

Module Boundary Violations

  • Reaching into another module's internal implementation (accessing private-by-convention methods)
  • Direct database queries in controllers/views that should go through a service/model
  • Tight coupling between components that should communicate through interfaces