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 = floor ∪ diff-triggered code reviewers ∪ deliverable-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 code ∈ deliverable_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-reviewerwhenangular.jsonis inPROJECT_ROOT,nestjs-reviewerwhennest-cli.jsonis inPROJECT_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 acomposer.jsonrepo 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 0× 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.