jerry/buzz
1
0
Fork 0
mirror of https://github.com/block/buzz.git synced 2026-10-30 05:51:46 +00:00
Code Issues Packages Projects Releases Wiki Activity

Refine prototype review check tracks

Browse Source 
Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
This commit is contained in:
Taylor Ho
2026-08-27 14:21:05 -07:00
parent f2d7b9c7e4
commit 4fe6679b60
3 changed files with 106 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files

43
desktop/src/features/projects/lib/projectReviewChecks.mjs
View File

@@ -8,24 +8,47 @@ export const PROJECT_REVIEW_CHECK_RESULT_MARKER = "BUZZ_CHECK_RESULT_V1";
export const DEFAULT_PROJECT_REVIEW_CHECKS = [
{
id: "interface",
name: "Interface & design system",
name: "Interface & frontend",
description:
"Reviews user-facing changes against the design system and their backend contracts.",
"Reviews the user-facing experience and frontend implementation as one surface.",
instructions: [
"Inspect user-facing changes and the backend behavior that supplies them.",
"Check design-system component usage, interaction states, accessibility, and visual consistency.",
"Inspect the user-facing behavior and frontend implementation changed by this review.",
"Check design-system usage, interaction states, accessibility, loading and error states, performance, and visual consistency.",
"Flag hand-rolled UI where the project design system already provides the intended primitive.",
],
},
{
id: "frontend",
name: "Frontend quality",
id: "code-correctness",
name: "Code correctness",
description:
"Reviews frontend correctness, state boundaries, accessibility, and maintainability.",
"Checks whether the changed code behaves correctly in normal, edge, and failure paths.",
instructions: [
"Inspect the frontend implementation changed by this review.",
"Check data flow, loading and error states, accessibility, performance, and maintainability.",
"Only report issues introduced by or made materially worse by this review.",
"Trace the changed behavior through its callers, data flow, and external contracts.",
"Look for logic errors, invalid state transitions, missed edge cases, unsafe error handling, races, and resource-lifecycle defects.",
"Report behavior-level defects introduced by or made materially worse by this review; leave codebase conventions to their own check.",
],
},
{
id: "codebase-patterns",
name: "Codebase patterns",
description:
"Checks whether the implementation follows the architecture and established patterns of this codebase.",
instructions: [
"Compare the change with contributor guidance and analogous code already in the repository.",
"Check architectural boundaries, shared abstractions, naming, error handling, and framework idioms for consistency with established patterns.",
"Report departures that create meaningful maintenance or integration risk; leave functional defects to the code correctness check.",
],
},
{
id: "historical-intent",
name: "Historical intent",
description:
"Explores the history of the changed area to identify intentional or accidental shifts in project philosophy.",
instructions: [
"Inspect relevant line and path history with git blame and git log, then follow related commits into pull requests or review discussion when available.",
"Look for prior design rationale, analogous changes, reversions, and decisions from maintainers or frequent contributors to this area.",
"Evaluate whether this review continues, deliberately revises, or accidentally contradicts the philosophy behind the existing implementation; clearly distinguish historical evidence from inference.",
"Do not reject a change merely because it differs from precedent. Recommend a fix only when an unexplained philosophical shift creates material product, architecture, or maintenance risk.",
],
},
{

28
desktop/src/features/projects/lib/projectReviewChecks.test.mjs
View File

@@ -12,13 +12,31 @@ import {
writeProjectReviewCheckRuns,
} from "./projectReviewChecks.mjs";
test("starter checks cover interface, frontend, and test quality", () => {
test("starter checks keep experience, correctness, patterns, history, and tests independent", () => {
assert.deepEqual(
DEFAULT_PROJECT_REVIEW_CHECKS.map((check) => check.id),
["interface", "frontend", "test-quality"],
[
"interface",
"code-correctness",
"codebase-patterns",
"historical-intent",
"test-quality",
],
);
});
test("historical intent check investigates precedent without treating it as a veto", () => {
const check = DEFAULT_PROJECT_REVIEW_CHECKS.find(
(candidate) => candidate.id === "historical-intent",
);
assert.ok(check);
const instructions = check.instructions.join("\n");
assert.match(instructions, /git blame and git log/);
assert.match(instructions, /pull requests or review discussion/);
assert.match(instructions, /historical evidence from inference/);
assert.match(instructions, /Do not reject a change merely because/);
});
test("parses an approved structured agent result", () => {
const result = parseProjectReviewCheckResult(`Analysis first.
@@ -220,7 +238,7 @@ test("builds a review-only prompt pinned to the exact commit", () => {
assert.match(prompt, /Review only; do not modify code/);
assert.match(prompt, /Commit under review: abc123/);
assert.match(prompt, /Frontend quality/);
assert.match(prompt, /Code correctness/);
assert.match(prompt, /"request_id":"request-123"/);
assert.match(prompt, /Echo request_id exactly/);
assert.match(prompt, /process every event independently/);
@@ -256,7 +274,9 @@ test("local check state is isolated by relay, signer, repo, and review", () => {
getItem: (key) => entries.get(key) ?? null,
setItem: (key, value) => entries.set(key, value),
};
const runs = { frontend: { agentPubkey: "agent", status: "idle" } };
const runs = {
"code-correctness": { agentPubkey: "agent", status: "idle" },
};
writeProjectReviewCheckRuns(storage, first, runs);
assert.deepEqual(readProjectReviewCheckRuns(storage, first), runs);
assert.deepEqual(readProjectReviewCheckRuns(storage, second), {});

70
desktop/tests/e2e/project-pr-review.spec.ts
View File

@@ -167,7 +167,7 @@ test("review checks dispatch to an agent authorized for the relay identity", asy
const checks = page.getByTestId("project-review-checks");
await expect(checks).toBeVisible();
await expect(checks.getByTestId("project-review-check")).toHaveCount(3);
await expect(checks.getByTestId("project-review-check")).toHaveCount(5);
const interfaceCheck = checks.getByTestId("project-review-check").first();
await page.getByTestId("project-review-debug-harness-trigger").click();
await page
@@ -201,7 +201,7 @@ test("review checks dispatch to an agent authorized for the relay identity", asy
| { content?: string; mentionPubkeys?: string[] }
| undefined;
});
expect(sentCheck?.content).toContain("Interface & design system");
expect(sentCheck?.content).toContain("Interface & frontend");
expect(sentCheck?.content).toContain("Commit under review:");
expect(sentCheck?.content).toContain("buzz messages send-diff");
expect(sentCheck?.mentionPubkeys).toContain(RELAY_REVIEW_AGENT_PUBKEY);
@@ -449,11 +449,11 @@ test("concurrent review checks only accept their correlated result", async ({
await page.getByRole("button", { name: "Checks", exact: true }).click();
const checks = page.getByTestId("project-review-check");
await expect(checks).toHaveCount(3);
await expect(checks).toHaveCount(5);
for (const check of await checks.all()) {
await check.getByRole("button", { name: "Run", exact: true }).click();
}
await expect(checks.filter({ hasText: "Running" })).toHaveCount(3);
await expect(checks.filter({ hasText: "Running" })).toHaveCount(5);
await expect
.poll(() =>
@@ -470,7 +470,7 @@ test("concurrent review checks only accept their correlated result", async ({
).length ?? 0,
),
)
.toBe(3);
.toBe(5);
const prompts = await page.evaluate(
() =>
@@ -490,12 +490,20 @@ test("concurrent review checks only accept their correlated result", async ({
requestIdFromCheckPrompt(
prompts.find((prompt) => prompt.includes(`"${checkName}"`)),
);
const interfaceRequestId = requestIdFor("Interface & design system");
const frontendRequestId = requestIdFor("Frontend quality");
const interfaceRequestId = requestIdFor("Interface & frontend");
const correctnessRequestId = requestIdFor("Code correctness");
const patternsRequestId = requestIdFor("Codebase patterns");
const historicalIntentRequestId = requestIdFor("Historical intent");
const testRequestId = requestIdFor("Test quality");
const openerByRequestId = new Map(
await Promise.all(
[interfaceRequestId, frontendRequestId, testRequestId].map(
[
interfaceRequestId,
correctnessRequestId,
patternsRequestId,
historicalIntentRequestId,
testRequestId,
].map(
async (requestId) =>
[requestId, await checkOpenerEventId(page, requestId)] as const,
),
@@ -503,14 +511,14 @@ test("concurrent review checks only accept their correlated result", async ({
);
const responses = [
{
request_id: frontendRequestId,
request_id: patternsRequestId,
conclusion: "fix-recommended",
summary: "Frontend-specific result.",
summary: "Pattern-specific result.",
findings: [
{
title: "Frontend-only finding",
detail: "This must stay on the frontend check.",
file: "desktop/src/frontend.tsx",
title: "Pattern-only finding",
detail: "This must stay on the codebase patterns check.",
file: "desktop/src/pattern.ts",
line: 12,
},
],
@@ -527,6 +535,18 @@ test("concurrent review checks only accept their correlated result", async ({
summary: "Interface-specific result.",
findings: [],
},
{
request_id: correctnessRequestId,
conclusion: "approved",
summary: "Correctness-specific result.",
findings: [],
},
{
request_id: historicalIntentRequestId,
conclusion: "approved",
summary: "Historical-intent-specific result.",
findings: [],
},
];
await waitForMockLiveSubscription(page, "DM");
@@ -547,9 +567,9 @@ test("concurrent review checks only accept their correlated result", async ({
response: responses[0],
},
);
await expect(checks.nth(1)).toContainText("Running");
await expect(checks.nth(2)).toContainText("Running");
const targetIndexes = [1, 2, 0];
const targetIndexes = [2, 4, 0, 1, 3];
for (const [index, response] of responses.entries()) {
await page.evaluate(
({ agentPubkey, createdAt, openerEventId, response }) => {
@@ -574,16 +594,24 @@ test("concurrent review checks only accept their correlated result", async ({
);
if (index === 0) {
await expect(checks.nth(0)).toContainText("Running");
await expect(checks.nth(2)).toContainText("Running");
await expect(checks.nth(1)).toContainText("Running");
await expect(checks.nth(3)).toContainText("Running");
await expect(checks.nth(4)).toContainText("Running");
}
}
await expect(checks.nth(0)).toContainText("Interface-specific result.");
await expect(checks.nth(1)).toContainText("Frontend-specific result.");
await expect(checks.nth(1)).toContainText("Frontend-only finding");
await expect(checks.nth(2)).toContainText("Test-specific result.");
await expect(checks.nth(0)).not.toContainText("Frontend-specific result.");
await expect(checks.nth(2)).not.toContainText("Frontend-specific result.");
await expect(checks.nth(1)).toContainText("Correctness-specific result.");
await expect(checks.nth(2)).toContainText("Pattern-specific result.");
await expect(checks.nth(2)).toContainText("Pattern-only finding");
await expect(checks.nth(3)).toContainText(
"Historical-intent-specific result.",
);
await expect(checks.nth(4)).toContainText("Test-specific result.");
await expect(checks.nth(0)).not.toContainText("Pattern-specific result.");
await expect(checks.nth(1)).not.toContainText("Pattern-specific result.");
await expect(checks.nth(3)).not.toContainText("Pattern-specific result.");
await expect(checks.nth(4)).not.toContainText("Pattern-specific result.");
});
test("same-second request changes supersedes approval", async ({ page }) => {