# Aspect Reviewers

Aspects are verified by reviewers. Yggdrasil ships three reviewer kinds — all operate on the same aspect-node-flow graph; the kind is inferred from which rule source file is present in the aspect directory.

- **LLM reviewer** (inferred when `content.md` is present): ships a `content.md` rule file. An LLM reads the rule and the node's source code, then accepts or rejects.
- **Deterministic reviewer** (inferred when `check.mjs` is present): ships a `check.mjs` module run by a deterministic runner at zero LLM cost. The check returns a `Violation[]` — no LLM, no nondeterminism, no per-call cost. There is one `check(ctx)` contract: the check receives the node, its subject files (each with a tree-sitter parse tree via `file.ast` when the language has a grammar), the file system, and the graph topology, and can use any of them — inspect a single file's parse tree for syntactic rules, or read related nodes and the file system for cross-node and structural rules. See `yg knowledge read writing-deterministic-aspects`.
- **Aggregating aspect** (inferred when neither rule source is present but `implies:` is declared): a content-less, check-less named bundle. It has no own reviewer and produces no own verdict. When effective on a node, it expands its `implies:` list and each implied aspect is verified individually. Use it to attach a multi-rule contract as one named entry point backed by N atomic child aspects.

The `reviewer:` block in `yg-aspect.yaml` is **optional** — kind is inferred automatically. If present, an explicit `reviewer.type` must agree with the inferred kind; `yg check` enforces this. A `content.md` and a `check.mjs` are mutually exclusive on the same aspect. An aspect with neither rule source and no `implies:` is rejected. Deterministic aspects run locally at zero LLM cost.

---

## Choosing a reviewer

| Reviewer | Use when the rule is… | Examples |
|---|---|---|
| **Deterministic** | a **programmatic** check — either a per-file **syntactic** rule or a **graph/file-system shape** rule spanning more than one file | Forbidden API calls (`fs.readFileSync`, `eval`); naming conventions (PascalCase exports); import restrictions (no cross-module relatives); missing guards (`@Log` decorator required); "every command node has a sibling test file"; "every child of an engine node is of type engine-component"; "every knowledge topic is registered in the index" |
| **LLM** | a **semantic judgment** a human reviewer would read surrounding context to make | "Mutations must emit audit events"; "Error responses must follow the API contract"; "Business logic must respect rounding rules"; "This handler must validate input semantically" |

If a programmatic check can decide the rule — a regex or AST traversal over a single file, or a graph-aware check spanning multiple files and the file system — use the deterministic reviewer. If a human reviewer would need to read surrounding context to decide, use LLM. The deterministic reviewer runs locally at zero LLM cost; only the LLM reviewer makes paid calls.

The `reviewer:` block is **optional** — reviewer kind is inferred from rule-file presence (`content.md` → LLM, `check.mjs` → deterministic, neither + `implies:` → aggregate). Declare a `reviewer:` block only when you need to set `reviewer.tier:` on an LLM aspect. If you do declare a `reviewer.type`, it must agree with the inferred kind. LLM aspects may declare `reviewer.tier:` to opt into a specific tier from `yg-config.yaml` — see [Reviewer tiers](./configuration.md#reviewer-tiers) for tier configuration.

---

## LLM reviewer

The LLM reviewer is a separate LLM call from the coding agent — one LLM verifying the work of another. `yg check --approve` assembles each unverified LLM pair into one prompt — the aspect's `content.md` plus the subject files for that pair (the whole node under `per: node`, a single file under `per: file`). The reviewer also receives any reference files declared on the aspect, presented as authoritative context (not under review). It responds with SATISFIED or NOT SATISFIED, and the verdict is recorded in the lock. Each unverified LLM pair costs one reviewer call, multiplied by the tier's consensus count.

**Draft aspects produce no pairs.** When an aspect's effective status on a node is `draft`, no pair is expected for it — there is nothing to verify and nothing to record. Aspects with effective status `advisory` or `enforced` are verified normally; the level only changes how a refused or unverified pair renders in `yg check` (warning vs. error). Verdicts survive status flips, including a `draft` round-trip — returning an aspect to enforced re-uses the recorded verdict for unchanged inputs. See [Aspect Status](/aspect-status) for the lifecycle.

**LLM verdicts are not deterministic.** The same code against the same rule can come back SATISFIED on one run and NOT SATISFIED on another — most often on borderline rules. To avoid laundering a refusal into an approval, a recorded refusal is final for unchanged inputs: re-running `yg check --approve` does not re-roll it. The three honest ways out are fix the code, sharpen the rule (which re-verifies every pair of the aspect — check `yg impact --aspect` first), or add a `yg-suppress` marker with your sign-off. Manage variance up front by writing rules that are concrete and decidable rather than vague, by preferring a `deterministic` `check.mjs` whenever a rule is programmatically checkable (zero LLM cost, identical result every run), and by raising `consensus` on high-stakes or noisy aspects so a majority vote smooths out single-call variance. To explore whether the rule text is the problem, use `yg aspect-test` — a diagnostic re-run that never writes the lock.

### Directory structure

```
.yggdrasil/aspects/
  requires-audit/
    yg-aspect.yaml       ← reviewer: { type: llm }
    content.md           ← the rule, in plain Markdown
```

### `yg-aspect.yaml`

```yaml
name: Audit Logging
description: "Every mutation must emit an audit event"
reviewer:
  type: llm
```

### Writing `content.md`

Write rules the way you would write a code review comment — clear, specific, actionable.

```markdown
<!-- .yggdrasil/aspects/requires-audit/content.md -->
Every public mutation endpoint must emit an audit event before returning.

Use the shared `auditLog.emit()` utility. Do not build custom audit logic.
The event must include: user ID, action, timestamp, affected resource ID.
```

**Effective `content.md` is specific, not aspirational:**

- ✅ "Use `auditLog.emit()` before return. Event must include userId, action, timestamp, resourceId."
- ❌ "Audit logging should be appropriate and comprehensive."

The reviewer compares text against code. Vague rules produce vague verdicts; specific rules produce reproducible verdicts.

### Output and false positives

```text
$ yg check --approve

  payments / requires-audit — REFUSED
    chargeCard() does not emit an audit event.
    No call to auditLog.emit() found in any mutation path.

Result: FAIL — fix the violation, then re-run: yg check --approve
```

If the reviewer rejects compliant code, the fix is improving the aspect's `content.md` — make the rule clearer and more specific. Sharpening the rule re-verifies every pair of the aspect. The escape hatch is better rules, not bypassing enforcement.

### Cost

Cost is counted per pair. A `per: node` aspect on a node with 5 source files is one pair — one LLM call (times consensus). A typical fill for a node with 3 `per: node` aspects makes 3 LLM calls. A `per: file` aspect over those 5 files is 5 pairs — 5 calls. Using a fast model (Haiku, GPT-4o-mini, Gemini Flash) keeps cost under a few cents per call. Deterministic pairs are free regardless of scope. For local review, Ollama runs on your machine with no API cost. See [Configuration](/configuration) for provider setup.

### Consensus

Set `consensus: 3` (or any odd number) on a tier in `yg-config.yaml` to run multiple review passes and take the majority vote. Higher confidence, proportionally higher cost. Useful for high-stakes aspects or noisy borderline rules.

```yaml
reviewer:
  tiers:
    thorough:
      provider: anthropic
      consensus: 3          # majority vote — 2 of 3 must agree
      config:
        model: claude-opus-4-7
```

---

## Deterministic reviewer

The deterministic reviewer ships a `check.mjs` module run locally at zero LLM cost. Whatever `Violation[]` your `check` function returns is the verdict — no LLM, no nondeterminism, no per-call cost. There is **one** `check(ctx)` contract. The `ctx` exposes everything a check might need: `ctx.node` and `ctx.files` (the subject files, each with a `file.ast` parse tree when the language has a grammar), `ctx.fs` (the file system, within an allowed-reads boundary), and `ctx.graph` (the graph topology). A check uses whichever it needs — inspect a single file's parse tree for a syntactic rule, or read related nodes and the file system for a cross-node structural rule. See `yg knowledge read writing-deterministic-aspects`.

The two subsections below illustrate each end of that range: [parse-tree checks](#parse-tree-checks) for per-file syntactic rules, and [graph-aware checks](#graph-aware-checks) for rules spanning more than one node — but both are the same `check.mjs` contract and the same `reviewer.type: deterministic` field.

### Parse-tree checks

A parse-tree check reads each subject file's tree-sitter parse tree (`file.ast`) and calls your `check(ctx)` function with it. Whatever `Violation[]` you return is the verdict.

#### Directory structure

```
.yggdrasil/aspects/
  async-fs/
    yg-aspect.yaml       ← reviewer: { type: deterministic }
    check.mjs            ← your check function
```

#### `yg-aspect.yaml`

```yaml
name: No Sync FS
description: Forbid synchronous fs calls — use async equivalents
reviewer:
  type: deterministic
```

The `reviewer:` block is optional — the presence of `check.mjs` infers the deterministic kind. If you do declare it, `reviewer.type: deterministic` must agree with the inferred kind. The runner parses each source file by extension — built-in grammars cover TypeScript/TSX/JavaScript, Python, Go, Rust, Java, C#, C, C++, PHP, Ruby, Kotlin, JSON, YAML, and TOML — and passes all files to a single `check.mjs` invocation; a file whose extension has no registered grammar is still passed to the check, just without a parse tree. Per-language file filtering (a `language:` array on the aspect) is designed but not yet built. Everything else (`implies`, `when`, `aspects` on nodes) works identically across both reviewer types.

#### Writing `check.mjs`

```javascript
// check.mjs — must export a named 'check' function (not default)
import { walk, report, inFile } from '@chrisdudek/yg/ast';

export function check(ctx) {
  const violations = [];
  // ctx.files — the node's source files, each with a tree-sitter parse tree
  // ... examine ctx.files ...
  return violations;   // return Violation[], synchronous
}
```

`ctx` has shape:

```typescript
interface CheckContext {
  files: SourceFile[];
}

interface SourceFile {
  path: string;       // relative to project root
  content: string;    // raw file content
  ast: Tree;          // tree-sitter parse tree
}

interface Violation {
  file: string;       // relative to project root
  line: number;       // 1-based
  column: number;     // 0-based
  message: string;
}
```

#### End-to-end example — async-fs

```javascript
// .yggdrasil/aspects/async-fs/check.mjs
import { walk, report } from '@chrisdudek/yg/ast';

export function check(ctx) {
  const violations = [];
  for (const file of ctx.files) {
    walk(file.ast.rootNode, node => {
      if (node.type !== 'call_expression') return;
      const fn = node.childForFieldName('function');
      if (fn?.type !== 'member_expression') return;
      const obj = fn.childForFieldName('object');
      const prop = fn.childForFieldName('property');
      if (obj?.text === 'fs' && /Sync$/.test(prop?.text ?? '')) {
        violations.push(report(file, node, `fs.${prop.text} is synchronous — use async equivalent`));
      }
    });
  }
  return violations;
}
```

This flags any call matching `fs.<anythingSync>()` anywhere in the files, across function boundaries.

#### Minimal API

Five exports are available from `@chrisdudek/yg/ast` — no installation required. If `yg` works on the machine, the import resolves at runtime.

```javascript
import { walk, report, inFile, findComments, closest } from '@chrisdudek/yg/ast';

walk(node, visitor)           // DFS traversal; visitor returning false skips subtree
report(file, node, message)   // create a Violation — line 1-based, column 0-based
inFile(file, { glob })        // path filter: { glob }, { regex }, or { contains }
inFile(file, { regex })
inFile(file, { contains })
findComments(target)          // comment nodes for a file or subtree
closest(node, types)          // nearest ancestor whose type is in the array
```

Use direct tree-sitter node properties for everything else:

```javascript
node.type                        // grammar node type string
node.text                        // raw source text
node.namedChildren               // named children array
node.childForFieldName('name')   // child at a named grammar field
node.startPosition               // { row, column } — zero-based
node.parent                      // parent node
```

Full type signatures are in the CLI's `dist/ast.d.ts`. Locally installed users get editor completion automatically; global/npx users get runtime resolution.

#### Worked example — before and after

**Rule:** no array-mutation methods called on function parameters.

**Without helpers** (~30 lines):

```javascript
const fnTypes = ['function_declaration', 'function_expression', 'arrow_function', 'method_definition'];
for (const fnType of fnTypes) {
  for (const fn of file.ast.rootNode.descendantsOfType(fnType)) {
    const params = new Set();
    for (const p of (fn.childForFieldName('parameters')?.namedChildren ?? [])) {
      const id = p.childForFieldName('pattern') ?? p.namedChildren[0];
      if (id?.type === 'identifier') params.add(id.text);
    }
    for (const call of fn.descendantsOfType('call_expression')) {
      const fn2 = call.childForFieldName('function');
      if (fn2?.type !== 'member_expression') continue;
      const obj = fn2.childForFieldName('object');
      const prop = fn2.childForFieldName('property');
      if (obj?.type === 'identifier' && params.has(obj.text) &&
          /^(push|splice|pop|sort|reverse)$/.test(prop?.text ?? '')) {
        violations.push({ file: file.path, line: call.startPosition.row + 1, message: '...' });
      }
    }
  }
}
```

**With minimal API** (~10 lines):

```javascript
import { walk, report } from '@chrisdudek/yg/ast';

walk(file.ast.rootNode, fn => {
  if (fn.type !== 'function_declaration') return;
  const params = collectParamNames(fn);
  walk(fn, callNode => {
    if (callNode.type !== 'call_expression') return;
    const fnNode = callNode.childForFieldName('function');
    if (fnNode?.type !== 'member_expression') return;
    const obj = fnNode.childForFieldName('object');
    const prop = fnNode.childForFieldName('property');
    if (obj?.type === 'identifier' && params.has(obj.text) &&
        /^(push|splice|pop|sort|reverse)$/.test(prop?.text ?? '')) {
      violations.push(report(file, callNode, `Mutating parameter via .${prop?.text}() is forbidden`));
    }
  });
});
```

#### Purity rule

`check.mjs` must be pure: **no file writes, no network calls, no `process.exit`**. The runner does not sandbox or enforce this; respecting it is your responsibility. Impure checks produce non-deterministic results and can corrupt the project.

#### Testing parse-tree checks

```bash
# Run an aspect against specific files (diagnostic only — the lock is never written)
yg aspect-test --aspect async-fs --files src/utils/config.ts

# Use a node's mapping as the file list
yg aspect-test --aspect async-fs --node orders/order-service
```

`yg aspect-test` exits 0 for clean, 1 for violations, and never writes the lock. Output:

```text
src/utils/config.ts
  L12: fs.readFileSync is synchronous — use async equivalent
```

### Graph-aware checks

A graph-aware check is language-agnostic. It is the same `check.mjs` contract, but instead of leaning on a single file's parse tree it reaches for `ctx.fs` and `ctx.graph` — reading the node, its files, related nodes, and the file system. Use it for cross-node structural rules a parse-tree check cannot express: "every command node has a sibling test file", "every child of an engine node is of type engine-component", "every knowledge topic is registered in the index". See `yg knowledge read writing-deterministic-aspects`.

#### Directory structure

```
.yggdrasil/aspects/
  sibling-test-file/
    yg-aspect.yaml       ← reviewer: { type: deterministic }
    check.mjs            ← your check function
```

#### `yg-aspect.yaml`

```yaml
name: sibling-test-file
description: "Every command node must have a sibling test file"
reviewer:
  type: deterministic
```

Graph-aware aspects do NOT declare a `language:` array — the runner invokes `check.mjs` once per affected node regardless of file types. Setting `reviewer.tier:` on a deterministic aspect is a validator error; tiers apply only to LLM aspects.

#### Writing `check.mjs`

The `check(ctx)` function is synchronous and returns `Violation[]`. The `ctx` object exposes the graph and the file system rather than a single parse tree:

```typescript
interface Ctx {
  node: GraphNode;     // the node being reviewed
  files: File[];       // alias for node.files — own files with child carve-out applied

  fs: {
    exists(path: string): 'file' | 'dir' | false;
    list(dir: string): { name: string; kind: 'file' | 'dir' }[];
    read(path: string): string;
  };

  graph: {
    node(id: string): GraphNode | undefined;
    nodesByType(type: string): GraphNode[];
    relationsFrom(node: GraphNode): Relation[];
    relationsTo(node: GraphNode): Relation[];
    children(node: GraphNode): GraphNode[];
    flowParticipants(flowName: string): GraphNode[];
  };

  // Synchronous — pre-warmed by the dispatcher. Do NOT await.
  parseAst(file: File | string, language: string): unknown;
  parseYaml(file: File | string): unknown;
  parseJson(file: File | string): unknown;
  parseToml(file: File | string): unknown;
}

interface Violation {
  message: string;
  file?: string;    // repo-relative POSIX path
  line?: number;    // 1-based
  column?: number;  // 0-based
}
```

The same helper exports available to parse-tree checks (`walk`, `report`, `inFile`, `closest`, `findComments`) are re-exported from `@chrisdudek/yg/structure` for checks that also inspect parsed trees via `ctx.parseAst`. Most graph-aware checks work purely with `ctx.graph` and `ctx.fs` without parsing any AST.

#### Allowed reads

The graph-aware runner enforces a strict read boundary — reading outside it throws a runtime violation instead of returning data. A node may read its own mapping files, its declared relation targets (and their descendants), its ancestor mappings, and its own descendant mappings. If a check needs to reach a node outside this set, add an explicit relation in `yg-node.yaml` pointing to it — relations are the contract that widens the allowed reads. The boundary is a *discipline*, not a sandbox: `check.mjs` runs with full Node privileges, so keep it machine-independent (no local-only paths, no OS quirks, no line-ending assumptions).

#### The observation model: what invalidates a deterministic verdict

A deterministic verdict is reusable only while everything the check **observed** still hashes to the value it had when the verdict was recorded. As the check runs, the runner records every observation it makes beyond the subject files: each `ctx.fs.read` (with the content hash), each `ctx.fs.list` (with a hash of the directory's entry names), each `ctx.fs.exists` probe (including negative ones — a `false` result is an observation), and each `ctx.graph` access. These observations are folded into the pair's input hash, so a later change to any observed value re-verifies the pair — adding a file the check listed, making a probed path appear, or editing a related node all count.

The practical consequence: **every observation widens your invalidation surface.** Read and probe only what the rule needs, and the verdict survives longer between re-runs.

#### Testing graph-aware checks

```bash
# Test the check against a specific node — diagnostic only, the lock is never written
yg aspect-test --aspect sibling-test-file --node orders/order-service

# Verify the check is deterministic (same violations on every run)
yg aspect-test --aspect sibling-test-file --node orders/order-service --check-determinism
```

`yg aspect-test` exits 1 if violations exist and never writes the lock. Run it against both compliant and non-compliant nodes to confirm no false positives and no false negatives. `--check-determinism` runs the check twice and fails if the violation sets differ — your safeguard against machine-dependent or side-effecting checks.

---

## Suppression — shared across reviewer types

Source code comments can carry a `yg-suppress` marker to waive a specific aspect. All reviewer types honor the same syntax; they differ only in how scope is interpreted.

**Format:** `yg-suppress(<aspect-path>) <reason>`

- `<aspect-path>` — full aspect path (e.g., `cqrs/single-responsibility`)
- `<reason>` — required free-text explanation. Empty or whitespace-only reasons fail with `SUPPRESS_MARKER_MISSING_REASON`.
- Markers must live inside **comment nodes** — string literals are not matched.

### Single-line suppress

```typescript
// yg-suppress(async-fs) legacy code, refactor tracked in JIRA-456
const data = fs.readFileSync(path, 'utf-8');
```

- **Deterministic reviewer:** applies to the **immediately following line**. Deterministic, no scope inference.
- **LLM reviewer:** the reviewer interprets scope **contextually** — a marker inside a function applies to that function, at file top it applies to the whole file.

### Bracket suppress

```typescript
// yg-suppress-disable(async-fs) bootstrap block uses sync reads
const a = fs.readFileSync('a.json', 'utf-8');
const b = fs.readFileSync('b.json', 'utf-8');
// yg-suppress-enable(async-fs)
```

Applies to all lines between `disable` and `enable`. Reason is required on `disable`. Without a closing `enable`, the range extends to end of file. Block comments work too.

### Wildcard

Use `*` to suppress all aspects in a range:

```typescript
// yg-suppress-disable(*) generated block — do not edit
/* ... generated code ... */
// yg-suppress-enable(*)
```

A specific `enable(<id>)` does **not** punch through `disable(*)`. To re-enable a specific aspect inside a wildcard-disabled block, end the wildcard block first.

### Agent behavior

Agents may propose adding a suppress marker but must **never** write one without explicit user confirmation. The reason field is provided or approved by the human, never invented by the agent.

---

## Verdicts and the lock — shared

Both reviewer types record their results the same way: one content-addressed entry per `(aspect, unit)` pair in the committed `.yggdrasil/yg-lock.json`. Each entry stores the verdict and the hash of the inputs that produced it:

- **LLM pair:** the hash folds `content.md`, the subject files, the aspect description, the reference files, and the resolved tier identity. Change any of them → the pair is unverified.
- **Deterministic pair:** the hash folds `check.mjs`, the subject files, and the observation set — every `ctx.fs` read, listing, and existence probe and every `ctx.graph` access the check made beyond its subject files (see [the observation model](#the-observation-model-what-invalidates-a-deterministic-verdict) below). Change the check, a subject file, or any observed value → the pair is unverified.

`yg check` recomputes each pair's input hash and compares it to the lock — no LLM calls, no provider keys, runs instantly. A source edit and an aspect-content edit both surface the same way: the affected pairs no longer match their recorded hash, so check reports them as unverified until `yg check --approve` fills them again.

---

## Edge cases

### LLM reviewer

**Borderline rejections.** Compliant code can be rejected by an LLM that misread the rule. Fix: clarify `content.md`. The escape hatch is better rules, not `yg-suppress`. A recorded refusal is final for unchanged inputs — sharpening the rule is the way to overturn it (and it re-verifies every pair of the aspect).

**Cost spikes when an aspect changes.** Editing a widely-used aspect's content invalidates every pair it produces → N LLM calls to refill. Before such an edit, run `yg impact --aspect <id>` to see the count. Consider `consensus: 1` for high-fan-out aspects.

### Deterministic reviewer

**Imports inside `check.mjs`.** Yggdrasil hashes only `check.mjs` itself. If your check imports a helper from `node_modules`, changes to that helper do **not** invalidate the pair — Yggdrasil does not know about transitive dependencies. Guidance: keep all rule logic inside `check.mjs`. If you import a helper, consciously accept that bumping the helper version does not re-verify on its own.

**CLI version pinning.** Tree-sitter grammar versions and helper implementations live inside `@chrisdudek/yg`. A CLI upgrade can shift parse-tree node shapes or helper behavior. The input hash deliberately excludes the CLI version, so upgrading Yggdrasil does not invalidate verdicts. After a CLI upgrade that changes tree-sitter grammar behavior (announced in CHANGELOG), force a re-verification by touching the rule source, or accept that the recorded verdicts stand until their inputs change.

---

## See also

- [Core Concepts](/core-concepts) — nodes, aspects, the lock, and pairs
- [CLI Reference](/cli-reference) — `yg check --approve`, `yg aspect-test`, `yg aspects`
- [Configuration](/configuration) — reviewer provider setup
- [Conditional Aspects](/conditional-aspects) — `when` predicates for selective aspect application
