board: approved means GitLab says approved (#160)

* board: a fully approved roster short of a rule reads partial, not approved

Every assigned reviewer approving no longer buckets an MR as approved
while GitLab's isApproved is false. An unassigned codeowner section can
still be owed, so the pill shows the n/m partial count instead.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* board: approval counts read rule slots filled, not approvers

GitLab's required counts rule slots while given counts approvers, so one
approver on a many-section MR read 1/83. approvalSlots derives filled
from required minus remaining, and the pill, tooltip, standing line,
respond sheet and progress sort all read it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* board: an approval filling no rule slot keeps the conversation state

A roster approval that counts toward no rule no longer drops the row to
an amber needs review; it falls through to commented or comments
resolved. Base test fixtures carry remaining so no count renders NaN,
and the progress sort gets a case where approvers and slots disagree.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

Author: m4ttheweric

apps/board/src/__tests__/view.test.ts

@@ -39,7 +39,7 @@ function mr(overrides: Partial<BoardMR>): BoardMR {
     pipelineState: 'none',
     unresolvedThreads: 0,
     reviewerComments: 0,
-    reviews: { required: 2, given: 0, isApproved: false },
+    reviews: { required: 2, given: 0, remaining: 2, isApproved: false },
     blockers: {},
     autoMergeButton: { visible: false, isActive: false },
     ...overrides,
@@ -629,20 +629,59 @@ describe('sortMRs', () => {
     const list = [
       mr({
         iid: 1,
-        reviews: { required: 2, given: 0, isApproved: false } as any,
+        reviews: {
+          required: 2,
+          given: 0,
+          remaining: 2,
+          isApproved: false,
+        } as any,
       }),
       mr({
         iid: 2,
-        reviews: { required: 2, given: 2, isApproved: true } as any,
+        reviews: {
+          required: 2,
+          given: 2,
+          remaining: 0,
+          isApproved: true,
+        } as any,
       }),
       mr({
         iid: 3,
-        reviews: { required: 2, given: 1, isApproved: false } as any,
+        reviews: {
+          required: 2,
+          given: 1,
+          remaining: 1,
+          isApproved: false,
+        } as any,
       }),
     ];
     expect(sortMRs(list, 'progress').map(m => m.iid)).toEqual([2, 3, 1]);
   });
 
+  test('progress counts rule slots filled, not approvers', () => {
+    const list = [
+      mr({
+        iid: 1,
+        reviews: {
+          required: 4,
+          given: 2,
+          remaining: 2,
+          isApproved: false,
+        } as any,
+      }),
+      mr({
+        iid: 2,
+        reviews: {
+          required: 4,
+          given: 1,
+          remaining: 1,
+          isApproved: false,
+        } as any,
+      }),
+    ];
+    expect(sortMRs(list, 'progress').map(m => m.iid)).toEqual([2, 1]);
+  });
+
   test('does not mutate input', () => {
     const list = [
       mr({ iid: 1, createdAt: '2026-07-05T00:00:00Z' }),
@@ -893,22 +932,41 @@ describe('groupMRs status', () => {
     expect(groups.map(g => g.label)).toEqual(['approved']);
   });
 
-  test('every assigned reviewer approved buckets approved even short of the rule count', () => {
+  test('every assigned reviewer approved stays out of approved while a rule is short', () => {
     const list = [
       mr({
         iid: 1,
-        reviewerComments: 0,
+        reviewerComments: 2,
         threadSummary: { awaiting: 0, replied: 0, resolved: 4 },
         reviews: {
           required: 2,
           given: 1,
+          remaining: 1,
           isApproved: false,
           reviewers: [{ reviewState: 'APPROVED' }],
         } as any,
       }),
     ];
     const groups = groupMRs(list, 'status', [], NOW);
-    expect(groups.map(g => g.label)).toEqual(['approved']);
+    expect(groups.map(g => g.label)).toEqual(['needs review']);
+  });
+
+  test('an approval that fills no rule slot keeps the conversation state', () => {
+    const list = [
+      mr({
+        iid: 1,
+        reviewerComments: 2,
+        reviews: {
+          required: 2,
+          given: 1,
+          remaining: 2,
+          isApproved: false,
+          reviewers: [{ reviewState: 'APPROVED' }],
+        } as any,
+      }),
+    ];
+    const groups = groupMRs(list, 'status', [], NOW);
+    expect(groups.map(g => g.label)).toEqual(['commented']);
   });
 
   test('a reviewer still pending keeps a part-approved MR out of the approved bucket', () => {

apps/board/src/client/board/RespondSheet.tsx

@@ -19,6 +19,7 @@ import {
 import type { GateItemDisplay } from '@mattstack/gate-kit/react';
 import { Button, Chip } from '@mattstack/tui-kit';
 import type { GateRow } from '../../gates/store.ts';
+import { approvalSlots } from '../../view.ts';
 import type { BoardMRWithReview } from '../types.ts';
 import { parseGateCtx, type PlanCtx, type PostCtx } from './gate-ctx.ts';
 import type { GateFormState } from './GateForm.tsx';
@@ -444,6 +445,7 @@ function ResponseRows({
     row's status pill never disagree. */
 function MrStatusCard({ mr }: { mr: BoardMRWithReview }) {
   const b = mr.blockers;
+  const { filled, required } = approvalSlots(mr);
   const ci =
     mr.pipelineState === 'passed'
       ? { chip: 'pass', intent: 'ok' as const, text: 'pipeline passing' }
@@ -458,7 +460,7 @@ function MrStatusCard({ mr }: { mr: BoardMRWithReview }) {
       key: 'approvals',
       chip: mr.reviews.isApproved ? 'ok' : 'wait',
       intent: mr.reviews.isApproved ? ('ok' as const) : ('warn' as const),
-      text: `${mr.reviews.given} of ${mr.reviews.required} approvals`,
+      text: `${filled} of ${required} approvals`,
     },
     b.hasConflicts
       ? {

apps/board/src/client/board/__tests__/answered-sheet-dom.test.tsx

@@ -40,7 +40,7 @@ const MR = {
   author: { username: 'jvasquez', name: 'Joel Vasquez' },
   pipelineState: 'passed',
   blockers: { any: false, hasConflicts: false },
-  reviews: { isApproved: false, given: 0, required: 1 },
+  reviews: { isApproved: false, given: 0, required: 1, remaining: 1 },
 } as unknown as BoardMRWithReview;
 
 function shipped(overrides: Partial<GateRow> = {}): GateRow {

apps/board/src/client/board/__tests__/respond-header-dom.test.tsx

@@ -42,7 +42,7 @@ const MR = {
   pipelineState: 'passed',
   behindTarget: null,
   blockers: { any: false, hasConflicts: false },
-  reviews: { isApproved: false, given: 0, required: 1 },
+  reviews: { isApproved: false, given: 0, required: 1, remaining: 1 },
 } as unknown as BoardMRWithReview;
 
 const PLAN = JSON.stringify({

apps/board/src/client/board/__tests__/respond-sheet-dom.test.tsx

@@ -38,7 +38,7 @@ const MR = {
   pipelineState: 'passed',
   behindTarget: null,
   blockers: { any: false, hasConflicts: false },
-  reviews: { isApproved: false, given: 0, required: 1 },
+  reviews: { isApproved: false, given: 0, required: 1, remaining: 1 },
 } as unknown as BoardMRWithReview;
 
 const thread = (summary: string) =>

apps/board/src/client/board/__tests__/row-status.test.ts

@@ -7,6 +7,7 @@ import {
   EXECUTION_UNASSIGNED_MESSAGE,
   rowStatus,
   statusPhrase,
+  statusReasons,
 } from '../row-status.ts';
 
 const NOW = Date.parse('2026-09-12T12:00:00Z');
@@ -21,7 +22,13 @@ function mr(over: Partial<BoardMRWithReview> = {}): BoardMRWithReview {
     sourceBranch: 'feature/acme-2214',
     targetBranch: 'main',
     author: { username: 'pat', name: 'Pat' },
-    reviews: { isApproved: false, required: 1, given: 0, reviewers: [] },
+    reviews: {
+      isApproved: false,
+      required: 1,
+      given: 0,
+      remaining: 1,
+      reviewers: [],
+    },
     blockers: { any: false },
     mergeButton: { visible: false, disabled: false, loading: false },
     gates: [],
@@ -58,7 +65,13 @@ const settled = (over: Over = {}) =>
     ...over,
   } as never);
 const unapproved = (given: number, required: number) => ({
-  reviews: { isApproved: false, required, given, reviewers: [] },
+  reviews: {
+    isApproved: false,
+    required,
+    given,
+    remaining: required - given,
+    reviewers: [],
+  },
 });
 const blockedBy = (flags: Over) => ({ blockers: { any: true, ...flags } });
 const MERGEABLE = {
@@ -1592,6 +1605,40 @@ describe('statusPhrase: the pill says what the status group says', () => {
     });
   });
 
+  test('every assigned reviewer approved but a codeowner rule still open reads partial, not approved', () => {
+    const rosterApproved = {
+      reviews: {
+        isApproved: false,
+        required: 2,
+        given: 1,
+        remaining: 1,
+        reviewers: [{ username: 'pat', reviewState: 'APPROVED' }],
+      },
+    };
+    expect(statusPhrase(settled(rosterApproved))).toEqual({
+      text: '1/2 approved',
+      hue: 'cyan',
+    });
+    expect(
+      statusPhrase(settled({ ...rosterApproved, reviewerComments: 2 }))
+    ).toEqual({ text: '1/2 approved', hue: 'cyan' });
+  });
+
+  test('the count is rule slots filled, not approvers: one approver can fill many rules', () => {
+    const wide = settled({
+      reviews: {
+        isApproved: false,
+        required: 83,
+        given: 1,
+        remaining: 64,
+        reviewers: [{ username: 'pat', reviewState: 'APPROVED' }],
+      },
+      blockers: { any: true, awaitingApprovals: true },
+    });
+    expect(statusPhrase(wide)).toEqual({ text: '19/83 approved', hue: 'cyan' });
+    expect(statusReasons(wide)).toBe('blocked:\n· awaiting approvals (19/83)');
+  });
+
   test('an untouched MR is amber', () => {
     expect(statusPhrase(settled(unapproved(0, 2)))).toEqual({
       text: 'needs review',

apps/board/src/client/board/__tests__/row-view-dom.test.tsx

@@ -39,7 +39,13 @@ function mr(over: Partial<BoardMRWithReview> = {}): BoardMRWithReview {
     sourceBranch: 'feature/acme-2214',
     targetBranch: 'main',
     author: { username: 'pat', name: 'Pat' },
-    reviews: { isApproved: false, required: 1, given: 0, reviewers: [] },
+    reviews: {
+      isApproved: false,
+      required: 1,
+      given: 0,
+      remaining: 1,
+      reviewers: [],
+    },
     reviewerComments: 0,
     diff: { additions: 1455, deletions: 13, filesChanged: 4 },
     updatedAt: new Date(NOW - 32 * 3600_000).toISOString(),

apps/board/src/client/board/__tests__/stage-sheet-dom.test.tsx

@@ -41,7 +41,7 @@ const MR = {
   author: { username: 'jvasquez', name: 'Joel Vasquez' },
   pipelineState: 'passed',
   blockers: { any: false, hasConflicts: false },
-  reviews: { isApproved: false, given: 0, required: 1 },
+  reviews: { isApproved: false, given: 0, required: 1, remaining: 1 },
 } as unknown as BoardMRWithReview;
 
 function ship(overrides: Partial<GateRow> = {}): GateRow {

apps/board/src/client/board/row-status.ts

@@ -10,7 +10,7 @@ import { unwrapGateAnswer } from '@mattstack/gate-kit';
 import type { BoardMR } from '../../data.ts';
 import { hasChangesRequested } from '../../data.ts';
 import { respondOutcome } from '../../respond-outcome.ts';
-import { statusBucket } from '../../view.ts';
+import { approvalSlots, statusBucket } from '../../view.ts';
 import type {
   BoardMRWithReview,
   DoctorStatus,
@@ -89,10 +89,10 @@ export function statusReasons(mr: BoardMR): string {
   if (b.needsRebase) reasons.push('source branch needs a rebase');
   if (b.pipelineFailing) reasons.push('pipeline is failing');
   if (b.pipelineRunning) reasons.push('pipeline still running');
-  if (b.awaitingApprovals)
-    reasons.push(
-      `awaiting approvals (${mr.reviews.given}/${mr.reviews.required})`
-    );
+  if (b.awaitingApprovals) {
+    const { filled, required } = approvalSlots(mr);
+    reasons.push(`awaiting approvals (${filled}/${required})`);
+  }
   if (b.hasUnresolvedDiscussions)
     reasons.push(`unresolved discussions (${mr.unresolvedThreads})`);
   if (b.hasMergeError)
@@ -118,15 +118,9 @@ const PILL_HUE: Record<string, PillHue> = {
     approvals have got refines the untouched state rather than replacing it. */
 export function statusPhrase(mr: BoardMR): { text: string; hue: PillHue } {
   const { label } = statusBucket(mr);
-  if (
-    label === 'needs review' &&
-    mr.reviews.required > 0 &&
-    mr.reviews.given > 0
-  )
-    return {
-      text: `${mr.reviews.given}/${mr.reviews.required} approved`,
-      hue: 'cyan',
-    };
+  const { filled, required } = approvalSlots(mr);
+  if (label === 'needs review' && required > 0 && filled > 0)
+    return { text: `${filled}/${required} approved`, hue: 'cyan' };
   return { text: label, hue: PILL_HUE[label] ?? 'amber' };
 }
 
@@ -663,9 +657,9 @@ const CI_RUNNING: Candidate = {
 };
 
 function approvalsDetail(mr: BoardMRWithReview): string | undefined {
-  const { given, required } = mr.reviews;
-  return given > 0 && required > 0
-    ? `${given} of ${required} approvals`
+  const { filled, required } = approvalSlots(mr);
+  return filled > 0 && required > 0
+    ? `${filled} of ${required} approvals`
     : undefined;
 }
 

apps/board/src/view.ts

@@ -368,10 +368,21 @@ export function flattenStack<M extends BoardMR>(
   ];
 }
 
+/** GitLab counts approvals per rule, and one approver can fill several
+    rules, so `given` (approvers) and `required` (rule slots) are different
+    units. Every "n/m approved" reads this pair so the two never mix. */
+export function approvalSlots(mr: BoardMR): {
+  filled: number;
+  required: number;
+} {
+  const { required, remaining } = mr.reviews;
+  return { filled: Math.max(required - remaining, 0), required };
+}
+
 /** Approval ratio in [0,1]; used by the "progress" sort. */
 function progress(mr: BoardMR): number {
-  const req = mr.reviews.required;
-  if (req > 0) return mr.reviews.given / req;
+  const { filled, required } = approvalSlots(mr);
+  if (required > 0) return filled / required;
   return mr.reviews.given > 0 ? 1 : 0;
 }
 
@@ -493,10 +504,7 @@ function ageBucket(
   return { label: 'Older', order: 9 };
 }
 
-/** The roster's verdict, not the approval rule's arithmetic: every assigned
-    reviewer has approved. An MR in this state reads approved even while a
-    project rule still wants more approvals -- the shortfall stays visible as
-    the awaiting-approvals blocker, not as the review state. */
+/** Every assigned reviewer has approved. */
 function allReviewersApproved(mr: BoardMR): boolean {
   const reviewers = mr.reviews.reviewers ?? [];
   return (
@@ -516,8 +524,10 @@ export function statusBucket(mr: BoardMR): { label: string; order: number } {
   // not their own groups, so an MR with conflicts still shows under its review
   // state instead of being hidden in a "conflicts" bucket.
   if (hasChangesRequested(mr)) return { label: 'changes requested', order: 0 };
-  if (mr.reviews.isApproved || allReviewersApproved(mr))
-    return { label: 'approved', order: 4 };
+  if (mr.reviews.isApproved) return { label: 'approved', order: 4 };
+  // A rule no assigned reviewer covers (a codeowner section) can still be owed.
+  if (allReviewersApproved(mr) && approvalSlots(mr).filled > 0)
+    return { label: 'needs review', order: 2 };
   if (mr.reviewerComments > 0) return { label: 'commented', order: 1 };
   // Reviewed and all threads resolved, just not formally approved — further along
   // than an untouched MR, so it sits between "needs review" and "approved".