diff --git a/apps/board/AGENTS.md b/apps/board/AGENTS.md index 470026e7dd..3238a1e31d 100644 --- a/apps/board/AGENTS.md +++ b/apps/board/AGENTS.md @@ -43,10 +43,12 @@ resolves run to MR in the background; the request path never waits on it). ## Seeing every face -`bun run storybook` from the repo root, then `Gates/Board/Gallery`: one -story per gate kind and variant, built from real gate shapes with invented -data (`gate-gallery.fixtures.ts`). Add a story when you add a gate kind or -variant, with invented names only (the repo is public). +`bun run storybook` from the repo root, then `Gates/Board`: four stories +(review first round, review re-review, respond plan, respond post), each +rebuilt from a real gate's shape with invented data +(`gate-stories.fixtures.ts`). Keep it to gates a real run asks; a variant +belongs in a DOM test, not a new story. Invented names only (the repo is +public). ## Respond rounds diff --git a/apps/board/package.json b/apps/board/package.json index c8c7a78508..f679f90ed9 100644 --- a/apps/board/package.json +++ b/apps/board/package.json @@ -27,6 +27,7 @@ "@mattstack/tui-kit": "workspace:*", "@pierre/diffs": "catalog:", "invadrs": "^1.3.1", + "lucide-react": "catalog:", "motion": "14.2.0", "pngjs": "^7.0.0", "react": "catalog:", diff --git a/apps/board/src/__tests__/respond-rounds.test.ts b/apps/board/src/__tests__/respond-rounds.test.ts index 24ec0c5f0a..c1703c141d 100644 --- a/apps/board/src/__tests__/respond-rounds.test.ts +++ b/apps/board/src/__tests__/respond-rounds.test.ts @@ -7,8 +7,10 @@ import { afterEach, beforeEach, describe, expect, test } from 'bun:test'; import { attachRespondRounds, deleteRespondRound, + readRespondOutcomes, readRespondRounds, recordRespondRound, + reviewerOutcome, } from '../respond-rounds.ts'; import { openStateDb } from '../state/db.ts'; @@ -31,6 +33,21 @@ describe('respond rounds', () => { expect(recordRespondRound(URL_A, 'john', 2, db)).toBe(2); }); + test('outcomes come back in round order, null where none was recorded', () => { + recordRespondRound(URL_A, 'john', 1, db); + recordRespondRound(URL_A, 'john', 2, db, 'comment'); + recordRespondRound(URL_A, 'john', 3, db, 'approve'); + const out = readRespondOutcomes(db); + expect(out.get(URL_A)).toEqual({ john: [null, 'comment', 'approve'] }); + expect( + attachRespondRounds([{ webUrl: URL_A }], readRespondRounds(db), out)[0] + ).toEqual({ + webUrl: URL_A, + respondRounds: { john: 3 }, + respondOutcomes: { john: [null, 'comment', 'approve'] }, + }); + }); + test('rounds are per person', () => { recordRespondRound(URL_A, 'john', 1, db); recordRespondRound(URL_A, 'john', 2, db); @@ -71,3 +88,38 @@ describe('respond rounds', () => { expect(out[2]).toBe(none); }); }); + +describe('the outcome a response round records', () => { + const reviews = (r: object) => ({ + reviews: { approvedBy: [], reviewers: [], ...r }, + }); + test('an approver who was never an assigned reviewer approved', () => { + expect( + reviewerOutcome( + reviews({ approvedBy: [{ username: 'renee' }] }) as never, + 'renee' + ) + ).toBe('approve'); + }); + test('an assigned reviewer whose state reads approved approved', () => { + expect( + reviewerOutcome( + reviews({ + reviewers: [{ username: 'renee', reviewState: 'APPROVED' }], + }) as never, + 'renee' + ) + ).toBe('approve'); + }); + test('anyone else commented', () => { + expect( + reviewerOutcome( + reviews({ + reviewers: [{ username: 'renee', reviewState: 'REVIEWED' }], + }) as never, + 'renee' + ) + ).toBe('comment'); + expect(reviewerOutcome(undefined, 'renee')).toBe('comment'); + }); +}); diff --git a/apps/board/src/__tests__/state-db.test.ts b/apps/board/src/__tests__/state-db.test.ts index 8977bfc8ed..12b3adf77e 100644 --- a/apps/board/src/__tests__/state-db.test.ts +++ b/apps/board/src/__tests__/state-db.test.ts @@ -132,7 +132,7 @@ describe('state db', () => { expect(again.query('SELECT COUNT(*) AS n FROM agent_states').get()).toEqual( { n: 1 } ); - expect(SCHEMA_VERSION).toBe(5); + expect(SCHEMA_VERSION).toBe(6); }); test('v5 adds respond_rounds, and a v4 db upgrades in place with its rows intact', () => { diff --git a/apps/board/src/client/board/Board.tsx b/apps/board/src/client/board/Board.tsx index cc5cb0d35c..527b25f93e 100644 --- a/apps/board/src/client/board/Board.tsx +++ b/apps/board/src/client/board/Board.tsx @@ -2161,6 +2161,7 @@ export function Board() { onAnswered={retireActiveGate} onContinue={retireActiveGate} onLostChange={lost => queue.hold(lost ? activeGateId : null)} + self={seat} readOnly={gateReadOnlyReason( queue.active.gate, queue.active.mr, diff --git a/apps/board/src/client/board/ClickToEditField.stories.tsx b/apps/board/src/client/board/ClickToEditField.stories.tsx deleted file mode 100644 index ace80a57ce..0000000000 --- a/apps/board/src/client/board/ClickToEditField.stories.tsx +++ /dev/null @@ -1,323 +0,0 @@ -import { useState, type ReactNode } from 'react'; -import type { Meta, StoryObj } from '@storybook/react-vite'; - -import { Chip, Markdown } from '@mattstack/tui-kit'; -import { registerTheme, SoribashiProvider } from '@mattstack/tui-kit/provider'; -import { tuiTheme } from '@mattstack/tui-kit/theme'; - -import '@mattstack/tui-kit/theme.css'; -import '@mattstack/tui-kit/canvas.css'; -import '../../style.css'; - -import { ClickToEditField } from './ClickToEditField.tsx'; -import { AgentGlyph, CodeGlyph, CommentGlyph, FileGlyph } from './icons.tsx'; - -/** - * First build pass of the click-to-edit comment (board.pen B26 and B27): - * every comment reads rendered, a click on its text opens the raw Markdown - * with the caret at the click, and there is no Write/Preview toggle. The - * summary card stacks bare fields so it reads as the one comment the forge - * shows; a comment from a finding edits in the summary or in its own card, - * both on the same text. Invented content only (the repo is public). - */ -function Stage({ - scheme, - children, -}: { - scheme: 'light' | 'dark'; - children: ReactNode; -}) { - return ( -
- -
{children}
- -
- ); -} - -const meta = { - title: 'Gates/Board/Click to edit', - decorators: [ - (Story, context) => ( - - - - ), - ], - parameters: { layout: 'fullscreen' }, -} satisfies Meta; - -export default meta; - -type Story = StoryObj; - -registerTheme(tuiTheme); - -interface StoryFinding { - id: string; - title: string; - body: string; - comment: string; - file?: { path: string; line: number }; -} - -const FINDINGS: StoryFinding[] = [ - { - id: 'd1', - title: 'retryPolicy retries a job that already failed for good', - body: '`retryPolicy` retries every rejection, including `PermanentJobError`, so a job that can never succeed burns all five attempts.', - comment: - "**issue:** this retries `PermanentJobError` too, so a job that can't succeed eats all five attempts before it fails. could we skip retries for that one?", - file: { path: 'src/jobs/retry-policy.ts', line: 34 }, - }, - { - id: 'd2', - title: 'The delay never backs off', - body: 'The delay is fixed at `200ms`; the spec asks for exponential backoff capped at 5s.', - comment: - '**suggestion:** the spec wants exponential backoff capped at 5s, and this is a flat `200ms`. something like `Math.min(200 * 2 ** attempt, 5000)` would cover it.', - file: { path: 'src/jobs/retry-policy.ts', line: 41 }, - }, - { - id: 'f1', - title: 'No run for the --changed flag', - body: 'The AC for `--changed` on files under `src/jobs/**` has no run in the evidence.', - comment: - '**suggestion:** thanks for the extra evidence, that covers it nicely. the one AC without a run is `--changed` picking up a file under `src/jobs/**`... would you mind grabbing one quick run of that?', - }, - { - id: 'f2', - title: 'Stale comments and a stale stat name', - body: 'Comments in `scan-jobs.ts` still describe the old single folder, and `stats.reachableInJobs` now counts both.', - comment: - "**nitpick:** a few comments in `scan-jobs.ts` still describe the old single folder: `JobRow.path` (:88) says it's relative to `src/jobs/`. `stats.reachableInJobs` (:140) counts both folders now too, so maybe rename it?", - }, - { - id: 'f3', - title: 'README is out of step with the script', - body: 'The README still names the old folder, and nothing mentions the path change.', - comment: - '**suggestion:** the `README.md` still says the scanner reads `src/jobs` (:42). anyone with a local overlay keyed on the old paths will get `merged 0/N` with no hint why. could we add a line about both? see [the overlay docs](https://example.test/docs/overlay).', - }, -]; - -function SummaryAndFindings() { - const [summary, setSummary] = useState( - 'Left a few comments, the `retryPolicy` one is the main one.' - ); - const [texts, setTexts] = useState>( - Object.fromEntries(FINDINGS.map(f => [f.id, f.comment])) - ); - const [ticked, setTicked] = useState(new Set(FINDINGS.map(f => f.id))); - const [editing, setEditing] = useState(null); - const setText = (id: string) => (t: string) => - setTexts(prev => ({ ...prev, [id]: t })); - const track = (key: string) => (on: boolean) => - setEditing(prev => (on ? key : prev === key ? null : prev)); - const folded = FINDINGS.filter(f => !f.file && ticked.has(f.id)); - return ( - <> -
- Summary comment - - - for comment - - posts on !1182 - -
-
-
-
- - {folded.map(f => ( - - ))} -
- {editing?.startsWith('summary') && ( - Markdown supported - )} -
-
- -
- Findings - - {ticked.size} of {FINDINGS.length} selected - -
-
- {FINDINGS.map(f => { - const on = ticked.has(f.id); - return ( -
- - {on && ( - <> - {f.file && ( -
-
- - - The Code - -
-
-
- - - {f.file.path} - - :{f.file.line} - - -
-
-
- )} -
-
- - - Agent Finding - -
-
- - {f.body} - -
-
-
-
- - - Comment - - - {f.file - ? 'drafted by the agent' - : editing === `summary:${f.id}` - ? 'editing in summary' - : 'posts in summary'} - -
- -
- - )} -
- ); - })} -
- - ); -} - -export const SummaryWithFindings: Story = { - name: 'Summary and finding cards', - render: () => , -}; - -function OneField() { - const [text, setText] = useState( - [ - '**question:** does the retry here back off, or hammer the queue?', - '', - '- `retryPolicy.max` is 5', - '- the delay looks fixed at 200ms', - '', - '```ts', - 'await retry(job, { max: 5, delayMs: 200 });', - '```', - '', - 'see [the retry notes](https://example.test/retry) for the old numbers.', - ].join('\n') - ); - return ; -} - -export const SingleField: Story = { - name: 'One boxed field (lists, code, a link)', - render: () => , -}; - -function EmptyField() { - const [text, setText] = useState(''); - return ( - - ); -} - -export const Empty: Story = { - name: 'An empty comment', - render: () => , -}; diff --git a/apps/board/src/client/board/ClickToEditField.tsx b/apps/board/src/client/board/ClickToEditField.tsx index 6680655e47..ab317b63c7 100644 --- a/apps/board/src/client/board/ClickToEditField.tsx +++ b/apps/board/src/client/board/ClickToEditField.tsx @@ -92,6 +92,8 @@ export function ClickToEditField({ heading, actions, badge, + footer, + lead, }: { label: string; text: string; @@ -109,6 +111,12 @@ export function ClickToEditField({ actions?: ReactNode; /** At rest, a pill on the field's top-left border, such as "Edited". */ badge?: ReactNode; + /** Controls in a strip along the field's bottom, inside its border and + outside the text, so a click there never opens the field. */ + footer?: ReactNode; + /** A line inside the box above the text, such as who is writing ("You:"). + A click on it edits the field like a click on the text. */ + lead?: ReactNode; }) { const [editing, setEditing] = useState(false); const area = useRef(null); @@ -262,6 +270,24 @@ export function ClickToEditField({ onAnimationComplete={() => setSwapping(false)} >
+ {lead && ( + // The name line is part of the field: a click there edits, + // and while editing it must not blur the textarea. +
{ + if (editing) e.preventDefault(); + }} + onClick={() => { + if (readOnly) return; + if (editing) area.current?.focus(); + else open(text.length); + }} + > + {lead} +
+ )} {editing ? ( @@ -313,6 +339,7 @@ export function ClickToEditField({
+ {footer &&
{footer}
} {hint && (editing || variant === 'plain') && ( // At rest, plain's hint is the hover "Click to edit" pill; the // textbox already says it edits, so the pill stays out of the @@ -324,6 +351,11 @@ export function ClickToEditField({ className="tui-cte-hint" aria-hidden={!editing || undefined} data-flash={editing ? undefined : (flash ?? undefined)} + // The rest pill reads like a button, so a click on it opens + // the field too, with the caret at the end. + onClick={ + editing || readOnly ? undefined : () => open(text.length) + } > {editing ? ( <> diff --git a/apps/board/src/client/board/CommentComposer.tsx b/apps/board/src/client/board/CommentComposer.tsx index bc96ab51bf..96cece4a62 100644 --- a/apps/board/src/client/board/CommentComposer.tsx +++ b/apps/board/src/client/board/CommentComposer.tsx @@ -1,37 +1,7 @@ -import { - useEffect, - useId, - useLayoutEffect, - useRef, - useState, - type CSSProperties, - type KeyboardEvent, - type ReactNode, -} from 'react'; import { flushSync } from 'react-dom'; -import { useAutoGrowTextarea } from '@mattstack/tui-kit/hooks'; -import { CommentMarkdown } from './CommentMarkdown.tsx'; import { postedText } from './respond-post.ts'; -type FieldMode = 'write' | 'preview'; - -/** The selected tab of the Write/Preview pair nearest `el`: the field's own, - found by walking out to the first ancestor that holds a tab list. */ -function fieldTab(el: HTMLElement): HTMLElement | null { - for ( - let at: HTMLElement | null = el.parentElement; - at && at.getAttribute('role') !== 'dialog'; - at = at.parentElement - ) { - const tab = at.querySelector( - '.tui-field-tabs [aria-pressed="true"]' - ); - if (tab) return tab; - } - return null; -} - /** Runs a reset that also returns its field to Write, then focuses the field's text area, which Preview had unmounted. */ function resetToWrite(button: HTMLElement, scope: string, reset: () => void) { @@ -40,151 +10,6 @@ function resetToWrite(button: HTMLElement, scope: string, reset: () => void) { part?.querySelector('textarea')?.focus(); } -/** Write and Preview over one comment field. */ -function FieldTabs({ - label, - mode, - onMode, -}: { - label: string; - mode: FieldMode; - onMode: (mode: FieldMode) => void; -}) { - return ( - - {(['write', 'preview'] as const).map(m => ( - - ))} - - ); -} - -/** A comment's text: an auto-growing text area in Write, the same text - rendered as Markdown in Preview. In Write, Tab indents by two spaces; - Shift-Tab and Escape still leave the field. */ -function CommentField({ - label, - text, - mode, - readOnly = false, - invalid = false, - describedBy, - onChange, -}: { - label: string; - text: string; - mode: FieldMode; - readOnly?: boolean; - invalid?: boolean; - describedBy?: string; - onChange: (text: string) => void; -}) { - const ref = useAutoGrowTextarea([text, mode]); - const caret = useRef(null); - // The last height the text area grew to, so Preview opens at the same - // size instead of jumping the card. - const height = useRef(null); - useEffect(() => { - if (ref.current && ref.current.offsetHeight > 0) - height.current = ref.current.offsetHeight; - }); - // The grow hook measures once per text; a width change (the sheet - // settling, a late web font) rewraps the text, so it measures again. - useEffect(() => { - const el = ref.current; - if (!el || typeof ResizeObserver === 'undefined') return; - let width = el.clientWidth; - const regrow = () => { - el.style.height = 'auto'; - el.style.height = `${el.scrollHeight + el.offsetHeight - el.clientHeight}px`; - }; - const observer = new ResizeObserver(() => { - if (el.clientWidth === width) return; - width = el.clientWidth; - regrow(); - }); - observer.observe(el); - void document.fonts?.ready.then(regrow); - return () => observer.disconnect(); - }, [ref, mode]); - useLayoutEffect(() => { - const el = ref.current; - if (caret.current === null || !el) return; - el.setSelectionRange(caret.current, caret.current); - caret.current = null; - }, [text, ref]); - const onKeyDown = (e: KeyboardEvent) => { - const el = e.currentTarget; - if (e.key === 'Escape') { - // The first Escape only leaves the field, landing on its own Write - // tab so the keyboard carries on from here: a blur would drop focus - // to the body, outside the sheet's Tab trap. The sheet's own Escape - // closes it on the next press. - e.stopPropagation(); - (fieldTab(el) ?? el.closest('[role="dialog"]'))?.focus(); - return; - } - if ( - e.key !== 'Tab' || - e.shiftKey || - e.altKey || - e.ctrlKey || - e.metaKey || - readOnly - ) - return; - e.preventDefault(); - e.stopPropagation(); - // insertText keeps the browser's undo history; a DOM without it gets - // the spaces spliced in by hand. - if (document.execCommand?.('insertText', false, ' ')) return; - const { selectionStart: start, selectionEnd: end, value } = el; - caret.current = start + 2; - onChange(`${value.slice(0, start)} ${value.slice(end)}`); - }; - if (mode === 'preview') - return ( -
- {text.trim() === '' ? ( -

Nothing to preview

- ) : ( - {text} - )} -
- ); - return ( -