Merge step (e): port ce-review content into ce-code-review
Applies triage doc 020 — moves local ce-review/* modifications into upstream's renamed ce-code-review/ skill. - persona-catalog.md: rebase to local's richer 21-persona catalog (4 always-on + 8 conditional + 4 stack-specific + 5 language/framework) plus 4 CE conditional agents. Rename all agent refs from compound-engineering:review:<name> to ce-<name>. Keep upstream's swift-ios persona. Drop rails personas (local deleted those reviewers). - SKILL.md: add ce-design-conformance-reviewer, ce-zip-agent-validator, and ce-tiangolo-fastapi-reviewer to the conditional team; add Zip Agent Validation section as Stage 6 item 11; update dispatch context and artifact preservation notes for the new CE conditional agents. - review-output-template.md: add Zip Agent Validation example section between Deployment Notes and Coverage, plus formatting-rule entry. - Delete ce-review/ directory (content fully ported). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1,6 +1,6 @@
|
||||
# Persona Catalog
|
||||
|
||||
18 reviewer personas organized into always-on, cross-cutting conditional, and stack-specific conditional layers, plus CE-specific agents. The orchestrator uses this catalog to select which reviewers to spawn for each review.
|
||||
21 reviewer personas organized into always-on, cross-cutting conditional, stack-specific conditional, and language/framework conditional layers, plus CE-specific agents. The orchestrator uses this catalog to select which reviewers to spawn for each review.
|
||||
|
||||
## Always-on (4 personas + 2 CE agents)
|
||||
|
||||
@@ -33,36 +33,49 @@ Spawned when the orchestrator identifies relevant patterns in the diff. The orch
|
||||
| `api-contract` | `ce-api-contract-reviewer` | Route definitions, serializer/interface changes, event schemas, exported type signatures, API versioning |
|
||||
| `data-migrations` | `ce-data-migrations-reviewer` | Migration files, schema changes, backfill scripts, data transformations |
|
||||
| `reliability` | `ce-reliability-reviewer` | Error handling, retry logic, circuit breakers, timeouts, background jobs, async handlers, health checks |
|
||||
| `adversarial` | `ce-adversarial-reviewer` | Diff has >=50 changed non-test, non-generated, non-lockfile lines, OR touches auth, payments, data mutations, external API integrations, or other high-risk domains |
|
||||
| `adversarial` | `ce-adversarial-reviewer` | Diff has >=50 changed lines of executable code (not prose/instruction Markdown, JSON schemas, or config), OR touches auth, payments, data mutations, external API integrations, or other high-risk domains regardless of file type |
|
||||
| `cli-readiness` | `ce-cli-readiness-reviewer` | CLI command definitions, argument parsing, CLI framework usage, command handler implementations |
|
||||
| `previous-comments` | `ce-previous-comments-reviewer` | **PR-only.** Reviewing a PR that has existing review comments or review threads from prior review rounds. Skip entirely when no PR metadata was gathered in Stage 1. |
|
||||
|
||||
## Stack-Specific Conditional (6 personas)
|
||||
## Stack-Specific Conditional (4 personas)
|
||||
|
||||
These reviewers keep their original opinionated lens. They are additive with the cross-cutting personas above, not replacements for them.
|
||||
|
||||
| Persona | Agent | Select when diff touches... |
|
||||
|---------|-------|---------------------------|
|
||||
| `dhh-rails` | `ce-dhh-rails-reviewer` | Rails architecture, service objects, authentication/session choices, Hotwire-vs-SPA boundaries, or abstractions that may fight Rails conventions |
|
||||
| `kieran-rails` | `ce-kieran-rails-reviewer` | Rails controllers, models, views, jobs, components, routes, or other application-layer Ruby code where clarity and conventions matter |
|
||||
| `kieran-python` | `ce-kieran-python-reviewer` | Python modules, endpoints, services, scripts, or typed domain code |
|
||||
| `kieran-typescript` | `ce-kieran-typescript-reviewer` | TypeScript components, services, hooks, utilities, or shared types |
|
||||
| `julik-frontend-races` | `ce-julik-frontend-races-reviewer` | Stimulus/Turbo controllers, DOM event wiring, timers, async UI flows, animations, or frontend state transitions with race potential |
|
||||
| `swift-ios` | `ce-swift-ios-reviewer` | Swift files, SwiftUI views, UIKit controllers, `.entitlements`, `PrivacyInfo.xcprivacy`, `.xcdatamodeld`, `Package.swift`, `Package.resolved`, storyboards, XIBs, or semantic build-setting / target-membership / code-signing changes in `.pbxproj` |
|
||||
|
||||
## CE Conditional Agents (migration-specific)
|
||||
## Language & Framework Conditional (5 personas)
|
||||
|
||||
These CE-native agents provide specialized analysis beyond what the persona agents cover. Spawn them when the diff includes database migrations, schema.rb, or data backfills.
|
||||
Spawned when the orchestrator identifies language or framework-specific patterns in the diff. These provide deeper domain expertise than the general-purpose personas above.
|
||||
|
||||
| Agent | Focus |
|
||||
|-------|-------|
|
||||
| `ce-schema-drift-detector` | Cross-references schema.rb changes against included migrations to catch unrelated drift |
|
||||
| `ce-deployment-verification-agent` | Produces Go/No-Go deployment checklist with SQL verification queries and rollback procedures |
|
||||
| Persona | Agent | Select when diff touches... |
|
||||
|---------|-------|---------------------------|
|
||||
| `python-quality` | `ce-kieran-python-reviewer` | Python files, FastAPI routes, Pydantic models, async/await patterns, SQLAlchemy usage |
|
||||
| `fastapi-philosophy` | `ce-tiangolo-fastapi-reviewer` | FastAPI application code, dependency injection, response models, middleware, OpenAPI schemas |
|
||||
| `typescript-quality` | `ce-kieran-typescript-reviewer` | TypeScript files, React components, type definitions, generic patterns |
|
||||
| `frontend-races` | `ce-julik-frontend-races-reviewer` | Frontend JavaScript, Stimulus controllers, event listeners, async UI code, animations, DOM lifecycle |
|
||||
| `architecture` | `ce-architecture-strategist` | New services, module boundaries, dependency graphs, API layer changes, package structure |
|
||||
|
||||
## CE Conditional Agents (design, migration & external review)
|
||||
|
||||
These CE-native agents provide specialized analysis beyond what the persona agents cover. Spawn them when the diff includes database migrations, schema.rb, data backfills, design documents, or when the PR originates from specific platforms.
|
||||
|
||||
| Agent | Focus | Select when... |
|
||||
|-------|-------|----------------|
|
||||
| `ce-design-conformance-reviewer` | Surfaces deviations between the diff and the repo's design docs or implementation plan | The repo contains design documents (`docs/`, `docs/design/`, `docs/architecture/`, `docs/specs/`) or an active plan matching the current branch |
|
||||
| `ce-schema-drift-detector` | Cross-references schema.rb changes against included migrations to catch unrelated drift | The diff includes migration files or schema.rb |
|
||||
| `ce-deployment-verification-agent` | Produces Go/No-Go deployment checklist with SQL verification queries and rollback procedures | The diff includes migration files, schema.rb, or data backfills |
|
||||
| `ce-zip-agent-validator` | Pressure-tests zip-agent review comments for validity against full codebase context | The PR URL contains `git.zoominfo.com` |
|
||||
|
||||
## Selection rules
|
||||
|
||||
1. **Always spawn all 4 always-on personas** plus the 2 CE always-on agents.
|
||||
2. **For each cross-cutting conditional persona**, the orchestrator reads the diff and decides whether the persona's domain is relevant. This is a judgment call, not a keyword match.
|
||||
3. **For each stack-specific conditional persona**, use file types and changed patterns as a starting point, then decide whether the diff actually introduces meaningful work for that reviewer. Do not spawn language-specific reviewers just because one config or generated file happens to match the extension.
|
||||
4. **For CE conditional agents**, spawn when the diff includes migration files (`db/migrate/*.rb`, `db/schema.rb`) or data backfill scripts.
|
||||
5. **Announce the team** before spawning with a one-line justification per conditional reviewer selected.
|
||||
4. **For each language/framework conditional persona**, check whether the diff touches language or framework-specific patterns that warrant deeper domain expertise. These are additive with stack-specific personas, not replacements.
|
||||
5. **For CE conditional agents**, spawn when applicable: migration files (`db/migrate/*.rb`, `db/schema.rb`) or data backfill scripts trigger ce-schema-drift-detector and ce-deployment-verification-agent; design documents or active plans trigger ce-design-conformance-reviewer; PR URLs containing `git.zoominfo.com` trigger ce-zip-agent-validator.
|
||||
6. **Announce the team** before spawning with a one-line justification per conditional reviewer selected.
|
||||
|
||||
@@ -77,6 +77,15 @@ Use this **exact format** when presenting synthesized review findings. Findings
|
||||
- Verify: `SELECT COUNT(*) FROM exports WHERE status IS NULL;` should stay at `0`
|
||||
- Rollback: keep the old export path available until the backfill has been validated
|
||||
|
||||
### Zip Agent Validation
|
||||
|
||||
- Evaluated: 8 zip-agent comments
|
||||
- Validated: 2 (appear as findings #3 and #6 above)
|
||||
- Collapsed: 6
|
||||
- `app/services/order_service.rb:45`: "Missing error handling" -- handled by ApplicationService base class rescue
|
||||
- `app/controllers/api/orders_controller.rb:18`: "Unbounded query" -- pagination enforced by ApiController concern
|
||||
- _(4 more collapsed for stylistic/formatting concerns)_
|
||||
|
||||
### Coverage
|
||||
|
||||
- Suppressed: 2 findings below anchor 75 (1 at anchor 50, 1 at anchor 25)
|
||||
@@ -130,6 +139,7 @@ This fails because: no pipe-delimited tables, no severity-grouped `###` headers,
|
||||
- **Agent-Native Gaps section** -- results from ce-agent-native-reviewer. Omit if no gaps found.
|
||||
- **Schema Drift Check section** -- results from ce-schema-drift-detector. Omit if the agent did not run.
|
||||
- **Deployment Notes section** -- key checklist items from ce-deployment-verification-agent. Omit if the agent did not run.
|
||||
- **Zip Agent Validation section** -- summary of ce-zip-agent-validator evaluation. Omit if the agent did not run.
|
||||
- **Coverage section** -- suppressed count, residual risks, testing gaps, failed reviewers
|
||||
- **Summary uses blockquotes** for verdict, reasoning, and fix order
|
||||
- **Horizontal rule** (`---`) separates findings from verdict
|
||||
|
||||
Reference in New Issue
Block a user