Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apps/claude-code/unic-pr-review/CONTEXT.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ One specialised sub-agent lens applied to the whole diff — e.g. `code-reviewer
_Avoid_: dimension, pass, check

**Spawn Set**:
The `Set<string>` of Review Aspect agent names returned by `decideSpawnSet()` in `scripts/lib/changed-file-analyser.mjs`. Computed once before any agent runs, based on changed-file categories (ADR-0008).
The `Set<string>` of Review Aspect agent names returned by `decideSpawnSet()` in `scripts/lib/changed-file-analyser.mjs`. Computed once before any agent runs, based on changed-file categories (ADR-0008); additionally content-aware for the three semantic gates (`comment-analyzer`, `silent-failure-hunter`, `type-design-analyzer`) via deterministic diff-content sampling (ADR-0008 amendment, 2026-06).
_Avoid_: agent list, run set, active agents

## Relationships
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# 0008. Conditional sub-agent spawning over per-file chunking

**Status:** Accepted (2026-05)
**Status:** Accepted (2026-05); spawn-gating amended to Y-det hybrid for semantic gates (2026-06, issue #212)

## Context

Expand All @@ -13,21 +13,41 @@ Two alternatives were considered:

## Decision

The Plugin reviews a PR by fanning out to specialised Review Aspect sub-agents in parallel, each handling the whole diff under one lens. Spawning is conditional on what the diff contains — `code-reviewer` always runs; `pr-test-analyzer` runs only when test files changed; `silent-failure-hunter` runs when at least one non-test source file changed (a path/extension heuristic, not diff-content inspection); etc. The Plugin does NOT split the diff into per-file chunks.
The Plugin reviews a PR by fanning out to specialised Review Aspect sub-agents in parallel, each handling the whole diff under one lens. Spawning is conditional on what the diff contains — `code-reviewer` always runs; `pr-test-analyzer` runs only when test files changed; `silent-failure-hunter` runs when at least one non-test source file changed (a path/extension heuristic, not diff-content inspection — see amendment 2026-06 for the three semantic gates); etc. The Plugin does NOT split the diff into per-file chunks.

## Consequences

- `scripts/lib/changed-file-analyser.mjs` classifies the diff once and returns the Spawn Set before any agent runs.
- The Intent Checker is the one exception — it always runs first (regardless of file types) because its output seeds every other agent's context. Its result is broadcast to every spawned aspect agent.
- Adding a new Review Aspect is additive: define the agent in `agents/<name>.md`, add one entry to `SPAWN_TABLE` in `changed-file-analyser.mjs` — no orchestration rewrite needed.

### Spawn Table (as of PR #158)
### Spawn Table (as of PR #158; rows marked † amended by 2026-06 content-gating — see amendment section below)

| Agent | Spawn condition |
| ----------------------- | --------------------------------------------------------------------------------------------------- |
| `code-reviewer` | Always — any non-empty diff |
| `silent-failure-hunter` | At least one non-test source file (`.mjs`, `.cjs`, `.js`, `.ts`, `.tsx`, `.jsx`; excluding `.d.ts`) |
| `type-design-analyzer` | At least one `.d.ts`, `.ts`, `.tsx`, or file under `types/`, `schemas/`, `interfaces/` |
| `pr-test-analyzer` | At least one test file (`.test.*`, `.spec.*`, or under `tests/`, `__tests__/`) |
| `comment-analyzer` | At least one `.md` / `.mdx` file or file under `docs/` |
| `code-simplifier` | Three or more non-test source files (excluding `.d.ts`) |
| Agent | Spawn condition |
| ----------------------- | ----------------------------------------------------------------------------------------------------- |
| `code-reviewer` | Always — any non-empty diff |
| `silent-failure-hunter` | † At least one non-test source file (`.mjs`, `.cjs`, `.js`, `.ts`, `.tsx`, `.jsx`; excluding `.d.ts`) |
| `type-design-analyzer` | † At least one `.d.ts`, `.ts`, `.tsx`, or file under `types/`, `schemas/`, `interfaces/` |
| `pr-test-analyzer` | At least one test file (`.test.*`, `.spec.*`, or under `tests/`, `__tests__/`) |
| `comment-analyzer` | † At least one `.md` / `.mdx` file or file under `docs/` |
| `code-simplifier` | Three or more non-test source files (excluding `.d.ts`) |
Comment on lines +29 to +33

† Path classification is the fast path; additionally gated by deterministic diff-content sampling per the amendment below.

## Content-gated spawning for semantic gates (amended 2026-06, issue #212)

### Context for the amendment

A direct comparison of unic-pr-review v2.1.2 against the `pr-review-toolkit` on PR #5612 revealed that path/extension classification alone causes spawn divergences on three gates: the `comment-analyzer` missed findings when documentation comments were added inside source files (not under `docs/`); the `silent-failure-hunter` over-fired on PRs with no error-handling changes; the `type-design-analyzer` over-fired on PRs that touched `.ts` files but introduced no new types. The toolkit uses semantic (model-read) gate conditions.

Adopting model-assisted gating wholesale (Y-llm) was considered and rejected — it would break the unit-testable, pure-function classifier that is the foundation of the deterministic spawn decision.

### Decision (amendment)

For the three semantic gates — **comment-analyzer** (comments gate), **silent-failure-hunter** (errors gate), and **type-design-analyzer** (types gate) — the classifier additionally reads a small content sample from the diff using **deterministic parsing** (**Y-det**): pure functions over `+`/`-` diff hunks, unit-testable with fixtures, no model calls. Path classification remains the fast path and continues to gate `pr-test-analyzer` and `code-simplifier` without change.

Semantic gates are **biased toward spawning on ambiguity**: a false-positive spawn produces an empty result block (cheap); a false-negative miss silently omits a potentially valid finding set (as seen in the #5612 comparison). When the classifier cannot determine confidently that a gate's semantic condition is absent, it spawns.

### Y-llm promotion caveat

The **errors gate** (`silent-failure-hunter`) is the weakest of the three as a deterministic predicate — "error handling changed" is genuinely semantic and the hardest to capture reliably with regex over diff hunks. It is the **first candidate to promote to Y-llm** (model-assisted gate classification) if Y-det proves too noisy or too blind for that gate in practice.
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ What makes a good test for this Plugin: tests assert observable external behavio
- **Approval Loop** — Finding-state-machine transitions (accept / edit / skip), non-TTY abort, `--yes` bulk-accept, resumability after interruption, head-SHA mismatch prompt on resume, `<cwd>/.unic-pr-review/<key>/` write + `.gitignore` self-ignore, cleanup on Writer success. Tested by piping scripted stdin and asserting output JSON + state-file contents.
- **Mode detector** — input `{ hasUrl, hasPriorSignature, revisionsAvailable }` → mode literal (`pre-pr` / `first-review` / `re-review` / `first-review-fallback`). Pure-function table-driven.
- **Severity bucketer** — input confidence integer → severity bucket name (and the `<60` drop). Pure-function.
- **Changed-file analyser** — input changed-files list + a small content sample → set of aspect agents to spawn. Pure-function table-driven.
- **Changed-file analyser** — input changed-files list + a small content sample (content-aware for the comment, errors, and types semantic gates via deterministic diff-parsing; path classification is the fast path for all gates) → set of aspect agents to spawn. Pure-function table-driven.
- **Renderers** (`review-summary-renderer.mjs`, `inline-comment-renderer.mjs`) — input structured Findings / Coordinator plan → exact markdown blobs including the Bot Signature footer drawn from `signature.mjs`. Snapshot-style fixtures.
- **Setup Wizards** — file writing under `chmod 600` (where supported), idempotent re-runs, env-var override precedence. Tested against a temp HOME. Windows path branches assert "warn but don't fail".
- **Doctor** — preflight predicates (`az` present, extension present, `az devops login` valid, Confluence ping, Jira ping when configured). Tested with each predicate stubbed.
Expand All @@ -226,6 +226,6 @@ What makes a good test for this Plugin: tests assert observable external behavio
## Further Notes

- The Plugin retires the existing `apps/claude-code/pr-review` Plugin after acceptance testing. As of 2026-05 both coexist in the monorepo as **fully independent** Plugins — `unic-pr-review` shares no code, no prompts, no fixtures, and no soft dependency with `pr-review`. The maintainer will delete `pr-review` once `unic-pr-review` proves itself on the two target projects (one ADO, one Jira).
- The six aspect agents are written from scratch in this Plugin's own voice with our Confidence-Score rubric embedded verbatim. The conditional-spawning pattern is loosely inspired by Anthropic's `pr-review-toolkit`, but we take no soft dependency and copy no prompts.
- The six code-analysis aspect agents maintain a **behavioural-parity contract** with Anthropic's `pr-review-toolkit`: their spawn-gating conditions and analysis behaviour approximate the toolkit's semantic patterns for the comment, error-handling, and types aspects. The clean-slate doctrine holds — no code, no prompts, and no fixtures are taken from `apps/claude-code/pr-review/`; parity is by independent derivation against this PRD and the ADRs (see ADR-0008 amendment, issue #212).
- The root `CONTEXT-MAP.md` must be updated to add this Plugin's `CONTEXT.md` — that update is tracked by issue #143 (plugin scaffold), which also creates the `CONTEXT.md` file itself.
- The Plugin's `plugin.json` keywords include `pr-review`, `azure-devops`, `jira`, `confluence`, `code-review`, `unic`.
Loading