From 24596842eae17009c70fffef339da8aa35c11001 Mon Sep 17 00:00:00 2001 From: Ben Stull Date: Tue, 9 Jun 2026 04:19:20 -0700 Subject: [PATCH] =?UTF-8?q?feat(=C2=A78.12/=C2=A78.3):=20main-view=20Ask?= =?UTF-8?q?=20cuts=20an=20edit=20branch=20and=20runs=20the=20question=20th?= =?UTF-8?q?ere=20(v0.54.0)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The AI "Ask" affordance (selection tooltip + prompt bar) had no response surface on an entry's canonical `main` view: RFCView renders the human- discussion panel there, not the chat panel, so a chat turn posted from the reading view had nowhere to render — asking-while-reading silently did nothing. AI chat is an editing activity (a turn can emit proposals) and only runs on an edit branch. Option B: when Ask is invoked from main, transparently cut an edit branch via the same dispatch as Start Contributing (promote-to-branch for active, start-edit-branch for super-draft; idempotent — reuse an existing edit branch), navigate onto it so the chat panel mounts, and run the question (text + selected quote) as the branch's first chat turn once its view and main_thread_id resolve. The turn fires from the message-load effect's continuation (not a parallel effect) so a late message load can't clobber the optimistic turn; a live ref keeps the latest submitChatTurn closure in reach. Degrades gracefully: signed-out → login; a viewer who can't contribute has the cut rejected server-side and the error surfaced (no spurious branch); asking from a branch already, and Flag on main (human discussion thread), unchanged. Frontend-only; the branch chat endpoints already work on a branch. SPEC §8.3 records the behavior; new RFCView unit tests + an e2e spec cover the flow. backend 677 / frontend 66 / e2e 5 green. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 29 +++ SPEC.md | 14 ++ VERSION | 2 +- e2e/main-view-ask.spec.js | 53 ++++++ frontend/package.json | 2 +- frontend/src/components/RFCView.jsx | 62 ++++++- frontend/src/components/RFCView.test.jsx | 220 +++++++++++++++++++++++ 7 files changed, 374 insertions(+), 8 deletions(-) create mode 100644 e2e/main-view-ask.spec.js create mode 100644 frontend/src/components/RFCView.test.jsx diff --git a/CHANGELOG.md b/CHANGELOG.md index 120825d..3ea247d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,35 @@ skip versions are the composition of each intervening adjacent release's steps in order — no A-to-B path is pre-computed beyond that. +## 0.54.0 — 2026-06-09 + +**Minor — the AI "Ask" affordance now works from an entry's canonical view +(§8.12); no operator action required.** + +AI chat is an editing activity (a turn can emit proposed edits) and only runs +on an edit branch. On an entry's canonical `main` view the right column is the +human-discussion panel, not the chat panel — so invoking "Ask" (the selection +tooltip or the prompt bar) from the reading view pushed a chat turn that had +nowhere to render: it silently did nothing. Surfaced by dogfooding a non-default +project's spec entry on a live deployment. + +- **Asking while reading "just works" (Option B).** When Ask is invoked from + the canonical view, the app transparently cuts an edit branch via the same + dispatch as Start Contributing (`start-edit-branch` for a super-draft, + `promote-to-branch` for an active RFC — idempotent, so it reuses an existing + edit branch rather than double-cutting), navigates onto that branch so the + chat panel mounts, and runs the question (text + the selected quote) there + once the branch's chat thread has resolved. The viewer lands in the chat with + the streaming answer, quote intact. +- **Graceful degradation.** A signed-out viewer keeps the read-only path + (sign-in prompt; Ask disabled) and a viewer who cannot contribute has the + branch cut rejected server-side and the error surfaced — no silent no-op and + no spurious branch. Asking from a branch already, and Flag on the canonical + view (which still opens a human discussion thread), are unchanged. + +Frontend-only: the branch chat endpoints already worked on a branch. No +database migration and no configuration change. + ## 0.53.0 — 2026-06-09 **Minor — the branch/edit/body subsystem is now three-tier (project / diff --git a/SPEC.md b/SPEC.md index d86e0a8..72b28df 100644 --- a/SPEC.md +++ b/SPEC.md @@ -1022,6 +1022,20 @@ the user on it in contribute mode. New-branch naming defaults to an auto-generated value (user-renamable); the exact format is an implementation detail. +The AI chat is an editing activity — a turn can emit `` +proposals — so it runs on an edit branch, not on read-only main, whose +right column is the human-discussion surface (§8.12 is branch-scoped). +Invoking the AI **Ask** affordance (the selection tooltip's prompt, or +the prompt bar) from main therefore transparently cuts an edit branch +via the same dispatch as "Start Contributing" (promote-to-branch for an +active RFC, start-edit-branch for a super-draft per §9.5; idempotent, so +an existing edit branch is reused rather than a second one cut), lands +the user on it, and runs the question — text plus any selected quote — +as the branch's first chat turn. A viewer who cannot contribute has the +branch cut rejected and the error surfaced (no branch is created); a +signed-out viewer keeps the §8.7 read-only path. **Flag** from main is +unaffected — it opens a human discussion thread on main, not a branch. + Discuss vs. contribute is an *intent* affordance, not a *permission* affordance. A user without contribute access to a branch sees the toggle disabled, with a sign-in or request-access path (see §8.7). diff --git a/VERSION b/VERSION index 7f422a1..524456c 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.53.0 +0.54.0 diff --git a/e2e/main-view-ask.spec.js b/e2e/main-view-ask.spec.js new file mode 100644 index 0000000..8abfddb --- /dev/null +++ b/e2e/main-view-ask.spec.js @@ -0,0 +1,53 @@ +import { test, expect } from './lib/fixtures.js' +import { signIn, OWNER_EMAIL } from './lib/auth.js' +import { dismissCookies } from './lib/ui.js' + +// §8.12 Option B — asking-while-reading on the canonical `main` view. +// +// Before this fix, the AI "Ask" affordance had no chat surface on main (the +// human-discussion panel renders there, not the chat panel), so asking from the +// reading view silently did nothing. Option B transparently cuts an edit branch, +// navigates to it, and runs the question there. +// +// This spec asserts the model-independent core: the branch cut + navigation +// (old code did NOTHING here — the silent no-op), and that the question turn +// actually FIRED against the branch. The turn's outcome is environment-specific +// — PPE/prod (a model is configured) streams an answer and the optimistic +// question persists; the Tier-1 stack has no AI provider, so the turn errors +// with "Chat failed". Either outcome proves the turn fired; we match both so the +// one spec passes in both environments. The quote-passthrough + answer rendering +// are covered deterministically by the RFCView unit tests (model mocked). +// +// Driven through the prompt bar, which shares handlePrompt → handleMainAsk with +// the selection tooltip's onAsk. Runs against the faceted `bdd` collection entry +// (collection-scoped — the same three-tier path G-15 hardened). +const ENTRY = '/p/ohm/c/bdd/e/checkout-returning' + +test('Ask from the canonical view cuts an edit branch and lands the question in chat', async ({ page }) => { + await signIn(page, OWNER_EMAIL) + await page.goto(ENTRY) + await dismissCookies(page) + + // We start on the canonical (main) view: read-only, no chat surface — the + // discuss-mode banner is the canonical-view tell, and the URL has no branch. + await expect(page.locator('.discuss-mode-banner')).toContainText('read-only') + expect(new URL(page.url()).searchParams.get('branch')).toBeNull() + + // Ask a question from the prompt bar. + const question = `What problem does this entry solve? (${Date.now()})` + await page.locator('.prompt-input').fill(question) + await page.getByRole('button', { name: 'Ask', exact: true }).click() + + // Option B: an edit branch is cut and we navigate onto it (no silent no-op). + await expect(async () => { + expect(new URL(page.url()).searchParams.get('branch')).toMatch(/^edit/) + }).toPass({ timeout: 30_000 }) + + // The question turn fired against the new branch: either the optimistic + // question is showing (model answered / answering) or the turn surfaced a + // chat error (no AI provider in Tier-1). Both prove it ran — the pre-fix + // silent no-op showed neither. + await expect( + page.getByText(question).or(page.getByText(/Chat failed|No AI providers/i)), + ).toBeVisible({ timeout: 30_000 }) +}) diff --git a/frontend/package.json b/frontend/package.json index f0dacbf..5f2cc41 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,7 +1,7 @@ { "name": "rfc-app-frontend", "private": true, - "version": "0.53.0", + "version": "0.54.0", "type": "module", "scripts": { "dev": "vite", diff --git a/frontend/src/components/RFCView.jsx b/frontend/src/components/RFCView.jsx index 663ece7..fb796ad 100644 --- a/frontend/src/components/RFCView.jsx +++ b/frontend/src/components/RFCView.jsx @@ -93,6 +93,15 @@ export default function RFCView({ viewer }) { // surface to expose. const editorRef = useRef(null) const originalSourceLinesRef = useRef([]) + // §8.12 Option B — asking-while-reading on `main`. When Ask is invoked from + // the canonical view we transparently cut an edit branch, navigate to it, and + // run the question there once its view (and main_thread_id) loads. The pending + // question is stashed here and fired by the pending-ask effect below — we + // can't push optimistic chat messages before branchView.main_thread_id exists. + const pendingAskRef = useRef(null) // { text, quote, branch } | null + // Live ref to submitChatTurn so the (intentionally narrow-dep) message-load + // effect can fire the stashed §8.12 main-view ask with the latest closure. + const submitChatTurnRef = useRef(null) const [editorContent, setEditorContent] = useState('') // Mirror of the live CM6 doc for the Contribute-mode preview pane, // debounced so the preview doesn't re-render on every keystroke. @@ -237,8 +246,21 @@ export default function RFCView({ viewer }) { // Load chat messages whenever the branch's main thread id resolves. useEffect(() => { if (!branchView?.main_thread_id) return - loadAllMessages(slug, branchParam, branchView.threads).then(setMessages) - }, [branchView?.main_thread_id, slug, branchParam]) + loadAllMessages(slug, branchParam, branchView.threads).then(loaded => { + setMessages(loaded) + // §8.12 Option B — run the stashed main-view question now that the freshly + // cut branch's messages have loaded. Firing here (rather than in a parallel + // effect) appends the turn AFTER the loaded set, so this late message load + // can't clobber the optimistic chat turn. Land in contribute mode to + // surface any edits the turn proposes (AI chat is an editing activity). + const pending = pendingAskRef.current + if (pending && pending.branch === branchView.branch_name && pending.branch === branchParam) { + pendingAskRef.current = null + setMode('contribute') + submitChatTurnRef.current?.(pending.text, pending.quote) + } + }) + }, [branchView?.main_thread_id, branchView?.branch_name, slug, branchParam]) // Selection wiring (§8.12). MarkdownPreview reports {text, coords} // sourced from window.getSelection(); the SelectionTooltip @@ -335,8 +357,12 @@ export default function RFCView({ viewer }) { }, [manualCountdown, flushManualBuffer]) // ── Start contributing ───────────────────────────────────────────────── + // Returns the branch the viewer should now be on (the freshly cut branch on + // main, or the current branch on a mode-flip), or null if no branch was cut + // (signed-out → login redirect, or the server rejected the cut). §8.12's + // main-view Ask reuses this dispatch and consumes the returned branch name. const handleStartContributing = useCallback(async () => { - if (!viewer) { window.location.href = '/auth/login'; return } + if (!viewer) { window.location.href = '/auth/login'; return null } if (branchParam === 'main') { try { // §9.5 dispatch: super-drafts cut a meta-repo edit branch via @@ -345,18 +371,34 @@ export default function RFCView({ viewer }) { ? await startEditBranch(slug) : await promoteToBranch(slug) setSearchParams({ branch: branch_name }) + return branch_name } catch (err) { setError(err.message) + return null } - return } // Non-main: pure mode flip per §8.14. if (pendingDiscussChanges.length > 0) { setPendingDiscussChanges([]) } setMode('contribute') + return branchParam }, [viewer, slug, branchParam, pendingDiscussChanges, setSearchParams, isSuperDraft]) + // ── §8.12 Option B — Ask invoked from the canonical `main` view ───────── + // AI chat is an editing activity and only runs on an edit branch, so asking + // while reading transparently cuts one (same dispatch as Start Contributing), + // navigates to it, and stashes the question; the pending-ask effect fires it + // once the new branch's view (and its main_thread_id) has resolved. Degrades + // gracefully: signed-out → login; a viewer who can't contribute has the cut + // rejected server-side → the error surfaces and we neither navigate nor chat. + const handleMainAsk = useCallback(async (text, quote) => { + if (!viewer) { window.location.href = '/auth/login'; return } + const branch = await handleStartContributing() + if (!branch || branch === 'main') return // cut failed → setError already ran + pendingAskRef.current = { text, quote: quote || null, branch } + }, [viewer, handleStartContributing]) + // ── Submit a chat turn (prompt bar or selection tooltip) ─────────────── const submitChatTurn = useCallback(async (text, quote) => { if (!branchView?.main_thread_id || isStreaming) return @@ -427,16 +469,24 @@ export default function RFCView({ viewer }) { } }, [slug, branchParam, branchView?.main_thread_id, isStreaming, viewer, selectedModel, mode]) + // Keep submitChatTurnRef pointed at the latest submitChatTurn so the + // message-load effect's §8.12 main-view ask uses the current branch closure. + useEffect(() => { submitChatTurnRef.current = submitChatTurn }, [submitChatTurn]) + const handlePrompt = useCallback((text, sel) => { const quote = sel?.text || null + // On the canonical view there is no chat surface; Ask cuts an edit branch + // and runs the question there (§8.12 Option B). On a branch, ask directly. + if (branchParam === 'main') { handleMainAsk(text, quote); return } submitChatTurn(text, quote) - }, [submitChatTurn]) + }, [branchParam, handleMainAsk, submitChatTurn]) const handleTooltipAsk = useCallback(async (textOrNull, quote) => { if (textOrNull === null) { setSelection(null); return } setSelection(null) + if (branchParam === 'main') { await handleMainAsk(textOrNull, quote); return } await submitChatTurn(textOrNull, quote) - }, [submitChatTurn]) + }, [branchParam, handleMainAsk, submitChatTurn]) const handleTooltipFlag = useCallback(async (label, quote) => { if (!viewer) { window.location.href = '/auth/login'; return } diff --git a/frontend/src/components/RFCView.test.jsx b/frontend/src/components/RFCView.test.jsx new file mode 100644 index 0000000..31bb106 --- /dev/null +++ b/frontend/src/components/RFCView.test.jsx @@ -0,0 +1,220 @@ +import React from 'react' +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { render, screen, fireEvent, waitFor } from '@testing-library/react' +import { MemoryRouter, Routes, Route } from 'react-router-dom' + +// §8.12 Option B — asking-while-reading on the canonical `main` view. The AI +// "Ask" affordance (selection tooltip + prompt bar) has no chat surface on +// main (RFCDiscussionPanel renders there, not ChatPanel). These tests cover the +// transparent branch-cut: Ask on main cuts an edit branch, navigates to it, and +// runs the question there; Ask on a branch is unchanged; a rejected cut degrades +// gracefully; Flag on main still opens a discussion thread. + +const getRFC = vi.fn() +const getRFCMain = vi.fn() +const getBranch = vi.fn() +const listModels = vi.fn() +const getCollection = vi.fn() +const getThreadMessages = vi.fn() +const promoteToBranch = vi.fn() +const startEditBranch = vi.fn() +const streamChatTurn = vi.fn() +const createThread = vi.fn() + +vi.mock('../api', () => ({ + getRFC: (...a) => getRFC(...a), + getRFCMain: (...a) => getRFCMain(...a), + getBranch: (...a) => getBranch(...a), + listModels: (...a) => listModels(...a), + getCollection: (...a) => getCollection(...a), + getThreadMessages: (...a) => getThreadMessages(...a), + promoteToBranch: (...a) => promoteToBranch(...a), + startEditBranch: (...a) => startEditBranch(...a), + streamChatTurn: (...a) => streamChatTurn(...a), + createThread: (...a) => createThread(...a), + acceptChange: vi.fn(), declineChange: vi.fn(), editMetadata: vi.fn(), + manualFlush: vi.fn(), reaskChange: vi.fn(), resolveThread: vi.fn(), + setBranchVisibility: vi.fn(), claimOwnership: vi.fn(), + retireRFC: vi.fn(), unretireRFC: vi.fn(), +})) + +vi.mock('../lib/entryPaths', () => ({ + entryPath: () => '/p/ohm/c/rfc-app/e/the-entry', + entryPrPath: () => '/pr', + useProjectId: () => 'ohm', + useCollectionId: () => 'rfc-app', +})) +vi.mock('../lib/analytics', () => ({ EVENTS: { RFC_VIEWED: 'rfc_viewed' }, track: vi.fn() })) + +// Stub the heavy children; expose just the callbacks/data the flow needs. +vi.mock('./MarkdownSourceEditor.jsx', () => ({ default: () => null })) +vi.mock('./MarkdownPreview.jsx', () => ({ default: () =>
})) +vi.mock('./RFCDiscussionPanel.jsx', () => ({ default: () =>
})) +vi.mock('./ChatPanel.jsx', () => ({ + default: ({ messages }) => ( +
+ {(messages || []).map((m, i) => ( +
{m.text}
+ ))} +
+ ), +})) +vi.mock('./SelectionTooltip.jsx', () => ({ + default: ({ onAsk, onFlag, disabled }) => ( +
+ + +
+ ), +})) +vi.mock('./PromptBar.jsx', () => ({ + default: ({ onSubmit, disabled }) => ( + + ), +})) +vi.mock('./ChangePanel.jsx', () => ({ default: () => null, diffWords: () => [] })) +vi.mock('./PRModal.jsx', () => ({ default: () => null })) +vi.mock('./GraduateDialog.jsx', () => ({ default: () => null })) +vi.mock('./InvitationsModal.jsx', () => ({ default: () => null })) +vi.mock('./MetadataFieldsPanel.jsx', () => ({ default: () => null })) + +import RFCView from './RFCView.jsx' + +const ACTIVE_ENTRY = { + id: 'RFC-0001', title: 'The Entry', state: 'active', + owners: [], meta: {}, proposed_use_case: '', tags: [], +} +const MAIN_VIEW = { slug: 'the-entry', branches: [], open_prs: [] } + +function branchPayload(branch, { main_thread_id = 't-1' } = {}) { + return { + slug: 'the-entry', title: 'The Entry', branch_name: branch, body: '# Body\n', + body_sha: 'abc', main_thread_id, threads: [], changes: [], + visibility: {}, grants: [], creator: 'me', + capabilities: { can_contribute: true, can_change_branch_settings: true }, + } +} + +const VIEWER = { gitea_login: 'me', role: 'contributor' } + +function renderView(viewer = VIEWER, initialBranch = null) { + const path = initialBranch ? `/e/the-entry?branch=${initialBranch}` : '/e/the-entry' + return render( + + + } /> + + , + ) +} + +describe('RFCView — §8.12 main-view Ask (Option B)', () => { + beforeEach(() => { + vi.clearAllMocks() + getRFC.mockResolvedValue(ACTIVE_ENTRY) + getRFCMain.mockResolvedValue(MAIN_VIEW) + listModels.mockResolvedValue({ models: [{ id: 'claude' }], default: 'claude' }) + getCollection.mockResolvedValue({ fields: null }) + getThreadMessages.mockResolvedValue({ messages: [] }) + getBranch.mockImplementation((_slug, branch) => Promise.resolve(branchPayload(branch))) + promoteToBranch.mockResolvedValue({ branch_name: 'edit-me-1' }) + startEditBranch.mockResolvedValue({ branch_name: 'edit-the-entry' }) + createThread.mockResolvedValue({ thread_id: 1, message_id: 1 }) + streamChatTurn.mockImplementation(async (_slug, _branch, _threadId, { text, quote }, cb) => { + cb.onChunk?.('Here is the answer.') + cb.onChanges?.({ message_id: 42 }) + cb.onDone?.() + return { assistantId: 42, userMsgId: 41 } + }) + }) + + it('cuts an edit branch and runs the question (with the quote) there', async () => { + renderView() + await screen.findByTestId('discussion-panel') // we start on main + + fireEvent.click(screen.getByTestId('prompt-submit')) + + // The active-RFC dispatch is promote-to-branch. + await waitFor(() => expect(promoteToBranch).toHaveBeenCalledWith('the-entry')) + + // Once the new branch view loads, the turn runs there — not on main — + // with the selected quote intact. + await waitFor(() => expect(streamChatTurn).toHaveBeenCalled()) + const [slug, branch, threadId, payload] = streamChatTurn.mock.calls[0] + expect(slug).toBe('the-entry') + expect(branch).toBe('edit-me-1') + expect(threadId).toBe('t-1') + expect(payload).toMatchObject({ text: 'Why this approach?', quote: 'the selected quote' }) + + // The chat panel is now mounted (we left main) and shows the streamed answer. + const panel = await screen.findByTestId('chat-panel') + await waitFor(() => expect(panel).toHaveTextContent('Here is the answer.')) + }) + + it('uses start-edit-branch for a super-draft entry', async () => { + getRFC.mockResolvedValue({ ...ACTIVE_ENTRY, state: 'super-draft' }) + getBranch.mockImplementation((_slug, branch) => + Promise.resolve(branchPayload(branch === 'main' ? 'main' : 'edit-the-entry'))) + renderView() + await screen.findByTestId('discussion-panel') + + fireEvent.click(screen.getByTestId('tooltip-ask')) + + await waitFor(() => expect(startEditBranch).toHaveBeenCalledWith('the-entry')) + expect(promoteToBranch).not.toHaveBeenCalled() + await waitFor(() => expect(streamChatTurn).toHaveBeenCalled()) + expect(streamChatTurn.mock.calls[0][1]).toBe('edit-the-entry') + }) + + it('asking from an existing branch runs directly, without cutting a new one', async () => { + renderView(VIEWER, 'edit-me-1') + await screen.findByTestId('chat-panel') + + fireEvent.click(screen.getByTestId('prompt-submit')) + + await waitFor(() => expect(streamChatTurn).toHaveBeenCalled()) + expect(promoteToBranch).not.toHaveBeenCalled() + expect(startEditBranch).not.toHaveBeenCalled() + expect(streamChatTurn.mock.calls[0][1]).toBe('edit-me-1') + }) + + it('surfaces an error and does not chat when the branch cut is rejected', async () => { + promoteToBranch.mockRejectedValue(new Error('not a contributor')) + renderView() + await screen.findByTestId('discussion-panel') + + fireEvent.click(screen.getByTestId('prompt-submit')) + + await waitFor(() => expect(promoteToBranch).toHaveBeenCalled()) + // Explicit error path — not a silent no-op — and no spurious chat turn. + expect(await screen.findByText(/not a contributor/)).toBeInTheDocument() + expect(streamChatTurn).not.toHaveBeenCalled() + }) + + it('Flag on the canonical view still creates a discussion thread (no branch cut)', async () => { + renderView() + await screen.findByTestId('discussion-panel') + + fireEvent.click(screen.getByTestId('tooltip-flag')) + + await waitFor(() => expect(createThread).toHaveBeenCalled()) + const [, branch, body] = createThread.mock.calls[0] + expect(branch).toBe('main') + expect(body).toMatchObject({ thread_kind: 'flag' }) + expect(promoteToBranch).not.toHaveBeenCalled() + expect(streamChatTurn).not.toHaveBeenCalled() + }) + + it('signed-out viewer gets a read-only path on main — no prompt bar, Ask disabled, no cut', async () => { + renderView(null) + await screen.findByTestId('discussion-panel') + + expect(screen.queryByTestId('prompt-submit')).toBeNull() + expect(screen.getByTestId('tooltip-ask')).toBeDisabled() + expect(promoteToBranch).not.toHaveBeenCalled() + expect(startEditBranch).not.toHaveBeenCalled() + }) +})