feat: re-aligned revision cycles to clarify cycles

Author: jahilldev

api/prisma/migrations/20260628000000_rename_review_cycle_to_revision_cycle/migration.sql

@@ -0,0 +1,4 @@
+-- Rename JobRecords.reviewCycle -> revisionCycle.
+-- The column models the revision round (round 1 at plan approval, +1 per human-triggered
+-- work episode), not a per-review-pass counter; the new name removes that ambiguity.
+ALTER TABLE "JobRecords" RENAME COLUMN "reviewCycle" TO "revisionCycle";

api/prisma/schema.prisma

@@ -50,7 +50,7 @@ model JobRecords {
   headSha        String?
   confidence     Int? // latest review confidence (0-100)
   verifyCommand  String? // agent-discovered tests/build command; null/"" = none found yet (re-discovered until a real command appears)
-  reviewCycle    Int               @default(0) // incremented each time handleImplement completes
+  revisionCycle  Int               @default(0) // the revision round: starts at 1 on plan approval, +1 per human-triggered work episode (settled-PR changes). Verify attempts + review passes are budgeted per round.
   attempts       Int               @default(0)
   error          String?
   createdAt      DateTime          @default(now())
@@ -198,7 +198,7 @@ model ReviewPass {
   id         String     @id @default(cuid())
   job        JobRecords @relation(fields: [jobId], references: [id], onDelete: Cascade)
   jobId      String
-  cycle      Int        @default(1) // which implementation round (matches JobRecords.reviewCycle)
+  cycle      Int        @default(1) // which revision round (matches JobRecords.revisionCycle)
   passNumber Int // pass within this cycle
   confidence Int // advisory only — the gate is the rubric + verify, not this number
   verdict    String // ReviewVerdict union
@@ -218,7 +218,7 @@ model VerifyRun {
   id         String     @id @default(cuid())
   job        JobRecords @relation(fields: [jobId], references: [id], onDelete: Cascade)
   jobId      String
-  cycle      Int // implementation round (matches JobRecords.reviewCycle)
+  cycle      Int // revision round (matches JobRecords.revisionCycle)
   attempt    Int // verify attempt within this cycle
   command    String
   ok         Boolean

api/src/job/job.model.ts

@@ -34,7 +34,7 @@ export const ALLOWED_TRANSITIONS: Record<JobState, JobState[]> = {
   SELF_REVIEWING: ['REVISING', 'OPENING_PR'],
   REVISING: ['VERIFYING'],
   OPENING_PR: ['AWAITING_PR_APPROVAL', 'REVISING'],
-  AWAITING_PR_APPROVAL: ['IMPLEMENTING', 'DONE'],
+  AWAITING_PR_APPROVAL: ['REVISING', 'DONE'],
   DONE: [],
   FAILED: [],
   CANCELLED: [],
@@ -95,7 +95,7 @@ export interface JobSummaryDto {
   issueTitle: string;
   state: string;
   confidence: number | null;
-  reviewCycle: number;
+  revisionCycle: number;
   prNumber: number | null;
   prUrl: string | null;
   prIsDraft: boolean;

api/src/job/job.service.ts

@@ -186,7 +186,7 @@ export class JobService {
         issueTitle: job.issueTitle,
         state: job.state,
         confidence: job.confidence,
-        reviewCycle: job.reviewCycle,
+        revisionCycle: job.revisionCycle,
         prNumber: job.prNumber,
         prUrl: job.pr?.url ?? null,
         prIsDraft: job.pr?.isDraft ?? false,
@@ -273,7 +273,7 @@ export class JobService {
       issueBody: job.issueBody,
       state: job.state,
       confidence: job.confidence,
-      reviewCycle: job.reviewCycle,
+      revisionCycle: job.revisionCycle,
       prNumber: job.prNumber,
       prUrl: job.pr?.url ?? null,
       prIsDraft: job.pr?.isDraft ?? false,

api/src/job/job.utility.spec.ts

@@ -17,7 +17,8 @@ describe('canTransition', () => {
     expect(canTransition('VERIFYING', 'REVISING')).toBe(true);
     expect(canTransition('SELF_REVIEWING', 'REVISING')).toBe(true);
     expect(canTransition('REVISING', 'VERIFYING')).toBe(true);
-    expect(canTransition('AWAITING_PR_APPROVAL', 'IMPLEMENTING')).toBe(true);
+    expect(canTransition('AWAITING_PR_APPROVAL', 'REVISING')).toBe(true);
+    expect(canTransition('AWAITING_PR_APPROVAL', 'IMPLEMENTING')).toBe(false);
   });
 
   it('forbids illegal jumps', () => {

api/src/orchestrator/orchestrator.service.spec.ts

@@ -30,7 +30,7 @@ function makeJob(overrides: Record<string, unknown> = {}) {
   return {
     id: 'job1',
     state: 'VERIFYING',
-    reviewCycle: 1,
+    revisionCycle: 1,
     issueNumber: 5,
     issueTitle: 'Title',
     issueBody: 'Body',
@@ -289,17 +289,35 @@ describe('OrchestratorService.handleVerify', () => {
 });
 
 describe('OrchestratorService.handleImplement', () => {
-  it('single pass → increments cycle, then verifies', async () => {
+  it('single pass → verifies without bumping the revision round', async () => {
     const { service, queue, jobs, workspace } = setup({ job: { state: 'IMPLEMENTING' } });
 
     await callPrivate(service, 'handleImplement', 'job1');
 
     expect(workspace.commitAll).toHaveBeenCalled();
-    expect(jobs.update).toHaveBeenCalledWith('job1', { reviewCycle: { increment: 1 } });
+    // The round advances only at a human-episode transition (approval / settled-PR feedback),
+    // never inside IMPLEMENT.
+    expect(jobs.update).not.toHaveBeenCalledWith('job1', { revisionCycle: { increment: 1 } });
     expect(transitionedTo(jobs)).toContain('VERIFYING');
     expect(enqueuedKinds(queue)).toEqual(['VERIFY']);
   });
 
+  it('only folds in PR feedback newer than the last implement run', async () => {
+    const { service, prisma } = setup({ job: { state: 'IMPLEMENTING', prNumber: 7 } });
+    const anchor = new Date(1000);
+    prisma.agentRun.findFirst.mockResolvedValue({ createdAt: anchor });
+
+    await callPrivate(service, 'handleImplement', 'job1');
+
+    // Older, already-addressed feedback rounds must be excluded — the query is time-scoped to
+    // the last implement pass, not the whole job's feedback history.
+    expect(prisma.pullRequestFeedback.findMany).toHaveBeenCalledWith(
+      expect.objectContaining({
+        where: expect.objectContaining({ createdAt: { gt: anchor } }),
+      }),
+    );
+  });
+
   it('marks the run FAILED and throws when nothing was committed', async () => {
     const { service, agent, workspace, queue } = setup({ job: { state: 'IMPLEMENTING' } });
     workspace.commitAll.mockResolvedValue(null);
@@ -461,6 +479,8 @@ describe('OrchestratorService.approvePlan', () => {
       where: { jobId: 'job1', status: 'PROPOSED' },
       data: { status: 'APPROVED' },
     });
+    // Approval opens revision round 1.
+    expect(jobs.update).toHaveBeenCalledWith('job1', { revisionCycle: { increment: 1 } });
     expect(transitionedTo(jobs)).toContain('IMPLEMENTING');
     expect(enqueuedKinds(queue)).toEqual(['IMPLEMENT']);
   });
@@ -529,15 +549,17 @@ describe('OrchestratorService.onPullRequestReview (changes requested)', () => {
     isBot: false,
   };
 
-  it('records feedback and restarts the cycle when the PR is awaiting approval', async () => {
+  it('opens a new revision round (REVISE) when the PR is awaiting approval', async () => {
     const { service, prisma, queue, jobs } = setup({ job: { state: 'AWAITING_PR_APPROVAL' } });
     prisma.jobRecords.findFirst.mockResolvedValue(makeJob({ state: 'AWAITING_PR_APPROVAL' }));
 
     await service.onPullRequestReview(prEvent as never);
 
     expect(prisma.pullRequestFeedback.create).toHaveBeenCalled();
-    expect(transitionedTo(jobs)).toContain('IMPLEMENTING');
-    expect(enqueuedKinds(queue)).toEqual(['IMPLEMENT']);
+    // A settled PR re-enters via a scoped REVISE in a fresh round, not a full IMPLEMENT.
+    expect(jobs.update).toHaveBeenCalledWith('job1', { revisionCycle: { increment: 1 } });
+    expect(transitionedTo(jobs)).toContain('REVISING');
+    expect(enqueuedKinds(queue)).toEqual(['REVISE']);
   });
 
   it('records feedback but does NOT restart when the job is already working mid-cycle', async () => {
@@ -649,15 +671,16 @@ describe('OrchestratorService dashboard actions', () => {
     expect(jobs.transition).not.toHaveBeenCalled();
   });
 
-  it('requestChanges records feedback and restarts IMPLEMENT', async () => {
+  it('requestChanges records feedback and opens a new REVISE round', async () => {
     const { service, prisma, jobs, queue } = setup({ job: { state: 'AWAITING_PR_APPROVAL' } });
 
     const res = await service.requestChanges('job1', 'dashboard', 'fix Y');
 
     expect(res.ok).toBe(true);
     expect(prisma.pullRequestFeedback.create).toHaveBeenCalled();
-    expect(transitionedTo(jobs)).toContain('IMPLEMENTING');
-    expect(enqueuedKinds(queue)).toEqual(['IMPLEMENT']);
+    expect(jobs.update).toHaveBeenCalledWith('job1', { revisionCycle: { increment: 1 } });
+    expect(transitionedTo(jobs)).toContain('REVISING');
+    expect(enqueuedKinds(queue)).toEqual(['REVISE']);
   });
 
   it('setRepo updates the repo and discards the workspace while editable', async () => {

api/src/orchestrator/orchestrator.service.ts

@@ -185,7 +185,7 @@ export class OrchestratorService {
           orderBy: { createdAt: 'desc' },
           select: { phase: true, createdAt: true },
         }),
-        this.prisma.reviewPass.count({ where: { jobId: job.id, cycle: job.reviewCycle } }),
+        this.prisma.reviewPass.count({ where: { jobId: job.id, cycle: job.revisionCycle } }),
         this.prisma.queueTask.findFirst({
           where: { jobId: job.id, status: { in: ['PENDING', 'RUNNING'] } },
           orderBy: { createdAt: 'desc' },
@@ -196,7 +196,7 @@ export class OrchestratorService {
           select: { prNumber: true },
         }),
         this.prisma.reviewPass.findFirst({
-          where: { jobId: job.id, cycle: job.reviewCycle },
+          where: { jobId: job.id, cycle: job.revisionCycle },
           orderBy: { passNumber: 'desc' },
           select: { issues: true, verifyOk: true, dimensions: true },
         }),
@@ -274,9 +274,9 @@ export class OrchestratorService {
       return;
     }
 
-    // /hermes revise on a PR thread: store feedback and enqueue IMPLEMENT only when
-    // the job is parked (AWAITING_PR_APPROVAL). If an agent is already running the
-    // feedback is persisted and will be picked up on the next handleImplement call.
+    // /hermes revise on a PR thread: store feedback and open a new revision round (REVISE)
+    // only when the job is parked (AWAITING_PR_APPROVAL). If an agent is already running the
+    // feedback is persisted and folds into the in-flight round on the next revise call.
     if (command.kind === 'revise' && job.prNumber) {
       const prefix = this.config.get('COMMAND_PREFIX');
 
@@ -297,12 +297,16 @@ export class OrchestratorService {
       await this.safeCommentReaction(ref, evt.commentId, 'eyes');
 
       if (job.state === 'AWAITING_PR_APPROVAL') {
-        await this.jobs.transition(job.id, 'IMPLEMENTING', {
+        // A settled PR + new feedback = a fresh revision round: bump the round (fresh verify/
+        // review budget) and run REVISE, scoped to this feedback — not a full re-implement.
+        await this.jobs.update(job.id, { revisionCycle: { increment: 1 } });
+
+        await this.jobs.transition(job.id, 'REVISING', {
           reason: `revision requested by @${evt.author}`,
           actor: 'HUMAN',
         });
 
-        await this.queue.enqueue({ jobId: job.id, kind: 'IMPLEMENT' });
+        await this.queue.enqueue({ jobId: job.id, kind: 'REVISE' });
 
         await this.safeComment(
           ref,
@@ -410,13 +414,16 @@ export class OrchestratorService {
       });
 
       if (job.state === 'AWAITING_PR_APPROVAL') {
-        // The PR was settled — start a fresh revision cycle to address the feedback.
-        await this.jobs.transition(job.id, 'IMPLEMENTING', {
+        // The PR was settled — open a fresh revision round (fresh verify/review budget) and
+        // run REVISE, scoped to this feedback, rather than re-running the full IMPLEMENT.
+        await this.jobs.update(job.id, { revisionCycle: { increment: 1 } });
+
+        await this.jobs.transition(job.id, 'REVISING', {
           reason: `changes requested by @${evt.author}`,
           actor: 'HUMAN',
         });
 
-        await this.queue.enqueue({ jobId: job.id, kind: 'IMPLEMENT' });
+        await this.queue.enqueue({ jobId: job.id, kind: 'REVISE' });
 
         await this.safeComment(
           ref,
@@ -581,6 +588,9 @@ export class OrchestratorService {
       data: { status: 'APPROVED' },
     });
 
+    // Plan approval opens revision round 1 — the first (and only) IMPLEMENT pass.
+    await this.jobs.update(jobId, { revisionCycle: { increment: 1 } });
+
     await this.jobs.transition(jobId, 'IMPLEMENTING', {
       reason: `plan approved by ${by}`,
       actor: 'HUMAN',
@@ -700,12 +710,16 @@ export class OrchestratorService {
 
     await this.prisma.pullRequestFeedback.create({ data: { jobId, author: by, body } });
 
-    await this.jobs.transition(jobId, 'IMPLEMENTING', {
+    // Settled result + new feedback = a fresh revision round (fresh verify/review budget),
+    // addressed via a scoped REVISE rather than a full re-implement.
+    await this.jobs.update(jobId, { revisionCycle: { increment: 1 } });
+
+    await this.jobs.transition(jobId, 'REVISING', {
       reason: `changes requested by ${by}`,
       actor: 'HUMAN',
     });
 
-    await this.queue.enqueue({ jobId, kind: 'IMPLEMENT' });
+    await this.queue.enqueue({ jobId, kind: 'REVISE' });
 
     return { ok: true };
   }
@@ -955,10 +969,22 @@ export class OrchestratorService {
 
     // PR/result feedback applies once a PR exists (GitHub) or for any dashboard job, where
     // "request changes" stores feedback without a PR number. Older GitHub jobs with no PR
-    // yet have nothing to fold in.
+    // yet have nothing to fold in. Only feedback submitted *after* the last implement run is
+    // relevant — anything older was already incorporated by that pass; re-injecting it makes a
+    // fresh cycle re-attack already-resolved rounds (and mislabel them as current corrections).
+    // Mirrors the same guard in handleRevise.
     if (job.prNumber || job.origin === 'DASHBOARD') {
+      const lastImplementRun = await this.prisma.agentRun.findFirst({
+        where: { jobId, phase: 'IMPLEMENT' },
+        orderBy: { createdAt: 'desc' },
+        select: { createdAt: true },
+      });
+
       const prRevisions = await this.prisma.pullRequestFeedback.findMany({
-        where: { jobId },
+        where: {
+          jobId,
+          ...(lastImplementRun ? { createdAt: { gt: lastImplementRun.createdAt } } : {}),
+        },
         orderBy: { createdAt: 'asc' },
       });
 
@@ -1015,7 +1041,10 @@ export class OrchestratorService {
     });
 
     await this.jobs.incrementAttempts(jobId);
-    await this.jobs.update(jobId, { reviewCycle: { increment: 1 } });
+    // NB: revisionCycle is NOT incremented here. The round advances only on a human-triggered
+    // work episode (plan approval → round 1; each settled-PR changes request → +1), set at that
+    // transition. Incrementing per IMPLEMENT pass is the old coupling that forced PR feedback to
+    // route through IMPLEMENT just to get a fresh verify/review budget.
 
     await this.jobs.transition(jobId, 'VERIFYING', {
       reason: 'implementation complete',
@@ -1181,7 +1210,7 @@ export class OrchestratorService {
       branchName: this.branchFor(job),
     });
 
-    const cycle = job.reviewCycle;
+    const cycle = job.revisionCycle;
     const verifyCommand = await this.verifyCommandFor(job, ws.dir);
 
     // No automated checks in the repo (yet) — nothing to run, proceed to self-review.
@@ -1283,25 +1312,28 @@ export class OrchestratorService {
 
     const plan = await this.approvedPlan(jobId);
 
-    // PR feedback is only relevant if it was submitted after the last IMPLEMENT run;
-    // anything older was already incorporated into the code by that IMPLEMENT pass.
-    const lastImplementRun = await this.prisma.agentRun.findFirst({
-      where: { jobId, phase: 'IMPLEMENT' },
+    // PR feedback is only relevant if it was submitted after the last WORK run (IMPLEMENT or
+    // REVISE); anything older was already incorporated by that pass. Anchoring on either phase
+    // (not just IMPLEMENT) keeps each round isolated now that feedback drives REVISE, not
+    // IMPLEMENT — otherwise the anchor would freeze at the round-1 build and re-inject every
+    // prior round's feedback.
+    const lastWorkRun = await this.prisma.agentRun.findFirst({
+      where: { jobId, phase: { in: ['IMPLEMENT', 'REVISE'] } },
       orderBy: { createdAt: 'desc' },
       select: { createdAt: true },
     });
 
     const [reviewPasses, prFeedback] = await Promise.all([
       this.prisma.reviewPass.findMany({
-        where: { jobId, cycle: job.reviewCycle },
+        where: { jobId, cycle: job.revisionCycle },
         orderBy: { passNumber: 'desc' },
         take: 2,
         select: { issues: true },
       }),
       this.prisma.pullRequestFeedback.findMany({
         where: {
           jobId,
-          ...(lastImplementRun ? { createdAt: { gt: lastImplementRun.createdAt } } : {}),
+          ...(lastWorkRun ? { createdAt: { gt: lastWorkRun.createdAt } } : {}),
         },
         orderBy: { createdAt: 'asc' },
       }),
@@ -1329,7 +1361,7 @@ export class OrchestratorService {
     // If the most recent verify in this cycle failed, the build/tests are the priority
     // fix — surface their output to the revise agent.
     const lastVerify = await this.prisma.verifyRun.findFirst({
-      where: { jobId, cycle: job.reviewCycle },
+      where: { jobId, cycle: job.revisionCycle },
       orderBy: { createdAt: 'desc' },
       select: { ok: true, command: true, output: true },
     });
@@ -1410,7 +1442,7 @@ export class OrchestratorService {
     const base = ws.baseBranch;
     const plan = await this.approvedPlan(jobId);
     const maxPasses = this.review.maxPasses;
-    const cycle = job.reviewCycle;
+    const cycle = job.revisionCycle;
 
     // One ordered read of this cycle's passes backs three things: the pass number, the
     // immediately-preceding pass's issues (so the reviewer can verify each was resolved), and the
@@ -1673,7 +1705,7 @@ export class OrchestratorService {
     await this.jobs.update(jobId, { headSha });
 
     const lastReview = await this.prisma.reviewPass.findFirst({
-      where: { jobId, cycle: job.reviewCycle },
+      where: { jobId, cycle: job.revisionCycle },
       orderBy: { passNumber: 'desc' },
     });
 

app/src/components/JobCard.tsx

@@ -88,8 +88,8 @@ export default function JobCard({ job }: Props) {
         )}
       </td>
       <td class="py-3 px-4 whitespace-nowrap">
-        {job.reviewCycle > 0 ? (
-          <span class="text-xs text-zinc-400">cycle {job.reviewCycle}</span>
+        {job.revisionCycle > 0 ? (
+          <span class="text-xs text-zinc-400">revision {job.revisionCycle}</span>
         ) : (
           <span class="text-zinc-700">—</span>
         )}