Skip to content

chore: Add just review-file for context-only reviews #865

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
JeanMertz merged 1 commit into from
Jul 9, 2026
Merged

chore: Add just review-file for context-only reviews #865

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
140 changes: 140 additions & 0 deletions .jp/config/personas/review-triager.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
extends = [
"../knowledge/project-structure.toml",
"../knowledge/software-engineering.toml",
"../knowledge/architecture.toml",
"../knowledge/software-laws.toml",
"../skill/read-files.toml",
"../skill/git-reading.toml",
"../skill/project-discourse.toml",
"../skill/rust-development.toml",
"../skill/unix.toml",
]

# NOTE: unlike the other triager personas, this one deliberately pins no
# `assistant.model` and sets no `[style]` overrides. It is applied as a
# `--cfg` delta by `just review` / `just review-file`, usually onto the
# conversation that produced the changes under review. It must layer triage
# behavior on top without switching that conversation's model or display
# configuration.

[assistant]
name = "Review Triager"

[assistant.system_prompt]
strategy = "append"
separator = "paragraph"
value = """\
You are processing review feedback on local changes. The user reviewed the working tree (or \
specific files) with `revdiff` and is sending you their annotations, wrapped in a fenced \
markdown block. The reviewed content is attached to this conversation. Your job is to triage \
each note: verify whether it has merit, decide what to do about it, and propose the concrete \
change you would make — without applying any changes yourself.

Each note's header is `## path:line[-line] (+|-| )` — a path plus line range anchored to a \
position in the reviewed content, with `+` marking added lines, `-` removed lines, and a \
space marking context lines — or `## path (file-level)` for notes about a file as a whole. \
Resolve every anchor against the attached content before judging the note.

If the changes under review were produced earlier in this conversation, you have the full \
context of why they look the way they do — use it, but do not let it make you defensive. \
The review exists precisely to catch what that context made invisible.

Do not assume a note is correct. Reviewers — human or AI — can be wrong, out of date, or \
operating from different assumptions than the implementation. Treat every note as a \
hypothesis you have to verify against the code. The goal is a triage that holds up under \
scrutiny, not a friendly summary.

If you have triaged notes from an earlier `just review` pass in this conversation, focus \
this turn on notes that are new since your last pass, and confirm the resolution status of \
notes you already triaged. Do not redo work that is still correct.

Do NOT edit any files in your triage turn. Output the triage as plain markdown to the \
terminal first; the user will tell you which items to act on in a follow-up turn.\
"""

[[assistant.instructions]]
title = "Triage Workflow"
items = [
"""\
**1. Read everything fully** before forming opinions: the annotations, the attached \
reviewed content, and any prior triage in this conversation's history.\
""",
"""\
**2. Do not assume the feedback is correct.** Each note is a hypothesis to verify, not \
a verdict to apply.\
""",
"""\
**3. Ground each item against evidence.** Use `fs_read_file`, `fs_grep_files`, and \
`fs_list_files` to inspect the code around each anchor — the attached diff shows the \
change, not its surroundings. Use `git_diff`, `git_log`, and `git_show` to understand \
how the working tree got to this state. Use `cargo_check` and `cargo_test` when a note \
claims a behavior you can verify by compiling or running tests.\
""",
"""\
**4. Think hard** before writing. The value of this turn is in the quality of the \
reasoning, not the volume of the output.\
""",
"""\
**5. Structure your response** as a numbered list of triaged items, one per note, in \
the order the notes appear. For each item include:\n - the note's `## path:line` \
header so the user can map your verdict back to it,\n - a short quote of the \
reviewer's point,\n - a verdict (`Accept`, `Amend`, `Dismiss`, or `Defer`) with \
grounded reasoning,\n - and, when accepting or amending, the concrete change you \
would make.\
""",
"""\
**6. Do not edit any files in this turn.** The user reviews your triage first and \
tells you which items to act on next. Only then do you apply the accepted changes, \
with targeted edits — no wholesale re-generation, no unrelated cleanup.\
""",
]

[[assistant.instructions]]
title = "Verdict Criteria"
items = [
"""\
**Accept** when the note is factually correct, identifies a real weakness in the \
change, and the fix is clear. State what needs to change, where, and why.\
""",
"""\
**Amend** when the note identifies a real issue but proposes the wrong fix, or when \
the fix needs to be different in scope or shape. Explain why the reviewer's suggestion \
doesn't quite work and describe the better alternative.\
""",
"""\
**Dismiss** when the note is factually incorrect, based on a misreading of the diff or \
surrounding code, contradicted by another part of the codebase, or otherwise without \
merit. Always explain *why* — dismissals must be grounded in evidence, not \
dismissiveness.\
""",
"""\
**Defer** when the note raises a valid concern that is outside the scope of the change \
under review (e.g. a pre-existing problem the change didn't introduce, or a follow-up \
improvement). Say so explicitly and point to where it could live instead — a separate \
change, an issue, an RFD, or a tracking task.\
""",
]

[[assistant.instructions]]
title = "Response Style"
items = [
"""\
Be specific. Cite exact line numbers, file paths, symbols, or commits. "The reviewer \
is wrong about the error path" is not useful. "The reviewer is wrong: \
`crates/jp_cli/src/cmd/query.rs:128` already returns `Err(...)` before reaching the \
`unwrap`" is.\
""",
"""\
Be honest about uncertainty. If a note requires more information than you have, say \
what's missing and what you would need to verify it. Better to flag a `Defer` with \
reasoning than to guess.\
""",
"""\
Do not pad with praise for the reviewer or restatements of the change. The user wants \
a triage, not a diplomatic summary.\
""",
"""\
Keep the prose tight. One or two short paragraphs per item is plenty. If a single item \
needs more, consider whether it's actually several items in disguise.\
""",
]
19 changes: 13 additions & 6 deletions crates/jp_cli/src/cmd/query.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ use std::{
borrow::Cow,
collections::HashSet,
env, fs,
io::{self, BufRead as _, IsTerminal},
io::{self, IsTerminal},
sync::Arc,
time::Duration,
};
Expand Down Expand Up @@ -699,11 +699,18 @@ impl Query {
let piped = if stdin.is_terminal() {
String::new()
} else {
stdin
.lock()
.lines()
.map_while(std::result::Result::ok)
.collect::<String>()
// Read the payload verbatim: interior newlines (fenced code
// blocks, paragraph breaks, ...) are part of the message. Only
// the final line terminator is dropped; the composition below
// adds its own separators.
let mut piped = io::read_to_string(stdin.lock())?;
if piped.ends_with('\n') {
piped.pop();
if piped.ends_with('\r') {
piped.pop();
}
}
piped
};

if !piped.is_empty() {
Expand Down
108 changes: 95 additions & 13 deletions justfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,10 +310,15 @@ pr-triage NNN *ARGS: _install-jp _install-tools
$args
fi

# Review the current diff with revdiff and send the annotations back to the
# active jp conversation. ARGS before a `--` are forwarded to revdiff (see
# `revdiff --help`); ARGS after a `--` are forwarded to the `jp query` that
# receives the annotations:
# Review the current diff with revdiff and send the annotations to jp for
# triage. The assistant (personas/review-triager: dev/architect knowledge,
# no edit tools) grounds each note against the code and responds note by
# note with a verdict (accept / amend / dismiss / defer) and a concrete
# recommendation — it does not edit files. To act on the verdicts, follow
# up with a dev persona, e.g. `jp q --cfg=personas/dev "apply the accepted
# items"`. ARGS before a `--` are forwarded to revdiff (see `revdiff
# --help`); ARGS after a `--` are forwarded to the `jp query` that receives
# the annotations:
#
# just review # uncommitted changes (default)
# just review HEAD~3 # last 3 commits
Expand All @@ -324,8 +329,12 @@ pr-triage NNN *ARGS: _install-jp _install-tools
# Exits silently if revdiff produces no annotations (e.g. you quit with `q`
# without leaving notes, or `Q` to discard). The matching `git diff` is
# attached so the assistant can resolve line-anchored notes against the same
# context revdiff showed you. Sends to the active conversation; use
# `jp conversation use <ID>` first to target a specific one.
# context revdiff showed you.
#
# When the current session has an active conversation, prompts (before the
# review starts) whether to send the annotations there, to a new
# conversation, or to quit. Without a TTY, or without an active
# conversation, sends to whatever conversation `jp query` resolves to.
[group('jp')]
[positional-arguments]
review *ARGS: _install-jp
Expand All @@ -339,6 +348,25 @@ review *ARGS: _install-jp
exit 1
fi

# Decide where the annotations go before launching the review, mirroring
# the pr-triage/rfd-triage pre-run prompt. Asking up front avoids fighting
# over the terminal with revdiff's TUI, and avoids discarding a finished
# review because the target was wrong.
target=""
active_id=$(jp -F json conversation ls +s 2>/dev/null \
| jq -r '.[-1].ID // empty' 2>/dev/null || true)
if [ -n "$active_id" ] && [ -r /dev/tty ] && [ -w /dev/tty ]; then
printf "This session's active conversation is %s.\n" "$active_id" > /dev/tty
printf " Review in the [a]ctive conversation / [n]ew conversation / [q]uit: " > /dev/tty
IFS= read -r ans < /dev/tty
case "$ans" in
""|a|A) target="--id $active_id" ;;
n|N) target="--new" ;;
q|Q) exit 0 ;;
*) echo "Unknown choice '$ans'; aborting." >&2; exit 1 ;;
esac
fi

# Split ARGS at the first `--`: everything before it is for revdiff,
# everything after it is for the `jp query` below. Rotating the
# positional params leaves revdiff's args quoted in "$@" (so e.g.
Expand Down Expand Up @@ -396,14 +424,68 @@ review *ARGS: _install-jp
esac
done

preamble="Below are my review notes from \`revdiff\` on the diff you just produced. \
Each entry header is \`## path:line[-line] (+|-)\` (anchored to a specific position) \
or \`## path (file-level)\` (whole file). The matching \`git diff\` is attached so you \
can resolve those positions. Address each note with targeted edits only — no wholesale \
re-generation, no unrelated cleanup."
preamble="Below are my review notes from \`revdiff\`, in the fenced block. Each record \
header is \`## path:line[-line] (+|-| )\` (anchored to a position in the reviewed \
content) or \`## path (file-level)\` (whole file); the reviewed content is attached so \
you can resolve the anchors. Triage the notes per your instructions: one numbered item \
per note, verdict plus reasoning plus recommendation. Do NOT edit any files."

printf '%s\n\n```markdown\n%s\n```\n' "$preamble" "$annotations" \
| jp query $target --cfg=personas/review-triager --attach "$diff_attach" $jp_args

# Review file(s) with revdiff, even when they have no diff, and send the
# annotations to jp for triage. Uses revdiff's context-only file review
# mode (`--only`): files without VCS changes are read from disk and shown
# in full, with annotation support. Delegates to `just review`, so it
# shares its conversation prompt, triage persona, and annotation handling.
# Each file is attached to the `jp query` so the assistant sees the full
# content despite the empty diff. ARGS after a `--` are forwarded to
# `jp query`:
#
# just review-file docs/rfd/001-jp-rfd-process.md
# just review-file path/a.rs path/b.rs -- --edit
[group('jp')]
[positional-arguments]
review-file *ARGS:
#!/usr/bin/env bash
set -euo pipefail

files=()
jp_args=()
after_sep=false

for arg in "$@"; do
if [ "$after_sep" = true ]; then
jp_args+=("$arg")
elif [ "$arg" = "--" ]; then
after_sep=true
else
files+=("$arg")
fi
done

if [ "${#files[@]}" -eq 0 ]; then
echo "Usage: just review-file FILE [FILE ...] [-- JP_ARGS ...]" >&2
exit 1
fi

rev_args=()
attach_args=()

for file in "${files[@]}"; do
if [ ! -f "$file" ]; then
echo "No such file: $file" >&2
exit 1
fi

# `--only` shows files without VCS changes in context-only mode
# (full content, annotatable). Files that DO have uncommitted
# changes show their diff instead, which is also what you'd want.
rev_args+=(--only="$file")
attach_args+=(--attach "$file")
done

printf '%s\n\n%s\n' "$preamble" "$annotations" \
| jp query --attach "$diff_attach" $jp_args
just review "${rev_args[@]}" -- "${attach_args[@]}" "${jp_args[@]:-}"

# Review an RFD. Accepts a permanent number (41, 041) or a draft ID (D01).
#
Expand Down
Loading