From 7d8270770956567732e74fdeb9261e04c4b18bc3 Mon Sep 17 00:00:00 2001 From: Optic00 Date: Tue, 28 Jul 2026 14:11:37 +0200 Subject: [PATCH 01/22] feat(notes): add pure section-scoped transforms for the note body --- app/note-sections.js | 140 ++++++++++++++++++++++++++++++++++++++ app/note-sections.test.js | 103 ++++++++++++++++++++++++++++ app/package.json | 2 +- 3 files changed, 244 insertions(+), 1 deletion(-) create mode 100644 app/note-sections.js create mode 100644 app/note-sections.test.js diff --git a/app/note-sections.js b/app/note-sections.js new file mode 100644 index 00000000..106a8da4 --- /dev/null +++ b/app/note-sections.js @@ -0,0 +1,140 @@ +// Pure, section-scoped transforms over the BODY of a meeting note (.md) - +// everything after the closing frontmatter '---'. Every function replaces +// exactly one '## ' section and leaves every other byte untouched. This is the +// same contract upsertUserNotesSection (main.js) already proves for User Notes; +// this module generalises it so the note editor can patch the other sections. +// +// The canonical order is the one simple_recorder.py writes: the model's +// markdown (Summary, Key Topics, Key Points, Action Items), then Transcript, +// then User Notes. A section that does not exist yet is inserted at its +// canonical position so the file keeps the shape a user recognises. +// +// Section headings are matched case-insensitively because parseMeetingMarkdown +// lowercases them; they are WRITTEN in canonical casing. + +const SECTION_ORDER = [ + 'Summary', + 'Key Topics', + 'Key Points', + 'Action Items', + 'Participants', + 'Transcript', + 'User Notes', +]; + +// A line that would create or close a markdown section if it reached the file. +// Leading whitespace counts: parseMeetingMarkdown trims nothing on the way in, +// but a pasted " ## Transcript" is still a user trying to forge structure. +const STRUCTURAL_LINE = /^\s*#{1,6}\s/m; + +function containsStructuralLine(text) { + return STRUCTURAL_LINE.test(String(text ?? '')); +} + +function canonicalRank(heading) { + const i = SECTION_ORDER.findIndex( + (h) => h.toLowerCase() === String(heading).trim().toLowerCase(), + ); + // An unknown section sorts last so a future section we do not know about is + // never re-ordered ahead of Transcript. + return i === -1 ? SECTION_ORDER.length : i; +} + +// Split the body into ordered blocks. The first block carries any preamble +// before the first '## ' heading and has heading === null. +function splitSections(body) { + const blocks = [{ heading: null, lines: [] }]; + for (const line of String(body ?? '').split('\n')) { + // '### Topic' does NOT match: index 2 is '#', not a space. Same rule as + // parseMeetingMarkdown, so topics stay inside their parent section. + if (line.startsWith('## ')) { + blocks.push({ heading: line.slice(3).trim(), lines: [] }); + } else { + blocks[blocks.length - 1].lines.push(line); + } + } + return blocks; +} + +function joinSections(blocks) { + const out = []; + for (const block of blocks) { + if (block.heading !== null) out.push(`## ${block.heading}`); + out.push(...block.lines); + } + // Trim the tail so repeated edits cannot accrete blank lines, then restore + // exactly one terminating newline. + return `${out.join('\n').replace(/\s+$/, '')}\n`; +} + +// The body lines of a section: one blank line after the heading, the content, +// one blank line before whatever follows. +function sectionLines(content) { + return ['', ...String(content).split('\n'), '']; +} + +function setSection(body, heading, content) { + const blocks = splitSections(body); + const trimmed = String(content ?? '').replace(/\s+$/, ''); + const at = blocks.findIndex( + (b) => b.heading !== null && b.heading.toLowerCase() === heading.toLowerCase(), + ); + + if (at !== -1) { + // An empty value removes the section, matching upsertUserNotesSection. + if (!trimmed) { + blocks.splice(at, 1); + return joinSections(blocks); + } + blocks[at].lines = sectionLines(trimmed); + return joinSections(blocks); + } + + if (!trimmed) return joinSections(blocks); + + const rank = canonicalRank(heading); + let insertAt = blocks.length; + for (let i = 0; i < blocks.length; i += 1) { + if (blocks[i].heading !== null && canonicalRank(blocks[i].heading) > rank) { + insertAt = i; + break; + } + } + blocks.splice(insertAt, 0, { heading, lines: sectionLines(trimmed) }); + return joinSections(blocks); +} + +function renderBulletList(items) { + return (Array.isArray(items) ? items : []) + .map((item) => String(item).trim()) + .filter(Boolean) + .map((item) => `- ${item}`) + .join('\n'); +} + +function renderTopics(areas) { + return (Array.isArray(areas) ? areas : []) + .map((area) => { + const title = String(area && area.title ? area.title : '').trim(); + if (!title) return ''; + const analysis = String(area && area.analysis ? area.analysis : '').trim(); + return analysis ? `### ${title}\n\n${analysis}` : `### ${title}`; + }) + .filter(Boolean) + .join('\n\n'); +} + +const setSummary = (body, text) => setSection(body, 'Summary', String(text ?? '').trim()); +const setKeyPoints = (body, items) => setSection(body, 'Key Points', renderBulletList(items)); +const setActionItems = (body, items) => setSection(body, 'Action Items', renderBulletList(items)); +const setDiscussionAreas = (body, areas) => setSection(body, 'Key Topics', renderTopics(areas)); + +module.exports = { + SECTION_ORDER, + containsStructuralLine, + setSection, + setSummary, + setKeyPoints, + setActionItems, + setDiscussionAreas, +}; diff --git a/app/note-sections.test.js b/app/note-sections.test.js new file mode 100644 index 00000000..4d2eff10 --- /dev/null +++ b/app/note-sections.test.js @@ -0,0 +1,103 @@ +const { test } = require('node:test'); +const assert = require('node:assert'); + +const { + setSummary, + setKeyPoints, + setActionItems, + setDiscussionAreas, + containsStructuralLine, +} = require('./note-sections'); + +// A note body in the exact shape simple_recorder.py writes: the model's +// markdown, then ## Transcript, then ## User Notes. +const BODY = [ + '', + '## Summary', + '', + 'We agreed the budget.', + '', + '## Key Topics', + '', + '### Budget', + '', + 'Numbers were reviewed.', + '', + '## Key Points', + '', + '- Budget approved', + '', + '## Action Items', + '', + '- Anna sends the draft', + '', + '## Transcript', + '', + '[You] Hello.', + '', + '## User Notes', + '', + 'my own note', + '', +].join('\n'); + +test('setSummary replaces only the summary and leaves every other section byte-identical', () => { + const out = setSummary(BODY, 'We agreed the budget for Q3.'); + assert.match(out, /## Summary\n\nWe agreed the budget for Q3\.\n/); + assert.match(out, /## Transcript\n\n\[You\] Hello\.\n/); + assert.match(out, /## User Notes\n\nmy own note\n/); + assert.match(out, /### Budget\n\nNumbers were reviewed\.\n/); + assert.strictEqual(out.includes('We agreed the budget.\n'), false); +}); + +test('setKeyPoints rewrites the bullet list', () => { + const out = setKeyPoints(BODY, ['Budget approved', 'Anna owns the draft']); + assert.match(out, /## Key Points\n\n- Budget approved\n- Anna owns the draft\n/); + assert.match(out, /## Action Items\n\n- Anna sends the draft\n/); +}); + +test('setActionItems with an empty list removes the section entirely', () => { + const out = setActionItems(BODY, []); + assert.strictEqual(out.includes('## Action Items'), false); + assert.match(out, /## Key Points\n\n- Budget approved\n/); + assert.match(out, /## Transcript\n/); +}); + +test('setDiscussionAreas rewrites the ### topics under Key Topics', () => { + const out = setDiscussionAreas(BODY, [ + { title: 'Budget', analysis: 'Numbers were reviewed.' }, + { title: 'Hiring', analysis: 'Two roles open.' }, + ]); + assert.match(out, /## Key Topics\n\n### Budget\n\nNumbers were reviewed\.\n\n### Hiring\n\nTwo roles open\.\n/); +}); + +test('a topic without analysis renders as a bare heading', () => { + const out = setDiscussionAreas(BODY, [{ title: 'Budget' }]); + assert.match(out, /### Budget\n/); + assert.strictEqual(out.includes('undefined'), false); +}); + +test('a missing section is inserted at its canonical position, not appended', () => { + const withoutActions = setActionItems(BODY, []); + const out = setActionItems(withoutActions, ['Ben books the room']); + const actionsAt = out.indexOf('## Action Items'); + const keyPointsAt = out.indexOf('## Key Points'); + const transcriptAt = out.indexOf('## Transcript'); + assert.ok(keyPointsAt < actionsAt, 'Action Items must follow Key Points'); + assert.ok(actionsAt < transcriptAt, 'Action Items must precede Transcript'); +}); + +test('repeated edits do not accrete blank lines', () => { + let out = setSummary(BODY, 'One.'); + out = setSummary(out, 'Two.'); + out = setSummary(out, 'Three.'); + assert.strictEqual(/\n{3,}/.test(out), false); +}); + +test('containsStructuralLine catches a heading a user could paste into a field', () => { + assert.strictEqual(containsStructuralLine('## Transcript'), true); + assert.strictEqual(containsStructuralLine('ok\n### Sneaky'), true); + assert.strictEqual(containsStructuralLine(' ## indented'), true); + assert.strictEqual(containsStructuralLine('a #hashtag is fine'), false); + assert.strictEqual(containsStructuralLine('C# is fine'), false); +}); diff --git a/app/package.json b/app/package.json index 46f3463d..b05abf29 100644 --- a/app/package.json +++ b/app/package.json @@ -19,7 +19,7 @@ "typecheck:renderer": "tsc -p renderer/tsconfig.json --noEmit", "lint:renderer": "eslint --config renderer/eslint.config.mjs renderer/src", "format:renderer": "prettier --write renderer/src", - "test:unit": "node --test processing-log.test.js meeting-detect.test.js notes-file.test.js backend-stream.test.js ipc-contract.test.js shortcut-url.test.js setup-check-parse.test.js diagnostics-forward.test.js analytics-helpers.test.js live-snapshot-sweep.test.js backend-cli.test.js debug-log.test.js teardown.test.js folders-ipc.test.js settings-ipc.test.js regen-title-busy-guard.test.js update-idle-gate.test.js update-os-gate.test.js && vitest run", + "test:unit": "node --test processing-log.test.js meeting-detect.test.js notes-file.test.js note-sections.test.js backend-stream.test.js ipc-contract.test.js shortcut-url.test.js setup-check-parse.test.js diagnostics-forward.test.js analytics-helpers.test.js live-snapshot-sweep.test.js backend-cli.test.js debug-log.test.js teardown.test.js folders-ipc.test.js settings-ipc.test.js regen-title-busy-guard.test.js update-idle-gate.test.js update-os-gate.test.js && vitest run", "build": "npm run build:renderer && electron-builder", "pack:unsigned": "npm run build:renderer && electron-builder --dir --config electron-builder.ci.yml", "build-mac": "npm run build:renderer && electron-builder --mac", From db8966ab0ee30b6c0ee8fc404df6585fe0aa1c6a Mon Sep 17 00:00:00 2001 From: Optic00 Date: Tue, 28 Jul 2026 14:18:04 +0200 Subject: [PATCH 02/22] fix(notes): make duplicate-heading edits match parseMeetingMarkdown last-wins --- app/note-sections.js | 37 +++++++++--- app/note-sections.test.js | 118 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 148 insertions(+), 7 deletions(-) diff --git a/app/note-sections.js b/app/note-sections.js index 106a8da4..d8d29eb9 100644 --- a/app/note-sections.js +++ b/app/note-sections.js @@ -76,17 +76,40 @@ function sectionLines(content) { function setSection(body, heading, content) { const blocks = splitSections(body); const trimmed = String(content ?? '').replace(/\s+$/, ''); - const at = blocks.findIndex( - (b) => b.heading !== null && b.heading.toLowerCase() === heading.toLowerCase(), - ); + const matches = []; + for (let i = 0; i < blocks.length; i += 1) { + if (blocks[i].heading !== null && blocks[i].heading.toLowerCase() === heading.toLowerCase()) { + matches.push(i); + } + } - if (at !== -1) { - // An empty value removes the section, matching upsertUserNotesSection. + if (matches.length > 0) { if (!trimmed) { - blocks.splice(at, 1); + // An empty value removes the section, matching upsertUserNotesSection. + // Remove EVERY matching block, not just the last: parseMeetingMarkdown + // (main.js) walks the body and keeps overwriting sections[currentSection] + // as it goes, so it would still read a stale earlier duplicate as the + // section's content if we left one behind - resurrecting text the user + // just cleared. Splice from the highest index down so earlier removals + // don't shift the indices still queued for removal. + for (let i = matches.length - 1; i >= 0; i -= 1) { + blocks.splice(matches[i], 1); + } return joinSections(blocks); } - blocks[at].lines = sectionLines(trimmed); + // parseMeetingMarkdown's parse loop keeps overwriting + // sections[currentSection] on every '## ' heading it walks past, so when a + // heading repeats, the LAST occurrence is the one that survives into the + // parsed sections object - not the first. Write the edit at that same + // last position, then drop every earlier duplicate, so a later read + // through the parser sees exactly the value just written here. Do not + // "simplify" this to first-wins; that reintroduces the drift class + // that bit #346 and #313. + const last = matches[matches.length - 1]; + blocks[last].lines = sectionLines(trimmed); + for (let i = matches.length - 2; i >= 0; i -= 1) { + blocks.splice(matches[i], 1); + } return joinSections(blocks); } diff --git a/app/note-sections.test.js b/app/note-sections.test.js index 4d2eff10..98fd1cc8 100644 --- a/app/note-sections.test.js +++ b/app/note-sections.test.js @@ -101,3 +101,121 @@ test('containsStructuralLine catches a heading a user could paste into a field', assert.strictEqual(containsStructuralLine('a #hashtag is fine'), false); assert.strictEqual(containsStructuralLine('C# is fine'), false); }); + +// Mirrors parseMeetingMarkdown's section-splitting loop (main.js:2607-2620): it +// walks the body top to bottom and keeps overwriting sections[currentSection] +// on every '## ' heading it sees, so a repeated heading resolves to the LAST +// occurrence, not the first. Reimplemented here (rather than requiring +// main.js, which boots the Electron main process) purely to prove +// note-sections.js output stays parser-compatible on duplicate headings. +function readSectionsLikeParser(body) { + const sections = {}; + let currentSection = null; + let currentLines = []; + for (const line of body.split('\n')) { + if (line.startsWith('## ')) { + if (currentSection) sections[currentSection] = currentLines.join('\n').trim(); + currentSection = line.slice(3).trim().toLowerCase(); + currentLines = []; + } else { + currentLines.push(line); + } + } + if (currentSection) sections[currentSection] = currentLines.join('\n').trim(); + return sections; +} + +// A small local model repeating a '## Summary' heading is a realistic +// streamed-markdown output, and is exactly the parser-drift class that has +// bitten this project twice already (#346, #313). +const DUP_SUMMARY_BODY = [ + '', + '## Summary', + '', + 'First summary.', + '', + '## Summary', + '', + 'Second summary.', + '', + '## Key Points', + '', + '- Budget approved', + '', +].join('\n'); + +const DUP_ACTIONS_BODY = [ + '', + '## Key Points', + '', + '- Budget approved', + '', + '## Action Items', + '', + '- First action', + '', + '## Action Items', + '', + '- Second action', + '', + '## Transcript', + '', + '[You] Hello.', + '', +].join('\n'); + +const DUP_SUMMARY_WITH_NEIGHBOR_BODY = [ + '', + '## Summary', + '', + 'First summary.', + '', + '## Key Points', + '', + '- Point between', + '', + '## Summary', + '', + 'Second summary.', + '', + '## Transcript', + '', + '[You] Hello.', + '', +].join('\n'); + +test('a duplicate heading collapses to one, written at the LAST occurrence, matching parseMeetingMarkdown last-wins', () => { + const out = setSummary(DUP_SUMMARY_BODY, 'Third summary.'); + const headingCount = (out.match(/^## Summary$/gm) || []).length; + assert.strictEqual(headingCount, 1); + assert.match(out, /## Summary\n\nThird summary\.\n/); + assert.strictEqual(out.includes('First summary.'), false); + assert.strictEqual(out.includes('Second summary.'), false); +}); + +test('clearing a duplicated section removes every occurrence, not just the last', () => { + const out = setActionItems(DUP_ACTIONS_BODY, []); + assert.strictEqual(out.includes('## Action Items'), false); + assert.strictEqual(out.includes('First action'), false); + assert.strictEqual(out.includes('Second action'), false); + assert.match(out, /## Key Points\n\n- Budget approved\n/); + assert.match(out, /## Transcript\n/); +}); + +test('a duplicate-heading edit is what the parser actually reads back', () => { + const out = setSummary(DUP_SUMMARY_BODY, 'Edited summary.'); + const sections = readSectionsLikeParser(out); + assert.strictEqual(sections.summary, 'Edited summary.'); +}); + +test('the surviving block keeps the LAST occurrence position relative to its neighbours', () => { + const out = setSummary(DUP_SUMMARY_WITH_NEIGHBOR_BODY, 'Edited summary.'); + const summaryAt = out.indexOf('## Summary'); + const keyPointsAt = out.indexOf('## Key Points'); + const transcriptAt = out.indexOf('## Transcript'); + assert.ok( + keyPointsAt < summaryAt, + 'the surviving Summary must sit after Key Points, where the LAST occurrence was', + ); + assert.ok(summaryAt < transcriptAt); +}); From b9296fa86436f7878bb9ae2bb1c8a70eff1dc725 Mon Sep 17 00:00:00 2001 From: Optic00 Date: Tue, 28 Jul 2026 14:21:42 +0200 Subject: [PATCH 03/22] feat(notes): add an atomic file write helper --- app/atomic-write.js | 26 +++++++++++++++++++++++++ app/atomic-write.test.js | 42 ++++++++++++++++++++++++++++++++++++++++ app/package.json | 2 +- 3 files changed, 69 insertions(+), 1 deletion(-) create mode 100644 app/atomic-write.js create mode 100644 app/atomic-write.test.js diff --git a/app/atomic-write.js b/app/atomic-write.js new file mode 100644 index 00000000..95a1b3cb --- /dev/null +++ b/app/atomic-write.js @@ -0,0 +1,26 @@ +const fs = require('fs'); +const path = require('path'); +const crypto = require('crypto'); + +// Write via a temp file in the SAME directory, then rename. rename(2) is atomic +// within a filesystem, so a reader either sees the old file or the new one and +// never a truncated one. The temp name is randomised so two concurrent writers +// cannot collide, and it is dot-prefixed so a directory listing stays clean. +function writeFileAtomicSync(targetPath, data) { + const dir = path.dirname(targetPath); + const base = path.basename(targetPath); + const tmpPath = path.join(dir, `.${base}.${crypto.randomBytes(6).toString('hex')}.tmp`); + try { + fs.writeFileSync(tmpPath, data, 'utf8'); + fs.renameSync(tmpPath, targetPath); + } catch (err) { + try { + fs.unlinkSync(tmpPath); + } catch (_) { + // The temp file may never have been created. Nothing to clean up. + } + throw err; + } +} + +module.exports = { writeFileAtomicSync }; diff --git a/app/atomic-write.test.js b/app/atomic-write.test.js new file mode 100644 index 00000000..50aa54d7 --- /dev/null +++ b/app/atomic-write.test.js @@ -0,0 +1,42 @@ +const { test } = require('node:test'); +const assert = require('node:assert'); +const fs = require('fs'); +const os = require('os'); +const path = require('path'); + +const { writeFileAtomicSync } = require('./atomic-write'); + +function tmpFile(contents) { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'atomic-write-')); + const file = path.join(dir, 'note.md'); + fs.writeFileSync(file, contents, 'utf8'); + return file; +} + +test('writeFileAtomicSync replaces the file contents', () => { + const file = tmpFile('old'); + writeFileAtomicSync(file, 'new'); + assert.strictEqual(fs.readFileSync(file, 'utf8'), 'new'); +}); + +test('writeFileAtomicSync creates a file that does not exist yet', () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'atomic-write-')); + const file = path.join(dir, 'fresh.md'); + writeFileAtomicSync(file, 'hello'); + assert.strictEqual(fs.readFileSync(file, 'utf8'), 'hello'); +}); + +test('a failed write leaves the original intact and removes the temp file', () => { + const file = tmpFile('original'); + const dir = path.dirname(file); + const before = fs.readdirSync(dir); + + assert.throws(() => { + // A directory cannot be written as file data; the write fails after the + // temp file has been created, which is exactly the window under test. + writeFileAtomicSync(file, { toString() { throw new Error('boom'); } }); + }); + + assert.strictEqual(fs.readFileSync(file, 'utf8'), 'original'); + assert.deepStrictEqual(fs.readdirSync(dir).sort(), before.sort()); +}); diff --git a/app/package.json b/app/package.json index b05abf29..10327ac4 100644 --- a/app/package.json +++ b/app/package.json @@ -19,7 +19,7 @@ "typecheck:renderer": "tsc -p renderer/tsconfig.json --noEmit", "lint:renderer": "eslint --config renderer/eslint.config.mjs renderer/src", "format:renderer": "prettier --write renderer/src", - "test:unit": "node --test processing-log.test.js meeting-detect.test.js notes-file.test.js note-sections.test.js backend-stream.test.js ipc-contract.test.js shortcut-url.test.js setup-check-parse.test.js diagnostics-forward.test.js analytics-helpers.test.js live-snapshot-sweep.test.js backend-cli.test.js debug-log.test.js teardown.test.js folders-ipc.test.js settings-ipc.test.js regen-title-busy-guard.test.js update-idle-gate.test.js update-os-gate.test.js && vitest run", + "test:unit": "node --test processing-log.test.js meeting-detect.test.js notes-file.test.js note-sections.test.js atomic-write.test.js backend-stream.test.js ipc-contract.test.js shortcut-url.test.js setup-check-parse.test.js diagnostics-forward.test.js analytics-helpers.test.js live-snapshot-sweep.test.js backend-cli.test.js debug-log.test.js teardown.test.js folders-ipc.test.js settings-ipc.test.js regen-title-busy-guard.test.js update-idle-gate.test.js update-os-gate.test.js && vitest run", "build": "npm run build:renderer && electron-builder", "pack:unsigned": "npm run build:renderer && electron-builder --dir --config electron-builder.ci.yml", "build-mac": "npm run build:renderer && electron-builder --mac", From 648aa8c6f578350a87e6df53f69eeb483ea452e2 Mon Sep 17 00:00:00 2001 From: Optic00 Date: Tue, 28 Jul 2026 14:27:36 +0200 Subject: [PATCH 04/22] fix(notes): make the atomic-write failure test exercise the real cleanup path The previous test's forced failure (a toString() that throws) happened during fs's argument validation, before the temp file was ever created, so the directory-listing assertion passed vacuously - it would have passed even with the cleanup unlinkSync deleted. Mock fs.renameSync to throw instead, so the failure happens after the temp file genuinely exists on disk and the real cleanup path runs. --- app/atomic-write.test.js | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/app/atomic-write.test.js b/app/atomic-write.test.js index 50aa54d7..2c62b82e 100644 --- a/app/atomic-write.test.js +++ b/app/atomic-write.test.js @@ -26,15 +26,23 @@ test('writeFileAtomicSync creates a file that does not exist yet', () => { assert.strictEqual(fs.readFileSync(file, 'utf8'), 'hello'); }); -test('a failed write leaves the original intact and removes the temp file', () => { +test('a failed write leaves the original intact and removes the temp file', (t) => { const file = tmpFile('original'); const dir = path.dirname(file); const before = fs.readdirSync(dir); + // Force the failure AFTER the temp file has genuinely been written to disk, + // by making the rename step itself throw. This is the real window the + // atomic-write guarantee protects: the temp file exists, the rename fails, + // and the catch block's cleanup must remove it without touching the + // original. Using node:test's built-in mock keeps this dependency-free and + // restores fs.renameSync automatically once the test ends. + t.mock.method(fs, 'renameSync', () => { + throw new Error('boom'); + }); + assert.throws(() => { - // A directory cannot be written as file data; the write fails after the - // temp file has been created, which is exactly the window under test. - writeFileAtomicSync(file, { toString() { throw new Error('boom'); } }); + writeFileAtomicSync(file, 'new'); }); assert.strictEqual(fs.readFileSync(file, 'utf8'), 'original'); From b6221ccb6c4eccbc1c3f0d9368856405b3b4a61a Mon Sep 17 00:00:00 2001 From: Optic00 Date: Tue, 28 Jul 2026 14:31:48 +0200 Subject: [PATCH 05/22] feat(notes): add the original-output snapshot sidecar --- app/note-snapshot.js | 66 ++++++++++++++++++++++++++++++++++ app/note-snapshot.test.js | 76 +++++++++++++++++++++++++++++++++++++++ app/package.json | 2 +- 3 files changed, 143 insertions(+), 1 deletion(-) create mode 100644 app/note-snapshot.js create mode 100644 app/note-snapshot.test.js diff --git a/app/note-snapshot.js b/app/note-snapshot.js new file mode 100644 index 00000000..d3a460d5 --- /dev/null +++ b/app/note-snapshot.js @@ -0,0 +1,66 @@ +const fs = require('fs'); +const { writeFileAtomicSync } = require('./atomic-write'); + +// The model's original output for one note, kept in a sidecar rather than in +// the note itself because reprocess rebuilds the note file completely +// (simple_recorder.py). Without this file there is no diff base: a regenerate +// would silently destroy user corrections, and no later learning mechanism +// could tell a correction from the text it replaced. +const SNAPSHOT_VERSION = 1; + +function noteSnapshotPath(summaryPath) { + return String(summaryPath).replace(/_summary\.md$/, '_original.json'); +} + +function readSnapshot(summaryPath) { + const file = noteSnapshotPath(summaryPath); + try { + if (!fs.existsSync(file)) return null; + const parsed = JSON.parse(fs.readFileSync(file, 'utf8')); + // A snapshot from a future version is not ours to interpret. + if (!parsed || parsed.version !== SNAPSHOT_VERSION) return null; + return parsed; + } catch (_) { + // A corrupt sidecar must never break opening a note. It reads as absent, + // which costs the learning signal for this note and nothing else. + return null; + } +} + +// `capture` is 'generation' when Python or main snapshots freshly generated +// output, and 'first_edit' when main snapshots a pre-existing note the moment +// the user first edits it. The second is accurate only because the note was +// never editable before this feature; later consumers should treat it as +// slightly weaker evidence. +function captureSnapshot(summaryPath, fields, capture) { + const existing = readSnapshot(summaryPath); + if (existing) return existing; + const snapshot = { + version: SNAPSHOT_VERSION, + captured_at: new Date().toISOString(), + capture: capture === 'generation' ? 'generation' : 'first_edit', + original: { + summary: fields.summary ?? '', + key_points: fields.key_points ?? [], + action_items: fields.action_items ?? [], + discussion_areas: fields.discussion_areas ?? [], + participants: fields.participants ?? [], + }, + edited_fields: [], + edited_at: null, + }; + writeFileAtomicSync(noteSnapshotPath(summaryPath), JSON.stringify(snapshot, null, 2)); + return snapshot; +} + +function markEdited(summaryPath, changedFields) { + const snapshot = readSnapshot(summaryPath); + if (!snapshot) return null; + const merged = new Set([...(snapshot.edited_fields || []), ...(changedFields || [])]); + snapshot.edited_fields = [...merged]; + snapshot.edited_at = new Date().toISOString(); + writeFileAtomicSync(noteSnapshotPath(summaryPath), JSON.stringify(snapshot, null, 2)); + return snapshot; +} + +module.exports = { noteSnapshotPath, readSnapshot, captureSnapshot, markEdited, SNAPSHOT_VERSION }; diff --git a/app/note-snapshot.test.js b/app/note-snapshot.test.js new file mode 100644 index 00000000..1898bb94 --- /dev/null +++ b/app/note-snapshot.test.js @@ -0,0 +1,76 @@ +const { test } = require('node:test'); +const assert = require('node:assert'); +const fs = require('fs'); +const os = require('os'); +const path = require('path'); + +const { + noteSnapshotPath, + readSnapshot, + captureSnapshot, + markEdited, +} = require('./note-snapshot'); + +function tmpNote() { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'note-snapshot-')); + const file = path.join(dir, 'Weekly_Sync_summary.md'); + fs.writeFileSync(file, '---\ntitle: "Weekly Sync"\n---\n\n## Summary\n\nhi\n', 'utf8'); + return file; +} + +const FIELDS = { + summary: 'We agreed the budget.', + key_points: ['Budget approved'], + action_items: ['Anna sends the draft'], + discussion_areas: [{ title: 'Budget', analysis: 'Reviewed.' }], + participants: ['Anna'], +}; + +test('noteSnapshotPath swaps the _summary.md suffix for _original.json', () => { + assert.strictEqual( + path.basename(noteSnapshotPath('/x/Weekly_Sync_summary.md')), + 'Weekly_Sync_original.json', + ); +}); + +test('readSnapshot returns null when no sidecar exists', () => { + assert.strictEqual(readSnapshot(tmpNote()), null); +}); + +test('captureSnapshot writes the original fields and is readable back', () => { + const note = tmpNote(); + captureSnapshot(note, FIELDS, 'generation'); + const back = readSnapshot(note); + assert.strictEqual(back.version, 1); + assert.strictEqual(back.capture, 'generation'); + assert.deepStrictEqual(back.original, FIELDS); + assert.deepStrictEqual(back.edited_fields, []); +}); + +test('captureSnapshot never overwrites an existing snapshot', () => { + const note = tmpNote(); + captureSnapshot(note, FIELDS, 'generation'); + captureSnapshot(note, { ...FIELDS, summary: 'DIFFERENT' }, 'first_edit'); + assert.strictEqual(readSnapshot(note).original.summary, 'We agreed the budget.'); + assert.strictEqual(readSnapshot(note).capture, 'generation'); +}); + +test('markEdited accumulates field names without duplicates and stamps a time', () => { + const note = tmpNote(); + captureSnapshot(note, FIELDS, 'generation'); + markEdited(note, ['summary']); + const back = markEdited(note, ['summary', 'action_items']); + assert.deepStrictEqual(back.edited_fields.sort(), ['action_items', 'summary']); + assert.ok(back.edited_at); +}); + +test('a corrupt sidecar reads as null rather than throwing', () => { + const note = tmpNote(); + fs.writeFileSync(noteSnapshotPath(note), '{ not json', 'utf8'); + assert.strictEqual(readSnapshot(note), null); +}); + +test('markEdited on a note with no snapshot is a no-op that does not throw', () => { + const note = tmpNote(); + assert.strictEqual(markEdited(note, ['summary']), null); +}); diff --git a/app/package.json b/app/package.json index 10327ac4..90614f4e 100644 --- a/app/package.json +++ b/app/package.json @@ -19,7 +19,7 @@ "typecheck:renderer": "tsc -p renderer/tsconfig.json --noEmit", "lint:renderer": "eslint --config renderer/eslint.config.mjs renderer/src", "format:renderer": "prettier --write renderer/src", - "test:unit": "node --test processing-log.test.js meeting-detect.test.js notes-file.test.js note-sections.test.js atomic-write.test.js backend-stream.test.js ipc-contract.test.js shortcut-url.test.js setup-check-parse.test.js diagnostics-forward.test.js analytics-helpers.test.js live-snapshot-sweep.test.js backend-cli.test.js debug-log.test.js teardown.test.js folders-ipc.test.js settings-ipc.test.js regen-title-busy-guard.test.js update-idle-gate.test.js update-os-gate.test.js && vitest run", + "test:unit": "node --test processing-log.test.js meeting-detect.test.js notes-file.test.js note-sections.test.js atomic-write.test.js note-snapshot.test.js backend-stream.test.js ipc-contract.test.js shortcut-url.test.js setup-check-parse.test.js diagnostics-forward.test.js analytics-helpers.test.js live-snapshot-sweep.test.js backend-cli.test.js debug-log.test.js teardown.test.js folders-ipc.test.js settings-ipc.test.js regen-title-busy-guard.test.js update-idle-gate.test.js update-os-gate.test.js && vitest run", "build": "npm run build:renderer && electron-builder", "pack:unsigned": "npm run build:renderer && electron-builder --dir --config electron-builder.ci.yml", "build-mac": "npm run build:renderer && electron-builder --mac", From 286f9f0095b1ada2c4da1b4278a1a16b76fde675 Mon Sep 17 00:00:00 2001 From: Optic00 Date: Tue, 28 Jul 2026 14:38:29 +0200 Subject: [PATCH 06/22] fix(notes): close snapshot-sidecar overwrite and no-op gaps from review - noteSnapshotPath now throws for a path not ending in _summary.md instead of silently returning it unchanged, which previously let write calls clobber the note file itself. - captureSnapshot now bases its write decision on file existence, not on whether readSnapshot can interpret the content, so a sidecar written by a newer app version is never mistaken for absent and overwritten. - markEdited documents its single-writer assumption, since a later task adds a second (Python) writer to this sidecar. - markEdited's no-op test now also asserts no sidecar file is created. --- app/note-snapshot.js | 43 +++++++++++++++++++++++++++++++++++---- app/note-snapshot.test.js | 15 ++++++++++++++ 2 files changed, 54 insertions(+), 4 deletions(-) diff --git a/app/note-snapshot.js b/app/note-snapshot.js index d3a460d5..85d0cb43 100644 --- a/app/note-snapshot.js +++ b/app/note-snapshot.js @@ -9,7 +9,16 @@ const { writeFileAtomicSync } = require('./atomic-write'); const SNAPSHOT_VERSION = 1; function noteSnapshotPath(summaryPath) { - return String(summaryPath).replace(/_summary\.md$/, '_original.json'); + const str = String(summaryPath); + // String.replace returns its input unchanged when the pattern does not + // match. Without this check, a caller bug that passes a path not ending in + // "_summary.md" would silently get that same path back, and the write + // functions below would then overwrite the note itself with snapshot JSON + // instead of writing a sidecar next to it. Fail loudly here instead. + if (!str.endsWith('_summary.md')) { + throw new Error(`noteSnapshotPath: expected a path ending in "_summary.md", got: ${str}`); + } + return str.replace(/_summary\.md$/, '_original.json'); } function readSnapshot(summaryPath) { @@ -33,8 +42,21 @@ function readSnapshot(summaryPath) { // never editable before this feature; later consumers should treat it as // slightly weaker evidence. function captureSnapshot(summaryPath, fields, capture) { - const existing = readSnapshot(summaryPath); - if (existing) return existing; + const file = noteSnapshotPath(summaryPath); + // Whether we may write depends on whether a sidecar FILE is already there, + // not on whether readSnapshot can make sense of it. readSnapshot returns + // null both for "nothing here" and for "something here we don't + // understand" (corrupt JSON, or a future version's format) - collapsing + // that distinction is correct for a reader, which just wants "no usable + // snapshot", but wrong for a writer. If we wrote whenever readSnapshot + // returned null, a sidecar from a newer app version would look absent and + // get silently replaced with a version-1 snapshot, destroying whatever the + // newer format held. So: file exists -> never write, return whatever + // readSnapshot makes of it (the valid snapshot, or null if unreadable). + // Only a genuinely absent file is safe to create. + if (fs.existsSync(file)) { + return readSnapshot(summaryPath); + } const snapshot = { version: SNAPSHOT_VERSION, captured_at: new Date().toISOString(), @@ -49,10 +71,23 @@ function captureSnapshot(summaryPath, fields, capture) { edited_fields: [], edited_at: null, }; - writeFileAtomicSync(noteSnapshotPath(summaryPath), JSON.stringify(snapshot, null, 2)); + writeFileAtomicSync(file, JSON.stringify(snapshot, null, 2)); return snapshot; } +// Read-modify-write with no locking or compare-and-swap: this is safe only +// because every caller today runs synchronously inside the single Electron +// main process, so two calls can never interleave. That assumption breaks +// the day the Python side also writes this sidecar (a later task in this +// plan) - two processes racing this read-modify-write could each read the +// same on-disk snapshot, and whichever writes second silently discards the +// other's edited_fields/edited_at update, with no error and no way to tell +// afterwards that data was lost. A future cross-process caller must not +// discover this by losing an edit; it needs either to funnel writes back +// through the main process (keeping the single-writer property) or to add +// real concurrency control (a file lock, or a compare-and-swap on version / +// edited_at before writing) - not attempted here because nothing today +// exercises the concurrent path. function markEdited(summaryPath, changedFields) { const snapshot = readSnapshot(summaryPath); if (!snapshot) return null; diff --git a/app/note-snapshot.test.js b/app/note-snapshot.test.js index 1898bb94..fd7ddab6 100644 --- a/app/note-snapshot.test.js +++ b/app/note-snapshot.test.js @@ -33,6 +33,10 @@ test('noteSnapshotPath swaps the _summary.md suffix for _original.json', () => { ); }); +test('noteSnapshotPath throws for a path that does not end in _summary.md', () => { + assert.throws(() => noteSnapshotPath('/x/Weekly_Sync.md'), /_summary\.md/); +}); + test('readSnapshot returns null when no sidecar exists', () => { assert.strictEqual(readSnapshot(tmpNote()), null); }); @@ -55,6 +59,16 @@ test('captureSnapshot never overwrites an existing snapshot', () => { assert.strictEqual(readSnapshot(note).capture, 'generation'); }); +test('captureSnapshot does not clobber a sidecar written by a newer version', () => { + const note = tmpNote(); + const file = noteSnapshotPath(note); + const future = JSON.stringify({ version: 999, future: true }, null, 2); + fs.writeFileSync(file, future, 'utf8'); + const result = captureSnapshot(note, FIELDS, 'generation'); + assert.strictEqual(result, null); + assert.strictEqual(fs.readFileSync(file, 'utf8'), future); +}); + test('markEdited accumulates field names without duplicates and stamps a time', () => { const note = tmpNote(); captureSnapshot(note, FIELDS, 'generation'); @@ -73,4 +87,5 @@ test('a corrupt sidecar reads as null rather than throwing', () => { test('markEdited on a note with no snapshot is a no-op that does not throw', () => { const note = tmpNote(); assert.strictEqual(markEdited(note, ['summary']), null); + assert.strictEqual(fs.existsSync(noteSnapshotPath(note)), false); }); From 73da9b9273048fd0f2fee45439edf57d718a69e2 Mon Sep 17 00:00:00 2001 From: Optic00 Date: Tue, 28 Jul 2026 14:43:54 +0200 Subject: [PATCH 07/22] fix(notes): restore readSnapshot's never-throws contract Moving noteSnapshotPath's validation into readSnapshot's try block in the previous round fixed the write-side clobber risk but broke the read-side promise that a malformed path never throws when opening a note. Reads now resolve a malformed path the same way as a missing or corrupt sidecar file (return null); writes (captureSnapshot, markEdited) still throw, since markEdited now derives its path directly instead of only through readSnapshot. --- app/note-snapshot.js | 23 +++++++++++++++++++---- app/note-snapshot.test.js | 12 ++++++++++++ 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/app/note-snapshot.js b/app/note-snapshot.js index 85d0cb43..7d2c9396 100644 --- a/app/note-snapshot.js +++ b/app/note-snapshot.js @@ -21,17 +21,26 @@ function noteSnapshotPath(summaryPath) { return str.replace(/_summary\.md$/, '_original.json'); } +// Reads never throw; writes fail loudly. This asymmetry is deliberate, not an +// oversight: a read backs "open this note", which must never fail because of +// its sidecar, while a write backs "create/update this sidecar", where +// silently swallowing a malformed path risks the write landing on the note +// itself (see noteSnapshotPath). Do not "fix" this back into symmetry - +// noteSnapshotPath is called inside the try below (not before it) precisely +// so a malformed summaryPath is caught here and returns null, exactly like a +// missing or corrupt sidecar file. function readSnapshot(summaryPath) { - const file = noteSnapshotPath(summaryPath); try { + const file = noteSnapshotPath(summaryPath); if (!fs.existsSync(file)) return null; const parsed = JSON.parse(fs.readFileSync(file, 'utf8')); // A snapshot from a future version is not ours to interpret. if (!parsed || parsed.version !== SNAPSHOT_VERSION) return null; return parsed; } catch (_) { - // A corrupt sidecar must never break opening a note. It reads as absent, - // which costs the learning signal for this note and nothing else. + // A corrupt sidecar, an unreadable file, or a malformed summaryPath must + // never break opening a note. All read as absent, which costs the + // learning signal for this note and nothing else. return null; } } @@ -89,12 +98,18 @@ function captureSnapshot(summaryPath, fields, capture) { // edited_at before writing) - not attempted here because nothing today // exercises the concurrent path. function markEdited(summaryPath, changedFields) { + // noteSnapshotPath is called directly (not only via readSnapshot below) so + // a malformed summaryPath throws here, before anything else runs. readSnapshot + // catches that same throw internally and would otherwise turn it into a + // silent null, which would make this write-path function swallow a caller + // bug instead of surfacing it - the opposite of the intended asymmetry. + const file = noteSnapshotPath(summaryPath); const snapshot = readSnapshot(summaryPath); if (!snapshot) return null; const merged = new Set([...(snapshot.edited_fields || []), ...(changedFields || [])]); snapshot.edited_fields = [...merged]; snapshot.edited_at = new Date().toISOString(); - writeFileAtomicSync(noteSnapshotPath(summaryPath), JSON.stringify(snapshot, null, 2)); + writeFileAtomicSync(file, JSON.stringify(snapshot, null, 2)); return snapshot; } diff --git a/app/note-snapshot.test.js b/app/note-snapshot.test.js index fd7ddab6..b8e4fa81 100644 --- a/app/note-snapshot.test.js +++ b/app/note-snapshot.test.js @@ -89,3 +89,15 @@ test('markEdited on a note with no snapshot is a no-op that does not throw', () assert.strictEqual(markEdited(note, ['summary']), null); assert.strictEqual(fs.existsSync(noteSnapshotPath(note)), false); }); + +test('readSnapshot returns null for a malformed note path instead of throwing', () => { + assert.strictEqual(readSnapshot('/x/not-a-note.txt'), null); +}); + +test('captureSnapshot still throws for a malformed note path', () => { + assert.throws(() => captureSnapshot('/x/not-a-note.txt', FIELDS, 'generation'), /_summary\.md/); +}); + +test('markEdited still throws for a malformed note path', () => { + assert.throws(() => markEdited('/x/not-a-note.txt', ['summary']), /_summary\.md/); +}); From 786985808c2973b4f25df79cee8673760ad3e557 Mon Sep 17 00:00:00 2001 From: Optic00 Date: Tue, 28 Jul 2026 14:55:13 +0200 Subject: [PATCH 08/22] feat(notes): patch summary sections on markdown notes atomically --- app/main.js | 158 +++++++++++++++++++++++++++++++++++- app/renderer/src/lib/ipc.ts | 18 +++- 2 files changed, 172 insertions(+), 4 deletions(-) diff --git a/app/main.js b/app/main.js index f8b9df6b..df412aad 100644 --- a/app/main.js +++ b/app/main.js @@ -62,6 +62,15 @@ const processingLog = require('./processing-log'); const { isMeetingApp, allowsDeviceLevelFallback, isMacos14Plus } = require('./meeting-detect'); const { sweepOrphanedLiveSnapshots } = require('./live-snapshot-sweep'); const { userNotesFilePath } = require('./notes-file'); +const { + setSummary, + setKeyPoints, + setActionItems, + setDiscussionAreas, + containsStructuralLine, +} = require('./note-sections'); +const { writeFileAtomicSync } = require('./atomic-write'); +const { readSnapshot, captureSnapshot, markEdited } = require('./note-snapshot'); const { makeLineReader } = require('./backend-stream'); // Pure deep-link (stenoai://) parsing/sanitizing lives in ./shortcut-url // (unit-tested). The stateful side — window creation, IPC dispatch, @@ -3921,6 +3930,12 @@ function upsertUserNotesSection(body, notes) { } ipcMain.handle('update-meeting', async (event, summaryFilePath, updates) => { + // Which note content fields this call rewrote. Declared at handler scope, not + // inside the markdown branch, so the single write site further down can record + // the edit without guessing whether the variable exists. Only the markdown + // branch fills it: the legacy .json format has no snapshot sidecar and keeps + // its previous behaviour, so it always reports an empty list. + const changed = []; try { // Security: the renderer is untrusted, so containment-check the summary path // (symlink-safe, output/ only) and operate exclusively on the canonical @@ -3931,6 +3946,63 @@ ipcMain.handle('update-meeting', async (event, summaryFilePath, updates) => { } const { realPath } = validated; + if (!updates || typeof updates !== 'object') { + return { success: false, error: 'Invalid update payload' }; + } + + // Type-check the note content fields before anything is read or written. + // The section writers coerce with String(), so a wrong-typed value would + // not fail loudly - it would land in the note as "[object Object]", and a + // nested one would slip a forged heading past the structural scan below, + // which can only inspect strings: key_points: [['## Transcript']] holds no + // string to scan yet stringifies straight back into a heading. Rejecting + // the whole payload is both safer and kinder than writing a mangled note. + const isStringArray = (value) => + Array.isArray(value) && value.every((item) => typeof item === 'string'); + // `analysis` may be absent (or null, which is how an empty analysis can + // come back through a JSON round trip); a title is what makes a topic. + const isDiscussionAreaList = (value) => + Array.isArray(value) && + value.every( + (area) => + area !== null && + typeof area === 'object' && + !Array.isArray(area) && + typeof area.title === 'string' && + (area.analysis === undefined || area.analysis === null || typeof area.analysis === 'string'), + ); + + if (updates.summary !== undefined && typeof updates.summary !== 'string') { + return { success: false, error: 'summary must be a string.' }; + } + if (updates.key_points !== undefined && !isStringArray(updates.key_points)) { + return { success: false, error: 'key_points must be an array of strings.' }; + } + if (updates.action_items !== undefined && !isStringArray(updates.action_items)) { + return { success: false, error: 'action_items must be an array of strings.' }; + } + if (updates.discussion_areas !== undefined && !isDiscussionAreaList(updates.discussion_areas)) { + return { + success: false, + error: 'discussion_areas must be an array of { title, analysis } objects.', + }; + } + + // The renderer is untrusted: a field containing a '## ' line would forge a + // section boundary and silently rewrite the note's structure for every + // consumer (both parsers, the clipboard export, the PDF export, org share). + const textCandidates = [ + updates.summary, + ...(Array.isArray(updates.key_points) ? updates.key_points : []), + ...(Array.isArray(updates.action_items) ? updates.action_items : []), + ...(Array.isArray(updates.discussion_areas) + ? updates.discussion_areas.flatMap((area) => [area && area.title, area && area.analysis]) + : []), + ].filter((value) => typeof value === 'string'); + if (textCandidates.some(containsStructuralLine)) { + return { success: false, error: 'A note field may not contain a markdown heading.' }; + } + // Read existing data if (!fs.existsSync(realPath)) { return { @@ -3968,6 +4040,15 @@ ipcMain.handle('update-meeting', async (event, summaryFilePath, updates) => { let body = raw; let updatedRaw = raw; + // Does this call touch the note's generated content (as opposed to only + // its title or the user's own notes)? Drives the snapshot capture and the + // missing-frontmatter guard below. + const structural = + updates.summary !== undefined || + updates.key_points !== undefined || + updates.action_items !== undefined || + updates.discussion_areas !== undefined; + if (raw.startsWith('---')) { // Split with NO limit and rejoin the tail: split('---', 3) DISCARDS any // '---' in the body (markdown thematic breaks in summaries, and the @@ -4020,11 +4101,81 @@ ipcMain.handle('update-meeting', async (event, summaryFilePath, updates) => { if (updates.user_notes !== undefined) { body = upsertUserNotesSection(body, updates.user_notes); } + + // Snapshot BEFORE the first structural edit. For a note that predates + // this feature the current content is the model's own output, because + // the note has never been editable - so a lazy capture here is still + // an accurate original. + if (structural && !readSnapshot(realPath)) { + try { + const before = parseMeetingMarkdown(raw, realPath); + captureSnapshot( + realPath, + { + summary: before.summary, + key_points: before.key_points, + action_items: before.action_items, + discussion_areas: before.discussion_areas, + participants: before.participants, + }, + 'first_edit', + ); + } catch (snapshotError) { + // note-snapshot's writes throw by design (a path it cannot derive + // a sidecar from must not be written to). That contract protects + // the note, and it must not also cost the user their edit: the + // sidecar is a diff base for later learning, the edit is the + // product. Log it and save anyway. The cost is that this note has + // no original to compare against, so a later regenerate cannot + // warn that it is about to discard edits (see the regenerate + // guard) - worth surfacing in the log, not worth refusing a save + // that would otherwise succeed. + console.error('Note snapshot capture failed (saving anyway):', snapshotError); + } + } + + if (updates.summary !== undefined) { + body = setSummary(body, updates.summary); + changed.push('summary'); + } + if (updates.discussion_areas !== undefined) { + body = setDiscussionAreas(body, updates.discussion_areas); + changed.push('discussion_areas'); + } + if (updates.key_points !== undefined) { + body = setKeyPoints(body, updates.key_points); + changed.push('key_points'); + } + if (updates.action_items !== undefined) { + body = setActionItems(body, updates.action_items); + changed.push('action_items'); + } + updatedRaw = `---${newLines.join('\n')}---${body}`; } } - fs.writeFileSync(realPath, updatedRaw, 'utf8'); + // A content edit only reaches `body` inside the frontmatter block above. + // If we get here with structural updates and nothing recorded, the note + // had no parsable frontmatter and the edit was silently dropped - report + // that instead of returning success over a file we did not change. + if (structural && changed.length === 0) { + return { success: false, error: 'Note has no readable frontmatter; refusing to edit it.' }; + } + + // Atomic: a crash or a full disk mid-write must not leave a truncated + // note behind, and this write can now carry the summary itself. + writeFileAtomicSync(realPath, updatedRaw); + if (changed.length) { + try { + markEdited(realPath, changed); + } catch (snapshotError) { + // Same trade as the capture above: the note is already saved, so a + // sidecar bookkeeping failure must not turn a successful save into a + // reported failure. + console.error('Note snapshot update failed (note was saved):', snapshotError); + } + } data = { session_info: { @@ -4063,7 +4214,10 @@ ipcMain.handle('update-meeting', async (event, summaryFilePath, updates) => { return { success: true, - message: 'Meeting updated successfully' + message: 'Meeting updated successfully', + // Markdown notes only: which generated sections this call rewrote. Empty + // for a title/notes-only save and for the legacy .json format. + edited_fields: changed, }; } catch (error) { console.error('Update meeting error:', error); diff --git a/app/renderer/src/lib/ipc.ts b/app/renderer/src/lib/ipc.ts index b0bdbe1d..0ac46358 100644 --- a/app/renderer/src/lib/ipc.ts +++ b/app/renderer/src/lib/ipc.ts @@ -108,12 +108,20 @@ export interface CalendarEvent { color?: string; } +/** A partial update for one meeting note. Every content field below is written + * into its own `## ` section of the .md, leaving the rest of the file + * (transcript included) byte-identical; main rejects any value carrying a + * markdown heading, since that would forge a section boundary. */ export interface UpdateMeetingPatch { name?: string; summary?: string; + /** Not editable on .md notes yet (the participants editor is a later build); + * still honoured by the legacy .json format. */ participants?: unknown[]; key_points?: string[]; - action_items?: unknown[]; + action_items?: string[]; + /** `## Key Topics`: one `### title` subsection per area, analysis below it. */ + discussion_areas?: { title: string; analysis?: string }[]; /** The user's own notes (My notes tab). Upserts the `## User Notes` body * section of the .md (or the `user_notes` field of a legacy .json); an * empty string removes the section. */ @@ -376,7 +384,13 @@ export type RecordingsDirResponse = Result<{ path: string }>; export type ListMeetingsResponse = Result<{ meetings: Meeting[] }>; export type GetMeetingResponse = Result<{ meeting: Meeting }>; -export type UpdateMeetingResponse = Result<{ message: string; updatedData: Meeting }>; +export type UpdateMeetingResponse = Result<{ + message: string; + updatedData: Meeting; + /** Which generated sections the save rewrote. Markdown notes only: empty for + * a title/notes-only save and for the legacy .json format. */ + edited_fields?: string[]; +}>; // Soft-delete (#234): main hides only the summary and returns an `id` + a // MAIN-owned `deadline` (epoch ms) so the renderer can offer Undo. `message` is // set instead when there was nothing to delete (no summary / already gone). From 28924b591ac06c1f6c38756c653c59bebafc5c4b Mon Sep 17 00:00:00 2001 From: Optic00 Date: Tue, 28 Jul 2026 15:09:19 +0200 Subject: [PATCH 09/22] fix(notes): gate note edits on the parsers' normalized grammar --- app/main.js | 41 +++++- e2e/specs/note-edit-guard.t2.spec.ts | 189 +++++++++++++++++++++++++++ 2 files changed, 225 insertions(+), 5 deletions(-) create mode 100644 e2e/specs/note-edit-guard.t2.spec.ts diff --git a/app/main.js b/app/main.js index df412aad..fa9e40b2 100644 --- a/app/main.js +++ b/app/main.js @@ -3972,14 +3972,33 @@ ipcMain.handle('update-meeting', async (event, summaryFilePath, updates) => { (area.analysis === undefined || area.analysis === null || typeof area.analysis === 'string'), ); + // A bullet-list entry is ONE line by construction: renderBulletList writes + // "- ", and both parsers keep only lines that start with "- " + // (app/main.js:2639/2647, and the Python mirror). A second line in an entry + // is therefore dropped on the next read, and the save after that rewrites + // the section from the parsed array and deletes it from the file too. The + // renderer is where such a string first becomes reachable, so refuse it + // here rather than joining or truncating it behind the user's back. + const LINE_BREAK = /[\r\n]/; + if (updates.summary !== undefined && typeof updates.summary !== 'string') { return { success: false, error: 'summary must be a string.' }; } - if (updates.key_points !== undefined && !isStringArray(updates.key_points)) { - return { success: false, error: 'key_points must be an array of strings.' }; + if (updates.key_points !== undefined) { + if (!isStringArray(updates.key_points)) { + return { success: false, error: 'key_points must be an array of strings.' }; + } + if (updates.key_points.some((item) => LINE_BREAK.test(item))) { + return { success: false, error: 'A key point may not contain a line break.' }; + } } - if (updates.action_items !== undefined && !isStringArray(updates.action_items)) { - return { success: false, error: 'action_items must be an array of strings.' }; + if (updates.action_items !== undefined) { + if (!isStringArray(updates.action_items)) { + return { success: false, error: 'action_items must be an array of strings.' }; + } + if (updates.action_items.some((item) => LINE_BREAK.test(item))) { + return { success: false, error: 'An action item may not contain a line break.' }; + } } if (updates.discussion_areas !== undefined && !isDiscussionAreaList(updates.discussion_areas)) { return { @@ -3991,6 +4010,18 @@ ipcMain.handle('update-meeting', async (event, summaryFilePath, updates) => { // The renderer is untrusted: a field containing a '## ' line would forge a // section boundary and silently rewrite the note's structure for every // consumer (both parsers, the clipboard export, the PDF export, org share). + // + // Scan the NORMALIZED text, because that is the grammar the parsers + // actually split on: both of them run normalizeMarkdownForParsing (and its + // Python mirror) BEFORE the '## ' split, which breaks a reasoning close-tag + // away from a heading glued to it - so "b## Summary" is a legal + // mid-line string at write time and a real heading at read time. Anchoring + // on '^' alone would let that through, and the forged heading would then + // win the parser's last-occurrence rule and blank the real section. This is + // not only an attack: the normalizer exists precisely because models emit + // that shape, so a user pasting model output hits it by accident. Tying the + // gate to the parser's own normalizer keeps the two from drifting apart. + // Reject, never strip: silently rewriting the user's text is worse. const textCandidates = [ updates.summary, ...(Array.isArray(updates.key_points) ? updates.key_points : []), @@ -3999,7 +4030,7 @@ ipcMain.handle('update-meeting', async (event, summaryFilePath, updates) => { ? updates.discussion_areas.flatMap((area) => [area && area.title, area && area.analysis]) : []), ].filter((value) => typeof value === 'string'); - if (textCandidates.some(containsStructuralLine)) { + if (textCandidates.some((value) => containsStructuralLine(normalizeMarkdownForParsing(value)))) { return { success: false, error: 'A note field may not contain a markdown heading.' }; } diff --git a/e2e/specs/note-edit-guard.t2.spec.ts b/e2e/specs/note-edit-guard.t2.spec.ts new file mode 100644 index 00000000..39390f55 --- /dev/null +++ b/e2e/specs/note-edit-guard.t2.spec.ts @@ -0,0 +1,189 @@ +import { test, expect } from '../fixtures/electron'; +import { mkdirSync, writeFileSync, readFileSync } from 'fs'; +import path from 'path'; + +/** + * T2 - the input gate on update-meeting's note content fields, driven through + * the REAL preload bridge against the REAL handler (main.js), asserting the + * note file on disk. + * + * The gate exists because the renderer is untrusted and because both parsers + * (parseMeetingMarkdown here, _parse_meeting_markdown in simple_recorder.py) + * read a note by splitting on `## ` AFTER normalizeMarkdownForParsing has + * broken a reasoning close-tag away from a heading glued to it. So a value like + * `b## Summary` is a harmless mid-line string when it is written and a + * real section boundary when it is read - and the forged heading, being the + * last occurrence, wins the parsers' last-one-wins rule and blanks the real + * section everywhere the note is consumed (detail page, clipboard, PDF, org + * share). Worse, it is unrecoverable through the UI: the section writers match + * `line.startsWith('## ')` and cannot see a mid-line heading, so every later + * edit writes the real section while the parser keeps reading the forged one. + * + * Not adversarial-only: the normalizer exists because reasoning models emit + * exactly that shape, so pasting model output into a note is enough to hit it. + * + * The other half of the gate is line breaks inside a bullet entry, which the + * parsers drop (they keep only lines starting with `- `) and the next save then + * deletes from the file. + * + * Model-free: a seeded note plus the note IPCs, no ASR and no Ollama. + */ + +const NOTE = [ + '---', + 'title: "Guarded Note"', + 'date: "2026-07-12T10:00:00"', + 'duration_seconds: 600', + 'language: "en"', + 'is_diarised: false', + '---', + '', + '## Summary', + '', + 'The team agreed to ship on Friday.', + '', + '## Key Topics', + '', + '### Billing', + '', + 'The migration is blocked on the vendor.', + '', + '## Key Points', + '', + '- Ship Friday', + '', + '## Action Items', + '', + '- Alice pings the vendor', + '', + '## Transcript', + '', + 'Alice: we ship Friday.', + '', +].join('\n'); + +test('update-meeting rejects a forged section boundary in every content field', async ({ + launchApp, + userDataDir, +}) => { + test.setTimeout(60_000); + const outputDir = path.join(userDataDir, 'output'); + mkdirSync(outputDir, { recursive: true }); + const summaryPath = path.join(outputDir, 'guarded_summary.md'); + writeFileSync(summaryPath, NOTE, 'utf-8'); + + const { page } = await launchApp(); + const update = (patch: Record) => + page.evaluate( + ([f, p]) => window.stenoai.meetings.update(f as string, p as object), + [summaryPath, patch] as const, + ); + + // Every field that reaches a section writer, each carrying a heading that is + // mid-line as written and a real heading once normalized. + const forged = 'b## Summary'; + const patches: Array> = [ + { summary: forged }, + { key_points: [forged] }, + { action_items: [forged] }, + { discussion_areas: [{ title: forged, analysis: 'ok' }] }, + { discussion_areas: [{ title: 'ok', analysis: forged }] }, + // The normalizer's whole tag set, case-insensitively. + { summary: 'x## Transcript' }, + { summary: 'x ### Topic' }, + { summary: 'x\n## Summary' }, + // A plain leading heading still has to be caught (the original gate). + { summary: '## Summary\nforged' }, + ]; + + for (const patch of patches) { + const res = await update(patch); + expect(res, `expected rejection for ${JSON.stringify(patch)}`).toMatchObject({ + success: false, + error: 'A note field may not contain a markdown heading.', + }); + // Nothing was written: the gate runs before the note is read or written. + expect(readFileSync(summaryPath, 'utf8')).toBe(NOTE); + } +}); + +test('update-meeting rejects a line break inside a key point or action item', async ({ + launchApp, + userDataDir, +}) => { + test.setTimeout(60_000); + const outputDir = path.join(userDataDir, 'output'); + mkdirSync(outputDir, { recursive: true }); + const summaryPath = path.join(outputDir, 'bullets_summary.md'); + writeFileSync(summaryPath, NOTE, 'utf-8'); + + const { page } = await launchApp(); + const update = (patch: Record) => + page.evaluate( + ([f, p]) => window.stenoai.meetings.update(f as string, p as object), + [summaryPath, patch] as const, + ); + + const r1 = await update({ key_points: ['line one\nline two'] }); + expect(r1).toMatchObject({ success: false, error: 'A key point may not contain a line break.' }); + expect(readFileSync(summaryPath, 'utf8')).toBe(NOTE); + + const r2 = await update({ action_items: ['do this\r\nand that'] }); + expect(r2).toMatchObject({ + success: false, + error: 'An action item may not contain a line break.', + }); + expect(readFileSync(summaryPath, 'utf8')).toBe(NOTE); + + // A multi-line SUMMARY is legitimate - the section carries prose, and the + // parser returns the whole block. Only bullet entries are single-line. + const r3 = await update({ summary: 'First paragraph.\n\nSecond paragraph.' }); + expect(r3.success).toBe(true); + expect(readFileSync(summaryPath, 'utf8')).toContain( + '## Summary\n\nFirst paragraph.\n\nSecond paragraph.\n', + ); +}); + +test('the heading gate does not reject legitimate text that merely contains a hash', async ({ + launchApp, + userDataDir, +}) => { + test.setTimeout(60_000); + const outputDir = path.join(userDataDir, 'output'); + mkdirSync(outputDir, { recursive: true }); + const summaryPath = path.join(outputDir, 'hashes_summary.md'); + writeFileSync(summaryPath, NOTE, 'utf-8'); + + const { page } = await launchApp(); + const update = (patch: Record) => + page.evaluate( + ([f, p]) => window.stenoai.meetings.update(f as string, p as object), + [summaryPath, patch] as const, + ); + + // None of these is a heading to either the writers or the parsers: no space + // after the hashes, a hash mid-line, or a hash inside a word. + const legit = 'Ported ##Foo to C# and tagged it #hashtag; issue #42 is next.'; + const res = await update({ + summary: legit, + key_points: [legit], + action_items: [legit], + discussion_areas: [{ title: 'C# migration', analysis: legit }], + }); + expect(res.success).toBe(true); + + const md = readFileSync(summaryPath, 'utf8'); + expect(md).toContain(`## Summary\n\n${legit}\n`); + expect(md).toContain(`- ${legit}`); + expect(md).toContain('### C# migration'); + // The note still has exactly the sections it started with. + expect(md.match(/^## /gm)?.length).toBe(5); + + // And the parser agrees, so nothing was smuggled into a new section. + const parsed = await page.evaluate(async (f) => { + const r = await window.stenoai.meetings.get(f as string); + return r.success ? r.meeting : { error: r.error }; + }, summaryPath); + expect(parsed.summary).toBe(legit); + expect(parsed.key_points).toEqual([legit]); +}); From dff5ce3d1d2c9a42ec10bd70701aba74aadbeb1a Mon Sep 17 00:00:00 2001 From: Optic00 Date: Tue, 28 Jul 2026 15:21:13 +0200 Subject: [PATCH 10/22] feat(notes): snapshot the model output when a note is generated --- simple_recorder.py | 38 ++++++++++++++++++++++++++++++ tests/test_note_snapshot.py | 47 +++++++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+) create mode 100644 tests/test_note_snapshot.py diff --git a/simple_recorder.py b/simple_recorder.py index 9215e5cc..555b74c4 100644 --- a/simple_recorder.py +++ b/simple_recorder.py @@ -812,6 +812,7 @@ async def process_recording_streaming(self, audio_file: str, session_name: str = md_lines.append('') md_lines.append(notes_text) summary_path.write_text('\n'.join(md_lines), encoding='utf-8') + _write_original_snapshot(summary_path, parsed) # Clean up from src.config import get_config @@ -840,6 +841,37 @@ async def process_recording_streaming(self, audio_file: str, session_name: str = } +def _write_original_snapshot(summary_path, parsed) -> None: + """Persist the model's own output next to a freshly written note. + + The note file is rebuilt from scratch by reprocess, so the snapshot cannot + live inside it. It is the diff base the note editor needs in order to warn + before a regenerate discards user corrections, and it is best-effort: a + failure here must never fail a pipeline run that produced a good note. + """ + try: + snapshot_path = Path(str(summary_path).replace('_summary.md', '_original.json')) + payload = { + 'version': 1, + 'captured_at': datetime.now().isoformat(), + 'capture': 'generation', + 'original': { + 'summary': parsed.get('summary', ''), + 'key_points': parsed.get('key_points', []), + 'action_items': parsed.get('action_items', []), + 'discussion_areas': parsed.get('discussion_areas', []), + 'participants': parsed.get('participants', []), + }, + # A regenerated note starts clean: its corrections were either + # confirmed as discarded by the user or never existed. + 'edited_fields': [], + 'edited_at': None, + } + snapshot_path.write_text(json.dumps(payload, indent=2), encoding='utf-8') + except Exception as exc: + logger.warning(f"Could not write original snapshot for {summary_path}: {exc}") + + def generate_default_template_report(summary_path, transcript, notes, language, duration_minutes, config, summarizer): """Best-effort: if the configured default template is not 'standard', generate @@ -1371,6 +1403,7 @@ def _heartbeat_sink(done, total): md_lines.append('') md_lines.append(notes_text) summary_path.write_text('\n'.join(md_lines), encoding='utf-8') + _write_original_snapshot(summary_path, parsed) # Clean up audio. When we fell back to the live transcript the batch # transcription was empty/failed, so KEEP the audio regardless of the @@ -3100,6 +3133,11 @@ def _transcribe_heartbeat(done, total): md_lines.append('') md_lines.append(notes_text) summary_path.write_text('\n'.join(md_lines), encoding='utf-8') + # The .md rebuild above writes the raw streamed markdown rather than + # structured fields, so parse it here (mirrors the JSON branch below) + # to get the dict the snapshot needs. + parsed = recorder._parse_streamed_markdown(streamed_md) + _write_original_snapshot(summary_path, parsed) else: # JSON format: parse streamed markdown into structured fields parsed = recorder._parse_streamed_markdown(streamed_md) diff --git a/tests/test_note_snapshot.py b/tests/test_note_snapshot.py new file mode 100644 index 00000000..7119dca1 --- /dev/null +++ b/tests/test_note_snapshot.py @@ -0,0 +1,47 @@ +import json +import tempfile +import unittest +from pathlib import Path + +from simple_recorder import _write_original_snapshot + + +class WriteOriginalSnapshotTests(unittest.TestCase): + def setUp(self): + self.dir = Path(tempfile.mkdtemp()) + self.summary_path = self.dir / "Weekly_Sync_summary.md" + self.summary_path.write_text("---\ntitle: \"x\"\n---\n\n## Summary\n\nhi\n", encoding="utf-8") + self.parsed = { + "summary": "We agreed the budget.", + "key_points": ["Budget approved"], + "action_items": ["Anna sends the draft"], + "discussion_areas": [{"title": "Budget", "analysis": "Reviewed."}], + "participants": [], + } + + def snapshot(self): + return json.loads((self.dir / "Weekly_Sync_original.json").read_text(encoding="utf-8")) + + def test_writes_the_model_output_with_generation_provenance(self): + _write_original_snapshot(self.summary_path, self.parsed) + data = self.snapshot() + self.assertEqual(data["version"], 1) + self.assertEqual(data["capture"], "generation") + self.assertEqual(data["original"]["summary"], "We agreed the budget.") + self.assertEqual(data["edited_fields"], []) + + def test_regeneration_overwrites_the_previous_snapshot(self): + _write_original_snapshot(self.summary_path, self.parsed) + self.parsed["summary"] = "A regenerated summary." + _write_original_snapshot(self.summary_path, self.parsed) + self.assertEqual(self.snapshot()["original"]["summary"], "A regenerated summary.") + + def test_a_write_failure_never_raises_into_the_pipeline(self): + # A read-only directory must not fail the whole run: the note matters, + # the snapshot is best-effort. + unwritable = Path("/nonexistent-dir-for-test") / "x_summary.md" + _write_original_snapshot(unwritable, self.parsed) + + +if __name__ == "__main__": + unittest.main() From 5d9133a2680f7d6f064853c88a14ca065c2f1c39 Mon Sep 17 00:00:00 2001 From: Optic00 Date: Tue, 28 Jul 2026 15:34:12 +0200 Subject: [PATCH 11/22] fix(notes): anchor and harden the original-snapshot writer Anchor the sidecar path derivation on the end of the string (raising, caught by the writer's own best-effort guard, on a path that does not end in _summary.md) instead of an unanchored str.replace() that could silently overwrite the note itself. Use the shared _atomic_write_json writer instead of a plain write_text() so a crash mid-write cannot leave a torn sidecar that permanently loses a note's diff base. Read the note back from disk and parse it with _parse_meeting_markdown (the mirror of app/main.js's parseMeetingMarkdown) instead of reusing _parse_streamed_markdown's output, so the snapshot agrees with what the note editor itself reads back from the same file. Drop the now-unused parsed argument and the two dead parsed assignments this leaves behind. Add reprocess-level coverage that the sidecar actually gets written, and a unit test for the anchored-path guard. --- simple_recorder.py | 52 +++++++++++++------ tests/test_note_snapshot.py | 77 +++++++++++++++++++++++------ tests/test_reprocess_frontmatter.py | 23 +++++++++ 3 files changed, 121 insertions(+), 31 deletions(-) diff --git a/simple_recorder.py b/simple_recorder.py index 555b74c4..034cabe0 100644 --- a/simple_recorder.py +++ b/simple_recorder.py @@ -784,9 +784,6 @@ async def process_recording_streaming(self, audio_file: str, session_name: str = print(f"TITLE:{session_name}", flush=True) print(f"Auto-generated title: {session_name}") - # Step 4: Parse streamed markdown into structured JSON - parsed = self._parse_streamed_markdown(streamed_md) - # Step 5: Save as .md (primary format for new meetings) summary_path = self.output_dir / f"{audio_path.stem}_summary.md" processed_at = datetime.now().isoformat() @@ -812,7 +809,7 @@ async def process_recording_streaming(self, audio_file: str, session_name: str = md_lines.append('') md_lines.append(notes_text) summary_path.write_text('\n'.join(md_lines), encoding='utf-8') - _write_original_snapshot(summary_path, parsed) + _write_original_snapshot(summary_path) # Clean up from src.config import get_config @@ -841,16 +838,39 @@ async def process_recording_streaming(self, audio_file: str, session_name: str = } -def _write_original_snapshot(summary_path, parsed) -> None: +def _write_original_snapshot(summary_path) -> None: """Persist the model's own output next to a freshly written note. The note file is rebuilt from scratch by reprocess, so the snapshot cannot live inside it. It is the diff base the note editor needs in order to warn before a regenerate discards user corrections, and it is best-effort: a failure here must never fail a pipeline run that produced a good note. + + Reads the note back from disk and parses it with `_parse_meeting_markdown` + (the mirror of app/main.js's parseMeetingMarkdown) rather than reusing a + streamed-markdown parse: `_parse_streamed_markdown` collapses whitespace + differently (joins the summary with spaces, strips each topic line), so + snapshotting its output would disagree with what the note editor itself + reads back from this same file — a spurious diff on every unedited note. + Parsing the just-written file is what makes the two agree by construction. """ try: - snapshot_path = Path(str(summary_path).replace('_summary.md', '_original.json')) + summary_path = Path(summary_path) + str_path = str(summary_path) + suffix = '_summary.md' + # Anchored on the end of the path, matching app/note-snapshot.js's + # noteSnapshotPath. An unanchored str.replace() would silently derive + # the wrong sidecar path for a summary file that doesn't end in + # "_summary.md" (e.g. reprocess called on a bare ".md" file) and the + # write below would then land on — and destroy — the note itself + # instead of a sidecar next to it. Raise here instead: the except + # below turns it into a logged warning, never a note-destroying write. + if not str_path.endswith(suffix): + raise ValueError( + f"expected a path ending in '{suffix}', got: {str_path}" + ) + snapshot_path = Path(str_path[: -len(suffix)] + '_original.json') + parsed = _parse_meeting_markdown(summary_path) payload = { 'version': 1, 'captured_at': datetime.now().isoformat(), @@ -867,7 +887,14 @@ def _write_original_snapshot(summary_path, parsed) -> None: 'edited_fields': [], 'edited_at': None, } - snapshot_path.write_text(json.dumps(payload, indent=2), encoding='utf-8') + # Atomic write (tempfile + os.replace): a plain write_text() truncates + # first, and a crash mid-write leaves a torn JSON file. That's worse + # than a transient bad read here — readSnapshot (app/note-snapshot.js) + # treats unparseable JSON as "absent", but captureSnapshot refuses to + # overwrite a FILE that already exists, so a torn sidecar permanently + # loses this note's diff base and silences the regenerate warning for + # it forever. + _atomic_write_json(snapshot_path, payload) except Exception as exc: logger.warning(f"Could not write original snapshot for {summary_path}: {exc}") @@ -1367,9 +1394,6 @@ def _heartbeat_sink(done, total): audio_path = Path(audio_file) summary_path = recorder.output_dir / f"{audio_path.stem}_summary.md" - # Parse the streamed markdown for title generation - parsed = MeetingPipeline._parse_streamed_markdown(streamed_md) - # Save as .md only (primary format for new meetings) summary_path = summary_path.with_suffix('.md') processed_at = datetime.now().isoformat() @@ -1403,7 +1427,7 @@ def _heartbeat_sink(done, total): md_lines.append('') md_lines.append(notes_text) summary_path.write_text('\n'.join(md_lines), encoding='utf-8') - _write_original_snapshot(summary_path, parsed) + _write_original_snapshot(summary_path) # Clean up audio. When we fell back to the live transcript the batch # transcription was empty/failed, so KEEP the audio regardless of the @@ -3133,11 +3157,7 @@ def _transcribe_heartbeat(done, total): md_lines.append('') md_lines.append(notes_text) summary_path.write_text('\n'.join(md_lines), encoding='utf-8') - # The .md rebuild above writes the raw streamed markdown rather than - # structured fields, so parse it here (mirrors the JSON branch below) - # to get the dict the snapshot needs. - parsed = recorder._parse_streamed_markdown(streamed_md) - _write_original_snapshot(summary_path, parsed) + _write_original_snapshot(summary_path) else: # JSON format: parse streamed markdown into structured fields parsed = recorder._parse_streamed_markdown(streamed_md) diff --git a/tests/test_note_snapshot.py b/tests/test_note_snapshot.py index 7119dca1..b3be4b7d 100644 --- a/tests/test_note_snapshot.py +++ b/tests/test_note_snapshot.py @@ -5,42 +5,89 @@ from simple_recorder import _write_original_snapshot +_NOTE_TEMPLATE = """--- +title: "Weekly Sync" +--- + +## Summary + +{summary} + +## Key Points + +- Budget approved + +## Action Items + +- Anna sends the draft + +## Key Topics + +### Budget + +Reviewed. +""" + class WriteOriginalSnapshotTests(unittest.TestCase): + """`_write_original_snapshot` reads the note it is given back from disk and + parses it with `_parse_meeting_markdown` (the mirror of app/main.js's + parseMeetingMarkdown), rather than accepting a fields dict directly — that + way the snapshot agrees field-for-field with what the note editor itself + reads back from the same file.""" + def setUp(self): self.dir = Path(tempfile.mkdtemp()) self.summary_path = self.dir / "Weekly_Sync_summary.md" - self.summary_path.write_text("---\ntitle: \"x\"\n---\n\n## Summary\n\nhi\n", encoding="utf-8") - self.parsed = { - "summary": "We agreed the budget.", - "key_points": ["Budget approved"], - "action_items": ["Anna sends the draft"], - "discussion_areas": [{"title": "Budget", "analysis": "Reviewed."}], - "participants": [], - } + self.summary_path.write_text( + _NOTE_TEMPLATE.format(summary="We agreed the budget."), encoding="utf-8" + ) def snapshot(self): return json.loads((self.dir / "Weekly_Sync_original.json").read_text(encoding="utf-8")) def test_writes_the_model_output_with_generation_provenance(self): - _write_original_snapshot(self.summary_path, self.parsed) + _write_original_snapshot(self.summary_path) data = self.snapshot() self.assertEqual(data["version"], 1) self.assertEqual(data["capture"], "generation") self.assertEqual(data["original"]["summary"], "We agreed the budget.") + self.assertEqual(data["original"]["key_points"], ["Budget approved"]) + self.assertEqual(data["original"]["action_items"], ["Anna sends the draft"]) + self.assertEqual( + data["original"]["discussion_areas"], + [{"title": "Budget", "analysis": "Reviewed."}], + ) self.assertEqual(data["edited_fields"], []) def test_regeneration_overwrites_the_previous_snapshot(self): - _write_original_snapshot(self.summary_path, self.parsed) - self.parsed["summary"] = "A regenerated summary." - _write_original_snapshot(self.summary_path, self.parsed) + _write_original_snapshot(self.summary_path) + self.summary_path.write_text( + _NOTE_TEMPLATE.format(summary="A regenerated summary."), encoding="utf-8" + ) + _write_original_snapshot(self.summary_path) self.assertEqual(self.snapshot()["original"]["summary"], "A regenerated summary.") def test_a_write_failure_never_raises_into_the_pipeline(self): - # A read-only directory must not fail the whole run: the note matters, - # the snapshot is best-effort. + # A missing/read-only directory must not fail the whole run: the note + # matters, the snapshot is best-effort. This path also never gets far + # enough to read the (nonexistent) note back, so it exercises the + # broad except around the whole body, not just the write call. unwritable = Path("/nonexistent-dir-for-test") / "x_summary.md" - _write_original_snapshot(unwritable, self.parsed) + _write_original_snapshot(unwritable) + + def test_a_path_not_ending_in_summary_md_is_rejected_without_writing(self): + # Anchored guard, mirroring app/note-snapshot.js's noteSnapshotPath: an + # unanchored str.replace()-based derivation would silently produce the + # wrong sidecar path for a summary file that doesn't end in + # "_summary.md" and could overwrite the note itself. reprocess gates + # its .md branch on suffix == '.md', not on the "_summary" stem, so a + # bare ".md" path is reachable in practice, not just theoretical. + bare_md = self.dir / "Weekly_Sync.md" + bare_md.write_text("not a real note", encoding="utf-8") + _write_original_snapshot(bare_md) + self.assertFalse((self.dir / "Weekly_Sync.json").exists()) + self.assertEqual(bare_md.read_text(encoding="utf-8"), "not a real note") if __name__ == "__main__": diff --git a/tests/test_reprocess_frontmatter.py b/tests/test_reprocess_frontmatter.py index b7aa189e..65c1d298 100644 --- a/tests/test_reprocess_frontmatter.py +++ b/tests/test_reprocess_frontmatter.py @@ -8,6 +8,7 @@ Dropping either silently removes the meeting from all its folders / loses the live-transcript flag on every regenerate. """ +import json import tempfile import unittest from pathlib import Path @@ -130,6 +131,28 @@ def test_missing_provenance_reprocesses_to_null_provenance(self): self.assertIsNone(reparsed["session_info"]["configured_language"]) self.assertIsNone(reparsed["session_info"]["detected_language"]) + def test_writes_original_snapshot_on_regenerate(self): + """reprocess must snapshot the regenerated note into _original.json + so the note editor has a diff base and can warn before a future + regenerate discards the user's own edits. Deleting the call this test + guards (simple_recorder._write_original_snapshot in reprocess's .md + branch) must make this test fail, not silently pass.""" + with tempfile.TemporaryDirectory() as tmp: + summary = _write_summary(tmp) + res = _run_reprocess(tmp, summary) + self.assertEqual(res.exit_code, 0, res.output) + + snapshot_path = Path(tmp) / "meeting_original.json" + self.assertTrue( + snapshot_path.exists(), + "reprocess must write an _original.json sidecar", + ) + snapshot = json.loads(snapshot_path.read_text()) + self.assertEqual(snapshot["version"], 1) + self.assertEqual(snapshot["capture"], "generation") + self.assertEqual(snapshot["original"]["summary"], "Regenerated summary body") + self.assertEqual(snapshot["edited_fields"], []) + if __name__ == "__main__": unittest.main() From cecd31098fa26f1d4bd1806bfa0778965b7b0119 Mon Sep 17 00:00:00 2001 From: Optic00 Date: Tue, 28 Jul 2026 16:07:27 +0200 Subject: [PATCH 12/22] feat(notes): edit a generated note behind an explicit edit affordance The note stays a document until the user clicks Edit; Save or Cancel leaves edit mode, so the boundary between the model's output and the user's corrections stays visible. - NoteEditor owns the draft, sends only changed sections as snake_case patch keys, and mirrors main's markdown-heading gate so a refusal shows next to the field instead of arriving as a failed write. - List rows and topic titles are single-line inputs, and a line break in any of them is refused client-side: renderBulletList would drop the remainder of a key point or action item, and a break in a topic title slides text into the analysis and desynchronises the JS and Python parsers. - A failed save keeps edit mode and the typing. - A summary chunk arriving while the editor is open is held rather than swapping the note out mid-edit; the view toggle and Generate notes are locked for the same reason. --- app/renderer/src/routes/MeetingDetail.tsx | 85 +++- app/renderer/src/routes/NoteEditor.test.tsx | 199 ++++++++ app/renderer/src/routes/NoteEditor.tsx | 502 ++++++++++++++++++++ e2e/fixtures/electron.ts | 10 +- e2e/specs/note-editing.t2.spec.ts | 123 +++++ 5 files changed, 914 insertions(+), 5 deletions(-) create mode 100644 app/renderer/src/routes/NoteEditor.test.tsx create mode 100644 app/renderer/src/routes/NoteEditor.tsx create mode 100644 e2e/specs/note-editing.t2.spec.ts diff --git a/app/renderer/src/routes/MeetingDetail.tsx b/app/renderer/src/routes/MeetingDetail.tsx index 60bf3cac..3e4c5510 100644 --- a/app/renderer/src/routes/MeetingDetail.tsx +++ b/app/renderer/src/routes/MeetingDetail.tsx @@ -34,6 +34,7 @@ import { useGenerateReport, useSetActiveReport, useDeleteReport, + useUpdateMeeting, useUpdateUserNotes, meetingsKeys, } from '@/hooks/useMeetings'; @@ -63,7 +64,7 @@ import { useCreateFolder, } from '@/hooks/useFolders'; import { useActiveMeeting } from '@/lib/askBarContext'; -import { ipc, type Meeting, type Report, type Template } from '@/lib/ipc'; +import { ipc, type Meeting, type Report, type Template, type UpdateMeetingPatch } from '@/lib/ipc'; import { buildTranscriptBundle, defaultExportFilename } from '@/lib/transcriptBundle'; import { buildNotesCopyText, type StructuredNoteSections } from '@/lib/notesCopy'; import { buildNotesHtml, hasNotesContent } from '@/lib/notesPdf'; @@ -74,6 +75,7 @@ import { stripReasoning } from '@/lib/markdown'; import { pendingTitleRegens, streamCache, type StreamPhase } from '@/lib/meetingDetailState'; import { useReprocessBridge } from '@/hooks/reprocessBridgeStore'; import { useRecording } from '@/hooks/useRecording'; +import { NoteEditor, type NoteDraft } from './NoteEditor'; const LAST_OPENED_KEY = 'steno-last-opened-meeting'; @@ -268,6 +270,20 @@ function DetailContent({ const [reprocessFailed, setReprocessFailed] = React.useState(false); const qc = useQueryClient(); + // Note editing (D9): the generated note is a document until the user asks to + // edit it. Declared up here because the streaming listeners below have to see + // it; they're registered once per meeting, so they read it through a ref + // rather than re-subscribing on every keystroke's re-render. + const [editing, setEditing] = React.useState(false); + const updateMeeting = useUpdateMeeting(); + const editingRef = React.useRef(editing); + // Layout effect, not a render-time assignment: it still lands before the + // renderer yields to the next task, so no IPC chunk can observe a stale + // value, and it keeps the ref out of the render path. + React.useLayoutEffect(() => { + editingRef.current = editing; + }, [editing]); + // Report switch: null = the structured Standard summary, otherwise the id of // a generated report in meeting.reports. Seeded from the meeting's persisted // active_report so reopening a note lands on whatever was last viewed. @@ -299,6 +315,11 @@ function DetailContent({ const sessionName = info.name; const offChunk = ipc().on.summaryChunk((e) => { if (e.summaryFile !== summaryFile) return; + // A regenerate that finishes while the user is editing must not swap the + // note out from under them. Holding the stream is the conservative choice: + // the regenerated note is still on disk and appears as soon as they leave + // edit mode. + if (editingRef.current) return; // Promote from idle too (not just analyzing): an instant-stop note's // first-ever summary streams in with no prior reprocess to seed // 'analyzing', so without idle→generating the StreamingView (gated on @@ -613,6 +634,24 @@ function DetailContent({ }; const summary = meeting.summary?.trim(); + // Seeded from what the read-only note actually shows, reasoning stripped — + // editing the summary must not resurrect a block the UI hides. + const noteDraft: NoteDraft = { + summary: summary ? stripReasoning(summary) : '', + keyPoints: meeting.key_points ?? [], + actionItems: asStringArray(meeting.action_items), + discussionAreas: asDiscussionAreas(meeting.discussion_areas), + }; + // A rejected mutation (main refuses a forged heading, or the write fails) + // propagates to the editor, which keeps edit mode and the typing. + const saveNoteEdits = async (patch: UpdateMeetingPatch) => { + await updateMeeting.mutateAsync({ summaryFile, patch }); + setEditing(false); + }; + // Same "is there a note here at all" test the PDF export uses, plus the two + // states that are about to rewrite the note anyway. + const canEditNote = + canExportNotesPdf && !activeReport && streamPhase === 'idle' && !reprocess.isPending; const participants = asStringArray(meeting.participants); const keyPoints = meeting.key_points ?? []; const actionItems = asStringArray(meeting.action_items); @@ -730,6 +769,23 @@ function DetailContent({ Home
+ {/* Edit the generated note (D9). Only for the Standard structured + note — a template report is generated output with no section + grammar to patch — and only while nothing else is rewriting it. */} + {tab === 'summary' && !editing && ( + + + setEditing(true)} + disabled={!canEditNote} + > + + + + Edit note + + )} {/* Disabled while a summary/report stream is on screen — the @@ -776,7 +832,14 @@ function DetailContent({ // Disable while a recording is live on THIS note — same // reason the floating CTA hides: don't summarise a // still-growing transcript out from under the recording. - disabled={reprocess.isPending || streamPhase !== 'idle' || isRecordingThisNote} + // Also off while the note editor is open — a regenerate + // would discard the edit being typed. + disabled={ + reprocess.isPending || + streamPhase !== 'idle' || + isRecordingThisNote || + editing + } > {tab === 'summary' && ( @@ -1096,7 +1160,13 @@ function DetailContent({ )} - {streamPhase !== 'idle' ? ( + {editing ? ( + setEditing(false)} + /> + ) : streamPhase !== 'idle' ? ( ) : activeReport ? (
void; @@ -1346,6 +1417,9 @@ function NoteViewToggle({ onDeleteReport: (reportId: string) => void; onGenerate: (templateId: string) => void; generating: boolean; + /** Locked while the note editor is open: every path out of this control + * (switching view, generating a report) would drop unsaved edits. */ + disabled?: boolean; }) { const [menuOpen, setMenuOpen] = React.useState(false); const [deleteTarget, setDeleteTarget] = React.useState(null); @@ -1373,7 +1447,7 @@ function NoteViewToggle({ role="tablist" aria-label="Note view" className="inline-flex items-stretch overflow-hidden rounded-full" - style={{ border: '1px solid var(--border-subtle)' }} + style={{ border: '1px solid var(--border-subtle)', opacity: disabled ? 0.5 : 1 }} > {/* Left — My notes */} + +
+ + + {error && ( +

+ {error} +

+ )} + +
+ Summary + setDraft((prev) => ({ ...prev, summary: next }))} + placeholder="Write the summary…" + minHeight={110} + fontSize={15.5} + /> +
+ +
+ Key topics +
+ {draft.discussionAreas.map((area, i) => ( +
+
+ + setDraft((prev) => ({ + ...prev, + discussionAreas: prev.discussionAreas.map((a, j) => + j === i ? { ...a, title: next } : a + ), + })) + } + placeholder="Topic" + weight={600} + /> + + setDraft((prev) => ({ + ...prev, + discussionAreas: prev.discussionAreas.filter((_, j) => j !== i), + })) + } + /> +
+ + setDraft((prev) => ({ + ...prev, + discussionAreas: prev.discussionAreas.map((a, j) => + j === i ? { ...a, analysis: next } : a + ), + })) + } + placeholder="What was discussed…" + minHeight={64} + fontSize={14} + className="mr-9" + /> +
+ ))} + + setDraft((prev) => ({ + ...prev, + discussionAreas: [...prev.discussionAreas, { title: '', analysis: '' }], + })) + } + /> +
+
+ + setList('keyPoints', next)} + /> + + setList('actionItems', next)} + /> + + ); +} + +// --------------------------------------------------------------------------- +// Pieces +// --------------------------------------------------------------------------- + +/** + * Same type treatment as `SectionTitle` in the read-only note, so switching + * into edit mode doesn't move or restyle a single section heading. Rendered as + * a real