Skip to content

Reviewer Persona Catalog — selection triggers (SPEC-145 US-3)

The single reference for which reviewers /vt-d-4-review selects for a given change. The deterministic implementation is ../scripts/persona-select.sh — this catalog is the human-readable spec of the same rules, and test-persona-selection.sh drives the script (not this prose) so the two cannot silently diverge (AD-7, anti-theater per structural-test-assertions.md).

Selection set = floordiff-triggered code reviewersdeliverable-type reviewers.

Floor — always selected (never trigger-droppable)

Reviewer floor Rationale
security-sentinel true Constitution I mandates a security pass on every review, regardless of diff. Fired in ~100% of historical reviews.
spec-compliance-reviewer true Runs on every spec-driven review as the compliance gate.

The floor is asserted present in 100% of selections as a standalone HARD gate — not inferred from history.

Diff-triggered code reviewers (only when codedeliverable_types)

Three general code-core reviewers are always selected when code is a deliverable type (they fired on ~100% of historical code reviews), plus tdd-compliance-reviewer (SPEC-159 FR-1 — test-first compliance is meaningless without code; new, so no historical firing); the rest are scoped by the changed-file patterns.

Reviewer Trigger Notes
code-simplicity-reviewer code in types general code-core, always
performance-oracle code in types general code-core, always
pattern-recognition-specialist code in types general code-core, always
tdd-compliance-reviewer code in types test-first compliance, always within code (selected on unknown diffs too, EC-1; never for a non-code deliverable, EC-3); advisory — caps at Medium, never blocks (SPEC-159)
kieran-typescript-reviewer TS/JS files (*.ts *.tsx *.js *.jsx *.mjs *.cts *.mts) or unknown diff default language reviewer; swapped out below
kieran-python-reviewer *.py files or pyproject.toml / setup.py / requirements.txt in PROJECT_ROOT or unknown diff Python quality; additive — coexists with the TS reviewer on a mixed diff (below)
php-reviewer *.php / composer.json / composer.lock in the diff or a composer.json at PROJECT_ROOT or depth 1 (excluding vendor/, node_modules/) or unknown diff PHP quality; additive. Only the diff signal suppresses the TS reviewer — the marker never does (below). Depth 1 matters: VisiFair's marker is visifair-files/composer.json
angular-reviewer angular.json in PROJECT_ROOT and a language signal (below) replaces kieran-typescript-reviewer
nestjs-reviewer nest-cli.json in PROJECT_ROOT and a language signal (below) replaces kieran-typescript-reviewer
julik-frontend-races-reviewer frontend files (*.html *.htm *.vue *.svelte *.tsx *.jsx *.scss *.sass *.css *.component.ts) or unknown diff UI race/async
accessibility-reviewer frontend files or UI_SCOPE=frontend or unknown diff WCAG on UI changes
design-implementation-reviewer UI_SCOPE=frontend or unknown diff Figma/design match; conservative on unknown diff so it is not dropped from the non-regression roster
data-integrity-guardian DB files (*.sql, *.migration.*, paths under migrations/, db/, schema) migration/data safety

Framework-conditional swap

This is the single statement of the framework rule. Every other surface links here rather than restating it — before SPEC-168's review, seven restatements existed across four files and none of them matched the code (PAT-8). If you change the rule, change it here and change persona-select.sh; do not re-describe it elsewhere.

kieran-typescript-reviewer is the default TS reviewer. A framework marker replaces it:

  • angular-reviewer when angular.json is in PROJECT_ROOT,
  • nestjs-reviewer when nest-cli.json is in PROJECT_ROOT,

but only when a language signal already opened the language block — that is, when the diff is unknown, contains TS/JS, or carries a Python signal (*.py in the diff, or a pyproject.toml/setup.py/requirements.txt marker). A framework marker on its own selects nothing: angular.json plus a README.md-only diff yields no framework reviewer.

That last clause is the one every previous restatement dropped, and it is not cosmetic. Saying "only when angular.json exists" describes a selector that dispatches an Angular specialist to a .php diff — which is exactly what this branch did until the H-E fix.

Why the condition is the pre-PHP language signal rather than the tidier "TS or unknown diff": because on main a .py diff in an Angular repo did select angular-reviewer, and SC-4 forbids changing selection for non-PHP repos. Narrowing it would have been a silent drop — measured at 108 of 408 cells. Whether a framework reviewer should fire on a pure-Python diff is a real question, deferred to SPEC-179.

If both markers exist, both framework reviewers are selected and kieran-typescript-reviewer is dropped. Angular and Nest replace it because both frameworks are TypeScript, so the framework reviewer subsumes the generic one.

kieran-python-reviewer is different — it is additive, not a replacement. Either signal selects it: a .py file in the diff, or a Python marker (pyproject.toml / setup.py / requirements.txt) in PROJECT_ROOT. The diff is the primary trigger since BUG-021 — this toolkit's own Python lives in plugin scripts/ dirs under no root marker, so marker-only detection let an all-.py diff select no language reviewer at all. But a Python marker does not imply the diff is not TypeScript (a monorepo can carry Python tooling alongside a TS app). So a mixed Python-marker + .ts diff keeps both kieran-python-reviewer and kieran-typescript-reviewer, while a pure-Python diff selects kieran-python-reviewer alone. (BUG-004 HP-1: an earlier fix suppressed the TS reviewer on any Python marker, silently dropping it from mixed diffs — the exact drop class this catalog and T3 exist to prevent.)

php-reviewer is additive in the same way. PHP has two signals — *.php/ composer.json/composer.lock in the diff, and a composer.json marker in the repo — and they are different kinds of fact, so they are kept separate (SPEC-168 D2/D3, revised).

One rule governs both: repo-scoped facts stay out of diff-scoped decisions

"This project is PHP" (or Python) is grounds to select that project's own reviewer. It is not evidence about what the diff contains, so it may not move the TypeScript decision in either direction:

  • it may not suppress the TS reviewer — that was the original defect, and it silently cost every .md/.sql/.yaml-only change in a composer.json repo its TypeScript reviewer;
  • it may not conjure one either — the marker opens the language block, so an unguarded default dispatched kieran-typescript-reviewer to read a docs-only diff alongside the marker's own reviewer. Two language reviewers, no code.

Both halves were live at different times and the suite was green for each, because the test asserted whichever behaviour was current. The generic TS reviewer is therefore selected on diff-scoped grounds only: the diff contains TS, or the diff is unknown. Concretely — a marker plus a .md-only diff selects php-reviewer alone (Python: kieran-python-reviewer alone), a pure-PHP diff selects php-reviewer alone, and a mixed PHP+TS diff keeps both.

Cost of selecting on a repo-scoped marker (SC-1, accepted deliberately). A PHP repo gets php-reviewer on diffs proven to contain no PHP. In such a repo a known diff that would otherwise select exactly 8 reviewers selects 9 and crosses MAX_CONCURRENT, costing a second dispatch wave. This is the price of SC-1 — VisiFair's composer.json sits at depth 1 and its PHP must be reviewed even when the selector cannot see PHP in the changed-file list. Recorded here so the trade is a decision on the record rather than an inherited accident; revisit it with the MAX_CONCURRENT headroom question in SPEC-179, not piecemeal.

The marker probe searches PROJECT_ROOT and depth 1, because VisiFair's real composer.json lives in visifair-files/ and root-only detection would miss it. It is a command substitution, never [ -f "$ROOT"/*/composer.json ] (that glob exits 2 with 2+ matches and skips silently — EC-5) and never find … | grep -q . (that pipeline is unsafe under set -o pipefail: find returns non-zero on any traversal error, so a marker that was found still reads as absent).

Conservative-on-unknown-diff (non-regression safety)

When the caller supplies no changed-file list, the diff cannot be scoped, so the selector is conservative: with code in types it selects the full code roster (every language reviewer — kieran-typescript-reviewer, kieran-python-reviewer and php-reviewer — plus julik, accessibility, and design-implementation-reviewer) rather than under-select. This mirrors the cache's "over-invalidate on ambiguity" rule and guarantees the non-regression property: fed a historical review (whose file list we no longer have), the selector re-selects every code reviewer that review fired. In real use the orchestrator passes the actual changed files, so the diff is known and the specialists scope down — that is where the 6→3-4 median reduction comes from. Scoping is never silent: persona-select.sh logs when it narrows.

Deliverable-type reviewers (by type, independent of diff triggers)

Selected by the presence of the type in deliverable_types — never diff-dependent, because history cannot exercise a type it never saw (presentation and project-plan fired historically; only the type-based union can protect them).

Type Reviewer
document document-quality-reviewer
presentation presentation-quality-reviewer
research research-quality-reviewer
project-plan project-plan-reviewer

Bounded dispatch

The selected set is dispatched under a concurrency bound — see bounded-dispatch.md.