fix(review): the coordinator and orchestrator texts say what the code does on refutation and the brief-conflict exception
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
878871a6ad
commit
738efa3e24
6 changed files with 70 additions and 7 deletions
|
|
@ -536,7 +536,7 @@ Declined to Judge list, and the coordinator carries those lines into
|
|||
|------|---------|-------------|
|
||||
| **Default** | `/trekreview --project <dir>` | Claude Code `/code-review` + `/security-review`, then brief-conformance + code-correctness reviewers in parallel, coordinator dedup + verdict, write `{dir}/review.md`, verdict gate |
|
||||
| **Since ref** | `/trekreview --project <dir> --since <ref>` | Override "before" SHA for the diff range. Validated via `git rev-parse --verify` |
|
||||
| **Quick** | `/trekreview --project <dir> --quick` | `/code-review` + code-correctness reviewer only; skip brief-conformance reviewer, `/security-review` and the coordinator's reasonableness filter — fast correctness-only pass |
|
||||
| **Quick** | `/trekreview --project <dir> --quick` | `/code-review` + code-correctness reviewer only; skip brief-conformance reviewer, `/security-review` and the coordinator's reasonableness filter except the brief-conflict rule — fast correctness-only pass |
|
||||
| **Validate** | `/trekreview --project <dir> --validate` | Schema-only check on existing `review.md`. No LLM calls |
|
||||
| **Dry run** | `/trekreview --project <dir> --dry-run` | Print discovered scope + triage map; skip writes |
|
||||
| **Profile** | `/trekreview --profile <name> --project <dir>` | (v4.1.0) Pin model profile for the review phase. See [Profile system](#profile-system-v410). |
|
||||
|
|
|
|||
|
|
@ -136,8 +136,9 @@ Succinctness and Actionability read the finding's *packaging*; neither
|
|||
examines the claim, so neither can establish the finding is unreal. Accuracy
|
||||
does: a citation that escapes the repo root, or a negative line, refutes the
|
||||
finding as a claim about this codebase. A line past the end of the file does
|
||||
not: the citation is wrong, the claim is unchecked. A line number alone never
|
||||
refutes a finding, so it goes to `unverified` (`accuracy:line-out-of-range`).
|
||||
not: the citation is wrong, the claim is unchecked. Any other line number alone
|
||||
never refutes the finding, so a line past the end of the file sends it to
|
||||
`unverified` (`accuracy:line-out-of-range`).
|
||||
|
||||
**No finding is struck for length** (ordre 5). Before review_version 1.1 an
|
||||
over-long `detail` sent a finding to `unverified`, and S118's most serious
|
||||
|
|
|
|||
|
|
@ -54,8 +54,8 @@ You will receive a prompt containing:
|
|||
contract that bounds review scope.
|
||||
- **Mode** — `default`, `quick`, `validate`, or `dry-run`.
|
||||
- `default` — run the full pipeline.
|
||||
- `quick` — skip the coordinator's reasonableness filter; use single
|
||||
reviewer (code-correctness only) for faster turnaround.
|
||||
- `quick` — skip the coordinator's reasonableness filter except the
|
||||
brief-conflict rule; use single reviewer (code-correctness only) for faster turnaround.
|
||||
- `validate` — schema-only check on existing review.md, no LLM calls.
|
||||
- `dry-run` — print the discovered scope and triage map; skip writes.
|
||||
- **Since-ref** (optional) — explicit `--since <ref>` override for the SHA
|
||||
|
|
@ -191,6 +191,9 @@ The coordinator runs a 4-pass process:
|
|||
`RULE_CATALOGUE` (`unknown-rule_key`) or whose file cannot be checked
|
||||
(`file-existence:indeterminate`) to `unverified`, which forbids `ALLOW`;
|
||||
reset a severity that does not match its catalogue tier.
|
||||
The one exception to the reset is the brief-conflict lowering: a
|
||||
`/code-review` finding marked `contradicts_brief`, at the MAJOR tier and
|
||||
at severity MAJOR, drops to MINOR with `brief_lowered_from: MAJOR`.
|
||||
Skipped at high effort, as in `quick` mode (below); at high effort an
|
||||
unknown `rule_key` is normalized to `PLAN_EXECUTE_DRIFT` instead
|
||||
(`agents/review-coordinator.md`, "High-effort normalization"); the
|
||||
|
|
|
|||
|
|
@ -62,7 +62,7 @@ FLAG_SCHEMA `trekreview` entry):
|
|||
|------|------|---------|
|
||||
| `--project <dir>` | valued | Required. Path to trekplan project folder containing `brief.md`. |
|
||||
| `--since <ref>` | valued | Optional. Override "before" SHA for the diff. Validated via `git rev-parse --verify`. |
|
||||
| `--quick` | boolean | Skip the brief-conformance pass; run only the code-correctness reviewer; skip the coordinator's reasonableness filter. |
|
||||
| `--quick` | boolean | Skip the brief-conformance pass; run only the code-correctness reviewer; skip the coordinator's reasonableness filter except the brief-conflict rule. |
|
||||
| `--validate` | boolean | Schema-only check on existing `{project_dir}/review.md`. No LLM calls. |
|
||||
| `--dry-run` | boolean | Print the discovered scope and triage map. Skip writes. |
|
||||
| `--fg` | boolean | No-op alias (foreground is default). |
|
||||
|
|
|
|||
|
|
@ -92,7 +92,7 @@ Every mode that executes refuses a plan whose approval in the approval log does
|
|||
| _(default)_ | Claude Code `/code-review` + `/security-review` (engine, via the Skill tool, never `--fix`), then brief-conformance + code-correctness reviewers in parallel, coordinator dedup + verdict, write `{project_dir}/review.md` with the required `verdict:`, decide with `lib/review/verdict-gate.mjs` (0 ALLOW/WARN · 1 BLOCK · 2 never green), propose recurring finding classes as project memory (operator approves each line) |
|
||||
| `--project <dir>` | **Required.** Path to trekplan project folder containing `brief.md`. Review is written to `{dir}/review.md` |
|
||||
| `--since <ref>` | Override "before" SHA for the diff range. Validated via `git rev-parse --verify` |
|
||||
| `--quick` | `/code-review` + code-correctness reviewer only; skip brief-conformance reviewer, `/security-review` and the coordinator's reasonableness filter — fast correctness-only pass |
|
||||
| `--quick` | `/code-review` + code-correctness reviewer only; skip brief-conformance reviewer, `/security-review` and the coordinator's reasonableness filter except the brief-conflict rule — fast correctness-only pass |
|
||||
| `--validate` | Schema-only check on existing `{dir}/review.md` plus the verdict gate. No LLM calls. A 1.0 review without `verdict:` fails here (it still seeds a plan in soft mode) |
|
||||
| `--dry-run` | Print discovered scope + triage map; skip writes |
|
||||
| `--fg` | No-op alias (foreground is default) |
|
||||
|
|
|
|||
|
|
@ -15,6 +15,14 @@
|
|||
// no longer say "still untracked".
|
||||
// M3 The "Back to planning" save line runs its git diff with --no-relative,
|
||||
// so a session in a subdirectory with diff.relative on saves every file.
|
||||
// M9 The review coordinator's Pass 2 text says what the lib does: a root
|
||||
// escape or a negative line refutes a finding; any other line number
|
||||
// alone never does, and a line past the end goes to `unverified`.
|
||||
// M2 The four flag-table summaries of `--quick` (trekreview, orchestrator,
|
||||
// command-modes, README) name the brief-conflict rule as the exception
|
||||
// to the Pass 3 skip.
|
||||
// K3 The orchestrator's 4-pass item 3 names the brief-conflict lowering as
|
||||
// the exception to the catalogue reset.
|
||||
// No static import of a new export: the file must load at be0b631.
|
||||
// Fix the SOURCE a pin guards, not the pin.
|
||||
|
||||
|
|
@ -143,3 +151,54 @@ test('[M3] the back-to-planning save line carries --no-relative', () => {
|
|||
assert.match(m[1], /^git (?:-c \S+ )*diff (?:--\S+ )*--no-relative (?:--\S+ )*\{start_sha\}$/,
|
||||
`the save line's git diff does not carry --no-relative: ${m[1]}`);
|
||||
});
|
||||
|
||||
// --- M9 ----------------------------------------------------------------------------
|
||||
|
||||
test('[M9] the coordinator text does not say a line number alone never refutes', () => {
|
||||
const coord = flat(read('agents/review-coordinator.md'));
|
||||
assert.doesNotMatch(coord, /\bA line number alone never refutes/,
|
||||
'agents/review-coordinator.md still says a line number alone never refutes, next to a negative line that does');
|
||||
const p2 = between('agents/review-coordinator.md', '### Pass 2 — HubSpot Judge filters', '### Pass 3 — ');
|
||||
assert.ok(sentencesWith(p2, 'a citation that escapes the repo root, or a negative line, refutes the finding').length > 0,
|
||||
'Pass 2 no longer says a root escape or a negative line refutes the finding');
|
||||
const hits = sentencesWith(p2, 'line number alone never refutes');
|
||||
assert.ok(hits.some((s) => /\b[Aa]ny other line number alone never refutes\b/.test(s)
|
||||
&& /`unverified` \(`accuracy:line-out-of-range`\)/.test(s)),
|
||||
`no Pass 2 sentence says any other line number alone never refutes and goes to \`unverified\`: ${hits.join(' | ')}`);
|
||||
});
|
||||
|
||||
// --- M2 ----------------------------------------------------------------------------
|
||||
|
||||
/** The one line of `rel` that starts with `prefix` and holds `needle`. */
|
||||
function oneLine(rel, prefix, needle) {
|
||||
const hits = read(rel).split('\n').filter((l) => l.startsWith(prefix) && l.includes(needle));
|
||||
assert.equal(hits.length, 1, `${rel} must hold exactly one line starting "${prefix}" with "${needle}"; found ${hits.length}`);
|
||||
return hits[0];
|
||||
}
|
||||
|
||||
const BRIEF_CONFLICT_SKIP = /the coordinator's reasonableness filter,? except the brief-conflict rule\b/;
|
||||
|
||||
test('[M2] each of the four flag-table summaries names the brief-conflict exception', () => {
|
||||
const summaries = [
|
||||
['commands/trekreview.md --quick row', oneLine('commands/trekreview.md', '| `--quick` |', 'reasonableness filter')],
|
||||
['agents/review-orchestrator.md quick mode', flat(between('agents/review-orchestrator.md', '- `quick` — ', '- `validate` — '))],
|
||||
['docs/command-modes.md --quick row', oneLine('docs/command-modes.md', '| `--quick` |', 'reasonableness filter')],
|
||||
['README.md Quick row', oneLine('README.md', '| **Quick** | `/trekreview ', 'reasonableness filter')],
|
||||
];
|
||||
for (const [where, text] of summaries) {
|
||||
assert.match(text, BRIEF_CONFLICT_SKIP, `${where} skips the reasonableness filter without the brief-conflict exception: ${text}`);
|
||||
}
|
||||
});
|
||||
|
||||
// --- K3 ----------------------------------------------------------------------------
|
||||
|
||||
test('[K3] orchestrator item 3 names the brief-conflict lowering as the exception to the reset', () => {
|
||||
const item = flat(between('agents/review-orchestrator.md', '3. **Cloudflare reasonableness**', '4. **Compute verdict**'));
|
||||
const at = item.indexOf('reset a severity that does not match its catalogue tier.');
|
||||
assert.ok(at !== -1, `item 3 no longer resets a severity to its catalogue tier: ${item}`);
|
||||
const next = sentencesWith(item.slice(at + 'reset a severity that does not match its catalogue tier.'.length), ' ')[0] ?? '';
|
||||
assert.match(next, /\bexception to (the|this) reset\b/, `the sentence after the reset does not name an exception to it: ${next}`);
|
||||
assert.match(next, /brief-conflict lowering/, `the exception to the reset is not the brief-conflict lowering: ${next}`);
|
||||
assert.match(next, /MAJOR[^.]*\bMINOR\b/, `the exception does not say a MAJOR drops to MINOR: ${next}`);
|
||||
assert.match(next, /`brief_lowered_from: MAJOR`/, `the exception does not name brief_lowered_from: MAJOR: ${next}`);
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue