Skip to content
Merged
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
38 changes: 33 additions & 5 deletions CONTEXT.md
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,13 @@ _Avoid_: run, invocation, job
A named set of **Reviewer**s, with how many instances of each to run and whether findings are
validated. One panel runs per review. Which one is a **Context**'s default, possibly raised by
an **Escalation**.

It may also name the **Judge** and the **Validator** that answer for it, instead of the ones the
**Review manifest** declares. A panel is how one context's reviewers are chosen, so it is where
the runs that reconcile them belong: the judge runs in every review, and without this a repo
reviewing locally with one **Provider** and its pull requests with another could say so for its
reviewers and not for its judge. Both are overrides — a panel naming neither uses the
manifest's, so declaring them on one panel is never the price of declaring them on all.
_Avoid_: profile, preset, tier

**Context**:
Expand Down Expand Up @@ -176,6 +183,20 @@ detecting one is language knowledge that has to be tested somewhere other than a
YAML. A repo names paths instead.
_Avoid_: heuristic, marker, flag

**Exclusion**:
A changed file no **Reviewer** is shown, and which counts toward nothing a rule measures. Most
are mechanical — the file changed, but nobody wrote the change: a lockfile, a vendored tree, a
generated file, a binary, a pure rename, a symlink. That vocabulary is closed and ships with
the binary, for the reason **Signal**'s is.

A repo adds its own as globs, for the one thing detection cannot reach: source a person wrote
that is not worth a review's budget. It only ever removes files, which is the opposite
direction from an **Escalation**, so it is declared in the **Review manifest** rather than
given on a command line — committed where anyone can read it, and read from the base ref so a
branch cannot exclude itself. A repo's own reason is reported ahead of a mechanical one,
because only it points at a line somebody can edit.
_Avoid_: ignore, skip, filter, exemption

**Finding**:
One issue a **Reviewer** reports: a file, a line range, a severity, and a body. The unit
**Judge**ment is applied to and the unit that becomes an inline comment.
Expand Down Expand Up @@ -378,14 +399,21 @@ _Avoid_: summary, header, footer, marker (bare)

**Review manifest**:
`.agents/code-review/manifest.yaml`: the single declaration of **Reviewer**s, **Panel**s and the
prompt bodies they use. Read by both engines — the in-session skill and `agtk code-review` — so
there is one roster and not two.
prompt bodies they use. Read by `agtk code-review` and by nothing else. A roster a skill also
carried would be a second one, and the two would disagree the first time either changed.
_Avoid_: panels.json, roster file, review config

## Flagged ambiguities
**"Marker"** — the bare noun is a **Signal** synonym to avoid; the HTML comment that carries a
**Fingerprint** is a **Fingerprint marker**, always both words.

**"Exclusion" vs "Suppression"** — both withhold, and they withhold different things at
different ends of a run. An **Exclusion** is about a *file*, decided before any reviewer runs:
the file is never shown, so no **Finding** about it exists. A **Suppression** is about a
*finding* that was made, withheld from a **Review** because a **Comment thread** already
carries its **Fingerprint**. An excluded file produces nothing to suppress, and a suppressed
finding came from a file that was reviewed.

**"False positive" vs "Suppression"** — both withhold something, and they are opposite acts.
**Suppression** is `agtk`'s and mechanical: a **Finding** is not posted again because a thread
already carries it. A **False positive** is a person's and is about the claim itself: the
Expand All @@ -396,9 +424,9 @@ finding is posted, stays posted, and is declared not to be a defect. Suppression
has not yet passed a **Validator**. The memory sense owns the bare noun; in review, say
"candidate finding" and never "candidate" alone.

**"Panel"** — `deep-code-review` used it for a per-stack group of reviewers *within* one run,
so a polyglot change had several. A **Panel** here is the entire roster for a runone runs,
named by a **Context**'s default and possibly raised by an **Escalation**. The per-stack sense
**"Panel"** — reads as a per-stack group of reviewers *within* one run, so that a polyglot
change would have several. A **Panel** is the entire roster for a run, and exactly one runs:
the one a **Context** defaults to, possibly raised by an **Escalation**. The per-stack sense
has no name because per-stack partitioning is not built.

**"Review"** — the activity and the artifact. **Review** is the artifact posted to the PR; say
Expand Down
3 changes: 3 additions & 0 deletions definitions/CONFIG-SCHEMA.md
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ The location is fixed rather than configurable: the manifest is configuration, a
| `escalate` | `[]Escalation` | no | Rules that raise the panel above a context's default. Every rule is evaluated and the highest target wins, so their order carries no meaning. |
| `approval` | `Approval` | no | What approving a reviewed head requires of a finding's severity. Absent means the default floor, AMBER. |
| `conventions` | `[]string` | no | Documents holding this repo's own written rules, as paths from the repo root, read at the base ref and injected raw into every reviewer's prompt. Replaces the default list rather than adding to it. Absent means the defaults: CLAUDE.md, AGENTS.md, .claude/CLAUDE.md, CONTEXT.md, CONTRIBUTING.md, docs/ARCHITECTURE.md, docs/CODE_STANDARDS.md. |
| `exclude` | `[]string` | no | Paths this repo does not want reviewed, as globs (** spans path segments, * and ? stay within one). Added to the built-in exclusions — lockfiles, vendored trees and generated files are already excluded and need no entry. An excluded file is reported with its reason and reaches no reviewer. |

### `reviewers` entry, `judge`, `validator` (`Runner`)

Expand All @@ -116,6 +117,8 @@ One configured model invocation. It says which CLI, which model and which prompt
| `reviewers` | `[]string` | **yes** | Names from the manifest's reviewers map. A panel that names one this manifest does not declare cannot staff itself, and is refused. |
| `quorum` | `int` | no | How many independent instances of each reviewer to run. Agreement between them is the confidence signal. Defaults to 1. |
| `validate` | `bool` | no | Whether findings are put to the validator. Unset leaves it to the context, and a context that posts validates regardless: a false finding on a PR is published and blocks approval. |
| `judge` | `Runner` | no | Judge for reviews this panel produces, instead of the manifest's. Unset uses the manifest's. |
| `validator` | `Runner` | no | Validator for reviews this panel produces, instead of the manifest's. Unset uses the manifest's. |

### `defaults`

Expand Down
22 changes: 15 additions & 7 deletions definitions/settings/skill-permissions.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: skill-permissions
description: Pre-approve the file and shell operations the bundled skills/agents need (memory-explorer store reads and candidate writes, backlog reporting, Serena memories, deep-code-review scripts, pr-code-review read-only gh calls) so they run without permission prompts.
description: Pre-approve the file and shell operations the bundled skills/agents need (memory-explorer store reads and candidate writes, backlog reporting, Serena memories, panel-code-review's model-free code-review commands, read-only gh calls) so they run without permission prompts.
platforms: [claude]
value:
permissions:
Expand All @@ -15,10 +15,18 @@ value:
# pre-approval, so the only cost of a miss is a prompt, never a block.
- "Read(**/.agents/memory/INDEX.md)"
- "Write(**/.agents/memory/candidates/**)"
- "Bash(git checkout *)"
- "Bash(*/skills/deep-code-review/scripts/detect-parent.sh*)"
- "Bash(*/skills/deep-code-review/scripts/capture-diff.sh*)"
- "Bash(*/skills/deep-code-review/scripts/list-changed.sh*)"
- "Bash(*/skills/deep-code-review/scripts/find-convention-docs.sh*)"
# The code-review subcommands that start no model. `run` is absent
# because it costs money, and `approve` because a person types that one.
#
# A prefix rule cannot exclude one flag, so `explain*` grants
# `explain --pr` too — which reads the pull request over the API and
# fetches its head, under this machine's App registration. That is
# deliberate rather than overlooked: it is the command the skill runs on
# every pull-request review, it starts no model and spends nothing, and
# the expensive step it precedes is `run --pr`, which is not pre-approved
# and asks. A grant whose comment claimed these touch no network would be
# the real hazard.
- "Bash(agtk code-review panels*)"
- "Bash(agtk code-review explain*)"
- "Bash(agtk code-review signals*)"
- "Bash(gh pr view *)"
- "Bash(gh pr diff *)"
136 changes: 0 additions & 136 deletions definitions/skills/deep-code-review/SKILL.md

This file was deleted.

Loading
Loading