4 Commits

Author SHA1 Message Date
temeddix dddac76955 Inline comments
Check / deno (pull_request) Successful in 55s
2026-09-14 09:08:46 +09:00
temeddix 2b51793c92 Review on request (#9)
A pull request comment that asks for a review now posts a real review, not a plain comment: the review file decides, the event no longer does.

Verified with deno fmt, lint and check.

Reviewed-on: #9
Co-authored-by: bot <temeddix@gmail.com>
Co-committed-by: bot <temeddix@gmail.com>
2026-09-13 23:48:04 +00:00
temeddix 4ed8b48b7a Verdict words (#8)
The review file starts with `Approved` or `Changes requested` instead of `Yes`/`No`/`With fixes`. The match is still the whole trimmed first line; `run.ts` prepends , 🛑, or 💬 when posting, so the mark never takes part in the match.

Verified: `deno fmt`, `deno lint`, `deno check` pass; a scratch run of the matcher maps `Approved` → APPROVED, `Changes requested` → REQUEST_CHANGES, and `Approved, mostly`, ` Approved`, `Yes` → COMMENT.
Reviewed-on: #8
Co-authored-by: Danny Kim <temeddix@gmail.com>
Co-committed-by: Danny Kim <temeddix@gmail.com>
2026-09-13 17:21:34 +00:00
temeddix 0d109ebec8 Review file (#7)
The reviewer writes its review to a file whose first line is the verdict, instead of relying on a narration-free final message. Sonnet put `Yes` after three paragraphs of narration on memona #936 (review 595), which the fail-closed verdict posted as a plain comment.

Reviewed-on: #7
Co-authored-by: Danny Kim <temeddix@gmail.com>
Co-committed-by: Danny Kim <temeddix@gmail.com>
2026-09-13 16:54:54 +00:00
2 changed files with 107 additions and 35 deletions
+31 -18
View File
@@ -18,7 +18,8 @@ the head of the pull request when there is one, with full history and the
author's push credentials. Read the code there, run its checks and tests when author's push credentials. Read the code there, run its checks and tests when
they bear on the task, and push from there. they bear on the task, and push from there.
For a `pull_request` event, review the PR without changing code, using the For a `pull_request` event, and for a comment on a pull request that asks you to
review it, review the PR without changing code, using the
`requesting-code-review` skill from superpowers: run its code reviewer template `requesting-code-review` skill from superpowers: run its code reviewer template
against the PR's base and head. Run the project's checks on the head and treat a against the PR's base and head. Run the project's checks on the head and treat a
failure as at least Important. Check the whole repository against the code rules failure as at least Important. Check the whole repository against the code rules
@@ -26,24 +27,36 @@ at the end of this prompt, not only the diff; a violation is at least Important
even when the diff did not cause it. Write the complete review, and nothing even when the diff did not cause it. Write the complete review, and nothing
else, to the file `${REVIEW_PATH}`: it is posted verbatim as a pull request else, to the file `${REVIEW_PATH}`: it is posted verbatim as a pull request
review from the bot account, and your final response is not posted at all. The review from the bot account, and your final response is not posted at all. The
file's first line must be exactly the template's verdict and nothing else: file's first line must be exactly the verdict and nothing else: `Approved` when
`Yes`, `No`, or `With fixes`. `Yes` approves and the other two request changes; the template's answer is yes, `Changes requested` otherwise. The mark in front
any other first line is posted as a plain comment, which wastes the run. Minor of it is added when posting, so write the words alone; any other first line is
issues alone never block, and neither does a finding the author has answered in posted as a plain comment, which wastes the run. Minor issues alone never block,
the comment history below as intended or a false alarm, once the code or docs and neither does a finding the author has answered in the comment history below
make that clear. When the verdict is not `Yes`, the second line names what must as intended or a false alarm, once the code or docs make that clear. When the
change in one line, addressed to the author; the author's own agent picks the verdict is `Changes requested`, the second line names what must change in one
fixes up, so never ask `@bot` to make them. For UI changes, check that the line, addressed to the author; the author's own agent picks the fixes up, so
result is aligned, clean, and pixel-perfect, and that included screenshots prove never ask `@bot` to make them. For UI changes, check that the result is aligned,
the intended result was achieved. clean, and pixel-perfect, and that included screenshots prove the intended
result was achieved.
The review must read at a glance: everything outside `<details>` blocks totals Every finding that belongs to one line of the diff goes on that line instead of
under 512 bytes. Only core information stays visible: the verdict, the summary into the body. Write those to `${ANCHORS_PATH}` as a JSON array, each entry
line, and the section headings. Anything verbose goes into a `<details>` block `{"path": "<path from the repository root>", "line": <number>, "side": "new" |
whose `<summary>` is a few words, such as the `file:line` and title of an issue "old", "body": "<the finding>"}`.
with the what, why, and how inside; the same for each strength, each `side` is `new` for a line in the head file and `old` for one only in the base
recommendation, the reasoning, and any compliance notes. Details blocks are file; `line` is that file's own line number, and it must be a line the diff
top-level, never inside a list item, because Gitea breaks them there. touches, or Gitea refuses the anchor. Write the file only when there is
something to anchor, and keep each body to the what, the why, and the how, with
no `file:line` prefix; the line carries that.
The review body must read at a glance: everything outside `<details>` blocks
totals under 512 bytes. Only core information stays visible: the verdict, the
summary line, and the section headings. Anything verbose goes into a `<details>`
block whose `<summary>` is a few words, such as the title of an issue with the
what, why, and how inside; the same for each strength, each recommendation, the
reasoning, and any compliance notes. A finding you anchored belongs there only
as its title, since its detail is on the line. Details blocks are top-level,
never inside a list item, because Gitea breaks them there.
For an `issue_comment` or `pull_request_review_comment` event, treat the `body` For an `issue_comment` or `pull_request_review_comment` event, treat the `body`
in the triggering comment payload below as the user's exact instruction. in the triggering comment payload below as the user's exact instruction.
+76 -17
View File
@@ -59,26 +59,84 @@ async function postComment(body: string): Promise<void> {
}); });
} }
// A pull request event is a review request, so the review is posted instead // A review is posted whenever the agent wrote one, whether a review request or
// of the response. It comes through a file, because a final chat message picks // a comment asked for it. It comes through a file, because a final chat message
// up narration while a file's first line is written on purpose. That line is // picks up narration while a file's first line is written on purpose. That line
// the verdict; anything unexpected only comments, never approves. // is the verdict, matched whole; anything unexpected only comments, never
const REVIEW_PATH = `${await Deno.makeTempDir()}/review.md`; // approves. The mark in front is added here, so it is never part of the match.
const VERDICTS: Record<string, string> = { const REVIEW_DIR = await Deno.makeTempDir();
Yes: "APPROVED", const REVIEW_PATH = `${REVIEW_DIR}/review.md`;
No: "REQUEST_CHANGES", const VERDICTS: Record<string, [event: string, mark: string]> = {
"With fixes": "REQUEST_CHANGES", Approved: ["APPROVED", "✅"],
"Changes requested": ["REQUEST_CHANGES", "🛑"],
}; };
async function postResult(body: string): Promise<void> { // A finding about one line is posted on that line of the diff rather than as
if (EVENT !== "pull_request") return postComment(body); // `file:line` prose in the body. Those anchors come as JSON, so the file and
const review = await Deno.readTextFile(REVIEW_PATH).catch(() => { // line are structured instead of parsed back out of English; a malformed entry
throw new Error(`no review was written to ${REVIEW_PATH}`); // fails the run, because a silently dropped finding is worse than a red run.
const ANCHORS_PATH = `${REVIEW_DIR}/anchors.json`;
type Anchor = { path: string; line: number; side: "new" | "old"; body: string };
function parseAnchors(text: string): Anchor[] {
const entries: unknown = JSON.parse(text);
if (!Array.isArray(entries)) throw new Error(`${ANCHORS_PATH}: not an array`);
return entries.map((entry: unknown, index) => {
const at = `${ANCHORS_PATH}[${index}]`;
if (typeof entry !== "object" || entry === null) {
throw new Error(`${at}: not an object`);
}
const { path, line, side = "new", body } = entry as Record<string, unknown>;
if (typeof path !== "string" || path === "") {
throw new Error(`${at}.path: expected a repository path`);
}
if (typeof line !== "number" || !Number.isInteger(line) || line < 1) {
throw new Error(`${at}.line: expected a line number`);
}
if (side !== "new" && side !== "old") {
throw new Error(`${at}.side: expected "new" or "old"`);
}
if (typeof body !== "string" || body.trim() === "") {
throw new Error(`${at}.body: expected the finding`);
}
return { path, line, side, body };
}); });
const [verdict] = review.split("\n", 1); }
await gitea(REVIEWER_TOKEN, `repos/${REPO}/pulls/${INDEX}/reviews`, {
body: stripAnsi(review), async function readAnchors(): Promise<Anchor[]> {
event: VERDICTS[verdict.trim()] ?? "COMMENT", const written = await Deno.readTextFile(ANCHORS_PATH).catch(() => null);
return written === null ? [] : parseAnchors(written);
}
async function postResult(body: string): Promise<void> {
const review = await Deno.readTextFile(REVIEW_PATH).catch(() => null);
if (review === null) {
if (EVENT === "pull_request") {
throw new Error(`no review was written to ${REVIEW_PATH}`);
}
return postComment(body);
}
const [verdict, ...rest] = review.split("\n");
const [event, mark] = VERDICTS[verdict.trim()] ?? ["COMMENT", "💬"];
const post = (anchors: Anchor[]) =>
gitea(REVIEWER_TOKEN, `repos/${REPO}/pulls/${INDEX}/reviews`, {
body: stripAnsi([`${mark} ${verdict.trim()}`, ...rest].join("\n")),
event,
comments: anchors.map(({ path, line, side, body }) => ({
path,
body: stripAnsi(body),
new_position: side === "new" ? line : 0,
old_position: side === "old" ? line : 0,
})),
});
const anchors = await readAnchors();
// Gitea rejects the whole review when an anchor names a line outside the
// diff, and a verdict that never lands blocks the pull request, so the body
// goes up alone rather than not at all.
await post(anchors).catch(async (error: Error) => {
if (anchors.length === 0) throw error;
console.error(`inline comments rejected: ${error.message}`);
await post([]);
}); });
} }
@@ -124,6 +182,7 @@ async function renderPrompt(): Promise<string> {
GITEA_REPOSITORY: REPO, GITEA_REPOSITORY: REPO,
ISSUE_INDEX: INDEX, ISSUE_INDEX: INDEX,
REVIEW_PATH, REVIEW_PATH,
ANCHORS_PATH,
}; };
const template = await Deno.readTextFile( const template = await Deno.readTextFile(
new URL("prompt.md", import.meta.url), new URL("prompt.md", import.meta.url),