-
-
Notifications
You must be signed in to change notification settings - Fork 155
fix(meetings): Copy notes copies the open template report, not the Standard note #318
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
Optic00
wants to merge
3
commits into
ruzin:main
Choose a base branch
from
Optic00:fix/copy-notes-active-report
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,88 @@ | ||
| import { describe, test, expect } from 'vitest'; | ||
| import { buildNotesCopyText, type NotesCopySections } from '@/lib/notesCopy'; | ||
|
|
||
| /** | ||
| * Unit coverage for buildNotesCopyText — the builder behind the "Copy notes" | ||
| * header action. The load-bearing case: when a generated template report is | ||
| * open, the copy must contain THAT report, not the Standard structured note | ||
| * (the on-screen content and the clipboard must never disagree). | ||
| */ | ||
|
|
||
| const sections: NotesCopySections = { | ||
| name: 'Weekly sync', | ||
| meta: 'Mon, Jun 23, 2026, 10:00 AM · 45m', | ||
| summary: 'We discussed the roadmap.', | ||
| discussionAreas: [ | ||
| { title: 'Roadmap', analysis: 'Q3 priorities agreed.' }, | ||
| { title: 'Hiring' }, | ||
| ], | ||
| keyPoints: ['Ship v2 in July'], | ||
| actionItems: ['Ben: draft the announcement'], | ||
| participants: ['Ben', 'Ruzin'], | ||
| }; | ||
|
|
||
| describe('buildNotesCopyText', () => { | ||
| test('no active report → the Standard structured note, all sections in order', () => { | ||
| const text = buildNotesCopyText(sections, null); | ||
| expect(text).toBe( | ||
| [ | ||
| 'Weekly sync', | ||
| 'Mon, Jun 23, 2026, 10:00 AM · 45m', | ||
| '', | ||
| 'SUMMARY', | ||
| 'We discussed the roadmap.', | ||
| '', | ||
| 'KEY TOPICS', | ||
| '- Roadmap: Q3 priorities agreed.', | ||
| '- Hiring', | ||
| '', | ||
| 'KEY POINTS', | ||
| '- Ship v2 in July', | ||
| '', | ||
| 'ACTION ITEMS', | ||
| '- Ben: draft the announcement', | ||
| '', | ||
| 'PARTICIPANTS', | ||
| 'Ben, Ruzin', | ||
| ].join('\n'), | ||
| ); | ||
| }); | ||
|
|
||
| test('active report → title + meta + the report markdown, no Standard sections', () => { | ||
| const text = buildNotesCopyText(sections, { | ||
| content: '## 1:1 Notes\n\n- Roadmap locked\n', | ||
| }); | ||
| expect(text).toBe( | ||
| [ | ||
| 'Weekly sync', | ||
| 'Mon, Jun 23, 2026, 10:00 AM · 45m', | ||
| '', | ||
| '## 1:1 Notes', | ||
| '', | ||
| '- Roadmap locked', | ||
| ].join('\n'), | ||
| ); | ||
| expect(text).not.toContain('SUMMARY'); | ||
| }); | ||
|
|
||
| test('empty sections are omitted entirely (no dangling headers)', () => { | ||
| const text = buildNotesCopyText( | ||
| { | ||
| name: 'Quick call', | ||
| meta: undefined, | ||
| summary: ' ', | ||
| discussionAreas: [], | ||
| keyPoints: [], | ||
| actionItems: [], | ||
| participants: [], | ||
| }, | ||
| null, | ||
| ); | ||
| expect(text).toBe('Quick call'); | ||
| }); | ||
|
|
||
| test('active report with only-whitespace content still copies just title + meta', () => { | ||
| const text = buildNotesCopyText(sections, { content: ' \n ' }); | ||
| expect(text).toBe('Weekly sync\nMon, Jun 23, 2026, 10:00 AM · 45m'); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| /** | ||
| * Builds the plain-text payload for the "Copy notes" header action in | ||
| * MeetingDetail. Pure so the report-vs-Standard selection is unit-testable: | ||
| * when a generated template report is open, the clipboard must carry that | ||
| * report's markdown, not the Standard structured note — the copy always | ||
| * matches what's on screen. | ||
| */ | ||
|
|
||
| export interface NotesCopySections { | ||
| name: string; | ||
| meta?: string; | ||
| summary?: string; | ||
| discussionAreas: { title: string; analysis?: string }[]; | ||
| keyPoints: string[]; | ||
| actionItems: string[]; | ||
| participants: string[]; | ||
| } | ||
|
|
||
| export function buildNotesCopyText( | ||
| sections: NotesCopySections, | ||
| activeReport: { content: string } | null, | ||
| ): string { | ||
| const lines: string[] = [sections.name]; | ||
| if (sections.meta) lines.push(sections.meta); | ||
|
|
||
| if (activeReport) { | ||
| const content = activeReport.content.trim(); | ||
| if (content) lines.push('', content); | ||
| return lines.join('\n'); | ||
| } | ||
|
|
||
| const summary = sections.summary?.trim(); | ||
| if (summary) { | ||
| lines.push('', 'SUMMARY', summary); | ||
| } | ||
| if (sections.discussionAreas.length) { | ||
| lines.push('', 'KEY TOPICS'); | ||
| sections.discussionAreas.forEach((a) => | ||
| lines.push(`- ${a.title}${a.analysis ? `: ${a.analysis}` : ''}`), | ||
| ); | ||
| } | ||
| if (sections.keyPoints.length) { | ||
| lines.push('', 'KEY POINTS'); | ||
| sections.keyPoints.forEach((p) => lines.push(`- ${p}`)); | ||
| } | ||
| if (sections.actionItems.length) { | ||
| lines.push('', 'ACTION ITEMS'); | ||
| sections.actionItems.forEach((a) => lines.push(`- ${a}`)); | ||
| } | ||
| if (sections.participants.length) { | ||
| lines.push('', 'PARTICIPANTS', sections.participants.join(', ')); | ||
| } | ||
| return lines.join('\n'); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,87 @@ | ||
| import { test, expect } from '../fixtures/electron'; | ||
| import type { Page } from '@playwright/test'; | ||
|
|
||
| /** | ||
| * T1 — renderer-only, mock IPC, no backend. "Copy notes" must copy whichever | ||
| * note is on screen: with a generated template report open it copies THAT | ||
| * report's markdown, not the Standard structured note (the regression: the | ||
| * clipboard silently disagreed with the displayed report). | ||
| * | ||
| * Seams (mirrors of the real ones, see app/e2e-mock-ipc.js): | ||
| * - STENOAI_E2E_SEED_MEETING=1 + STENOAI_E2E_SEED_REPORT=1 seed one known | ||
| * meeting carrying one generated report (rep_e2e_1, "Status Report"). | ||
| * - the clipboard is captured by replacing navigator.clipboard in-page (no OS | ||
| * clipboard dependency in CI), same recorder as transcript-export.t1. | ||
| */ | ||
|
|
||
| const SUMMARY_FILE = 'epsilon_summary.json'; | ||
|
|
||
| async function installClipboardRecorder(page: Page) { | ||
| await page.evaluate(() => { | ||
| const w = window as unknown as { __clipboardWrites: string[] }; | ||
| w.__clipboardWrites = []; | ||
| Object.defineProperty(navigator, 'clipboard', { | ||
| configurable: true, | ||
| value: { | ||
| writeText: (text: string) => { | ||
| w.__clipboardWrites.push(text); | ||
| return Promise.resolve(); | ||
| }, | ||
| }, | ||
| }); | ||
| }); | ||
| } | ||
|
|
||
| const clipboardWrites = (page: Page) => | ||
| page.evaluate(() => (window as unknown as { __clipboardWrites: string[] }).__clipboardWrites); | ||
|
|
||
| test('Copy notes copies the open report, and the Standard note when none is open', async ({ | ||
| launchApp, | ||
| }) => { | ||
| const { page } = await launchApp({ | ||
| mockIpc: true, | ||
| env: { STENOAI_E2E_SEED_MEETING: '1', STENOAI_E2E_SEED_REPORT: '1' }, | ||
| }); | ||
| await installClipboardRecorder(page); | ||
|
|
||
| await page.evaluate((f) => { | ||
| window.location.hash = `#/meetings/${encodeURIComponent(f)}`; | ||
| }, SUMMARY_FILE); | ||
| await expect(page.getByRole('button', { name: 'Copy notes' })).toBeVisible(); | ||
| // The seeded report shows up as a pill next to Standard. | ||
| const reportPill = page.getByTestId('report-switch').getByRole('button', { | ||
| name: /Status Report/, | ||
| }); | ||
| await expect(reportPill).toBeVisible(); | ||
|
|
||
| // Standard note open (active_report is null) → the structured-note copy. | ||
| await page.getByRole('button', { name: 'Copy notes' }).click(); | ||
| let writes = await clipboardWrites(page); | ||
| expect(writes).toHaveLength(1); | ||
| expect(writes[0]).toContain('Epsilon Planning'); | ||
| expect(writes[0]).toContain('PARTICIPANTS'); | ||
| expect(writes[0]).toContain('Alice, Bob'); | ||
| expect(writes[0]).not.toContain('## Status Report'); | ||
|
|
||
| // Open the generated report, then copy again → the report's markdown. | ||
| await reportPill.click(); | ||
| await expect(page.getByText('Pipeline healthy')).toBeVisible(); | ||
| await page.getByRole('button', { name: 'Copy notes' }).click(); | ||
| writes = await clipboardWrites(page); | ||
| expect(writes).toHaveLength(2); | ||
| expect(writes[1]).toContain('Epsilon Planning'); | ||
| expect(writes[1]).toContain('- Pipeline healthy'); | ||
| expect(writes[1]).toContain('- Next: open the reqs'); | ||
| expect(writes[1]).not.toContain('PARTICIPANTS'); | ||
| // The seeded report starts with a <think> block; the copy must strip | ||
| // reasoning like the rendered view does. | ||
| expect(writes[1]).not.toContain('secret chain of thought'); | ||
|
|
||
| // Switching back to Standard restores the structured-note copy. | ||
| await page.getByTestId('report-switch').getByRole('button', { name: 'Standard' }).click(); | ||
| await page.getByRole('button', { name: 'Copy notes' }).click(); | ||
| writes = await clipboardWrites(page); | ||
| expect(writes).toHaveLength(3); | ||
| expect(writes[2]).toContain('PARTICIPANTS'); | ||
| expect(writes[2]).not.toContain('Pipeline healthy'); | ||
| }); |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: Copy notes can show a false success state when clipboard write fails, because the handler marks
copiedimmediately instead of after a successful write. Matching the transcript copy pattern (or chaining.then) would keep feedback aligned with actual clipboard outcome.Prompt for AI agents