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
47 changes: 43 additions & 4 deletions app/src-tauri/src/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,12 @@ pub async fn get_evidence(
/// `base_sha` when set, else the base branch name; the head is `head_sha` when
/// set, else `HEAD` for a local read or the head branch name for a GitHub read.
///
/// Known limitation for imported PRs: their `base_sha` is empty (it is the
/// restack fork point, computed locally at kickoff — an import has none), so the
/// base side resolves by branch name and may drift past the PR's actual fork
/// point once the base branch advances. The New side is pinned to `head_sha` and
/// is authoritative.
///
/// Content is read locally with
/// [`git::file_at_rev`](cockpit_core::adapters::git::file_at_rev) (off the async
/// runtime, since `git2` is blocking) when a usable local repo dir exists — a
Expand Down Expand Up @@ -2398,10 +2404,13 @@ pub async fn fetch_review_requests(

/// Shared implementation for fetching PRs by filter.
///
/// When a review already exists in the store (matched by PR URL), only the
/// diff, branch, and base are refreshed — comments, gate state, agent run,
/// and stale flag are preserved. This prevents re-fetching from GitHub from
/// blowing away in-progress review work.
/// When a review already exists in the store (matched by PR URL), the diff,
/// branch, base, CI summary, and pinned base/head SHAs are refreshed from
/// GitHub — comments, gate state, agent run, and stale flag are preserved so a
/// re-fetch never blows away in-progress review work. The head SHA is held back
/// while a rework is in flight (an attached agent or `Dispatched` state): the
/// local worktree HEAD leads GitHub's last-reported OID then, so adopting the
/// fetched head would point the diff/full-file view at a stale revision.
async fn fetch_prs_by_filter(
state: State<'_, Arc<AppState>>,
filter: github::PrFilter,
Expand Down Expand Up @@ -2437,10 +2446,40 @@ async fn fetch_prs_by_filter(
if state.reviews.get(&pr_ref).is_some() {
let branch = pr.head_ref_name.clone();
let base = pr.base_ref_name.clone();
let head_sha = pr.head_ref_oid.clone();
let ci_summary = github::rollup_to_summary(&pr.status_check_rollup);
state.reviews.update(&pr_ref, |r| {
r.diff = cockpit_core::model::DiffData { raw: diff };
r.branch = branch;
r.base = base;

// Refresh CI + the pinned head SHA from GitHub. A failed per-PR
// enrichment falls back to an empty rollup / empty OID; treat
// those as "no fresh data" and keep what we already have rather
// than degrading the diff resolution to a branch-name lookup.
//
// `base_sha` is deliberately NOT refreshed here: it is the restack
// fork point (see `Review::base_sha`), not the base branch tip, so
// the kickoff-computed value must never be clobbered by a GitHub
// read (that would break restack once the base advances).
if let Some(ci) = ci_summary {
r.ci_summary = Some(ci);
}
// The head SHA is authoritative locally while a rework is in
// flight: a review with an attached agent, in `Dispatched`, or in
// `Reworked` has a worktree HEAD that leads what GitHub last
// reported, so adopting the fetched head here would point the
// diff/full-file view at a stale revision. `Reworked` in
// particular: after `apply_agent_completion` the local worktree
// HEAD leads GitHub until the agent's push is visible, and
// reverting it would make the interdiff read empty. Only take
// GitHub's head OID when no rework owns the branch.
let rework_in_flight = r.agent.is_some()
|| r.gate_state == GateState::Dispatched
|| r.gate_state == GateState::Reworked;
if !rework_in_flight && !head_sha.is_empty() {
r.head_sha = head_sha;
}
});
if let Some(updated) = state.reviews.get(&pr_ref) {
reviews.push(updated);
Expand Down
29 changes: 25 additions & 4 deletions app/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import type { ShortcutId } from "./lib/shortcuts";
import { Sidebar } from "./components/Sidebar";
import { ReviewCard } from "./components/ReviewCard";
import { StackContainer } from "./components/StackContainer";
import { FastLaneShelf } from "./components/FastLaneShelf";
import { ProjectCard } from "./components/ProjectCard";
import { ReviewWorkspace } from "./components/ReviewWorkspace";
import { PlanView } from "./components/PlanView";
Expand Down Expand Up @@ -38,6 +39,7 @@ import {
} from "lucide-react";
import { cn } from "@/lib/utils";
import { buildBoardItems } from "./lib/stack-tree";
import { isFastLane } from "./lib/attention";
import type { CardDensity } from "./components/ReviewCard";
import type { GateState } from "./bindings/GateState";
import type { Review } from "./bindings/Review";
Expand Down Expand Up @@ -514,13 +516,18 @@ function App() {

function renderProjectGroupedList(items: readonly Review[]) {
const filtered = filterReviews(items);
const groups = groupReviewsByProject(filtered, projects);

if (groups.length === 0) {
// Fast lane filters AFTER search/state filters, then its members are
// excluded from the project groups below so nothing renders twice.
const fastLane = filtered.filter(isFastLane);
const fastLaneIds = new Set(fastLane.map((r) => r.id));
const rest = filtered.filter((r) => !fastLaneIds.has(r.id));
const groups = groupReviewsByProject(rest, projects);

if (fastLane.length === 0 && groups.length === 0) {
return null;
}

return groups.map((group) => (
const projectSections = groups.map((group) => (
<section key={group.key} className="mb-6">
<h2 className="mb-3 font-display text-sm font-semibold uppercase tracking-wide text-muted-foreground">
{group.title}{" "}
Expand Down Expand Up @@ -552,6 +559,20 @@ function App() {
</div>
</section>
));

return (
<>
{fastLane.length > 0 && (
<FastLaneShelf
reviews={fastLane}
density={prsDensity}
onAction={handleReviewAction}
onRestack={handleRestack}
/>
)}
{projectSections}
</>
);
}

function renderPrsContent(items: readonly Review[], emptyIcon: LucideIcon, emptyTitle: string, emptyDescription: string) {
Expand Down
72 changes: 72 additions & 0 deletions app/src/components/FastLaneShelf.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
/**
* Fast lane shelf (C2): a visually distinct, teal-framed group of
* "small + green + low-risk" reviews surfaced above the project groups.
*
* The shelf compresses the *decision* — these are the reviews a human can
* approve in ~2 minutes — but never the *authority*: its cards use the exact
* same {@link ReviewCard} action path as the rest of the board, so there is
* deliberately no batch-approve or auto-approve here (roadmap C2 / CLAUDE.md
* §9). Membership is decided by {@link isFastLane}; App excludes members from
* the project groups below so nothing renders twice.
*/

import { Zap } from "lucide-react";
import { ReviewCard } from "./ReviewCard";
import type { CardDensity } from "./ReviewCard";
import type { Review } from "../bindings/Review";
import { attentionRank } from "../lib/attention";

interface FastLaneShelfProps {
/** The fast-lane members to render; the shelf is not rendered when empty. */
readonly reviews: readonly Review[];
/** Presentation density, mirroring the rest of the board. */
readonly density: CardDensity;
/** Same handler the board uses — no new terminal action is introduced here. */
readonly onAction: (pr: string) => void;
/** Restack a stale member; wired through for parity with the board cards. */
readonly onRestack: (pr: string) => void;
}

/** Order the shelf attention-first, using the board's rank + id tiebreak. */
function orderByAttention(reviews: readonly Review[]): readonly Review[] {
return [...reviews].sort((a, b) => {
const byRank = attentionRank(a) - attentionRank(b);
if (byRank !== 0) return byRank;
return a.id < b.id ? -1 : a.id > b.id ? 1 : 0;
});
}

/** The teal-framed fast-lane shelf. */
export function FastLaneShelf({
reviews,
density,
onAction,
onRestack,
}: FastLaneShelfProps) {
const ordered = orderByAttention(reviews);
const count = ordered.length;

return (
<section className="mb-6 rounded-xl border border-brand/40 bg-brand/5 p-3">
<h2 className="mb-3 flex items-center gap-1.5 font-mono text-xs tracking-wide text-brand">
<Zap className="h-3.5 w-3.5 shrink-0" aria-hidden="true" />
<span className="font-semibold uppercase">Fast lane</span>
<span className="text-brand/70">
· {String(count)} {count === 1 ? "PR" : "PRs"} · small + green +
low-risk
</span>
</h2>
<div className={density === "compact" ? "space-y-1.5" : "space-y-3"}>
{ordered.map((review) => (
<ReviewCard
key={review.id}
review={review}
density={density}
onAction={onAction}
onRestack={onRestack}
/>
))}
</div>
</section>
);
}
76 changes: 76 additions & 0 deletions app/src/components/ReviewCard.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,15 @@ import userEvent from "@testing-library/user-event";
import { ReviewCard } from "./ReviewCard";
import { makeReview, makeAgentRun, ALL_GATE_STATES } from "../test/fixtures";

/** A diff adding `n` lines to `path`, for exercising the risk chips. */
function addLines(path: string, n: number): string {
let s = `diff --git a/${path} b/${path}\n--- a/${path}\n+++ b/${path}\n@@ -0,0 +1,${String(n)} @@\n`;
for (let i = 0; i < n; i++) {
s += `+row ${String(i)}\n`;
}
return s;
}

describe("ReviewCard", () => {
it("shows the Restack button only when the review is stale", () => {
const fresh = makeReview({ stale: false });
Expand Down Expand Up @@ -119,4 +128,71 @@ describe("ReviewCard", () => {
screen.getByRole("button", { name: "Review" }),
).toBeInTheDocument();
});

it("renders a CI x/y chip from the review's ci_summary", () => {
render(
<ReviewCard
review={makeReview({
ci_summary: { passed: 2, total: 3, failed: 1, pending: 0 },
diff: { raw: addLines("data.txt", 10) },
})}
onAction={vi.fn()}
onRestack={vi.fn()}
/>,
);
expect(screen.getByText("2/3")).toBeInTheDocument();
});

it("adds the F6 splitting nudge to the size chip past 400 changed lines", () => {
render(
<ReviewCard
review={makeReview({ diff: { raw: addLines("data.txt", 500) } })}
onAction={vi.fn()}
onRestack={vi.fn()}
/>,
);
expect(
screen.getByTitle("Consider splitting (>400 changed lines)"),
).toBeInTheDocument();
});

it("surfaces a sensitive-path chip for a risky file", () => {
render(
<ReviewCard
review={makeReview({
diff: { raw: addLines("migrations/001_init.sql", 5) },
})}
onAction={vi.fn()}
onRestack={vi.fn()}
/>,
);
expect(screen.getByText("migrations")).toBeInTheDocument();
});

it("surfaces a test-touch chip when the diff touches tests", () => {
render(
<ReviewCard
review={makeReview({ diff: { raw: addLines("src/foo.test.ts", 5) } })}
onAction={vi.fn()}
onRestack={vi.fn()}
/>,
);
expect(screen.getByText("tests")).toBeInTheDocument();
});

it("layers a risk note under the gate reason", () => {
render(
<ReviewCard
review={makeReview({
gate_state: "Pending",
ci_summary: { passed: 1, total: 2, failed: 1, pending: 0 },
diff: { raw: addLines("data.txt", 10) },
})}
onAction={vi.fn()}
onRestack={vi.fn()}
/>,
);
expect(screen.getByText(/needs your review/i)).toBeInTheDocument();
expect(screen.getByText(/CI failing/)).toBeInTheDocument();
});
});
Loading
Loading