diff --git a/docs/reviews/youtube-embed-summary.md b/docs/reviews/youtube-embed-summary.md new file mode 100644 index 0000000..04c2f94 --- /dev/null +++ b/docs/reviews/youtube-embed-summary.md @@ -0,0 +1,50 @@ +# Review: youtube-embed + +**By architect.** + +## Task and commits reviewed +- Task: `youtube-embed` — render YouTube links in Memo post text as embedded players in + the recent-posts feed, preserving surrounding text and leaving non-embeddable URLs as + plain text. Read-only rendering feature in psf-memo-client (no Memo action, no DB change). +- Inbound handoff: refactorer `93baa25ad0` (merged fast-forward onto `swarmforge-architect`). +- Reviewed commits: `1466854` (spec), `8e606ae` (implementation by coder), `93baa25` + (refactorer CRAP reduction). The merged refactorer branch also carried `4eea327` + (psf-memo-db CRAP/DRY, a separate task) which was verified but is not the focus here. + +## Architectural findings and fixes applied +- **Good UI/Core separation:** `youtube-embed.js` is a pure service module with no React or + network dependencies, so the parser is directly unit-testable. `post-content.js` is written + in plain `React.createElement` style so the same component is reused by the browser JSX + build and by the acceptance adapter (`render-post.js`), which renders it to static HTML + under Node. Acceptance assertions therefore inspect the same markup the browser renders. +- **Refactorer CRAP reduction:** extracted `validVideoId`, `parseCandidate`, + `videoIdFromWatchUrl`, `videoIdFromShortUrl`, and `pushText` helpers. All functions now + have cyclomatic complexity ≤ 5 and CRAP ≤ 5.0 (well under threshold). +- **Hardening (this review):** added two unit tests to kill mutation survivors — a watch URL + with an invalid video id (special characters) and a `youtube.com` URL that is not `/watch`. +- **Carried-in psf-memo-db work (`4eea327`, separate task):** extracted pure filename/path + helpers into testable `db-backup-util.js` (keeping the zip/unzip adapter a thin shell) and + a shared `handleControllerError` across 7 REST controllers. Architecturally sound; verified + healthy but not the subject of this handoff. + +## Verification results +- **Language mutation** (`mutate4javascript src/services/youtube-embed.js --max-workers 8 --mutate-all`): + Killed 6, Survived 1, Uncovered 0. + - Survivor `line 82 1 -> 0` in `parsePostText` (`match[1]` → `match[0]`) is a **genuine + equivalent**: `URL_RE`'s capture group spans the entire pattern, so `match[0]` === + `match[1]`. Documented, not chased. +- **DRY** (`dry4javascript src/services/youtube-embed.js`): no duplicate candidates. +- **Soft Gherkin acceptance mutation** (`gherkin-mutator --level soft` on `youtube-embed.feature`): + 7 killed, 18 survived. All 18 survivors are single-character case/value mutations of example + values (addresses, txids, text, URLs) used consistently on both the setup and assertion sides + of their scenarios — intrinsic equivalents for a read-only feature, not implementation gaps. +- **CRAP** (`crap4javascript src/services/youtube-embed.js`): all functions CC ≤ 5, CRAP ≤ 5.0. + +## Suite status +- psf-memo-client: 260 unit passing, 33 property passing, all acceptance suites PASS + (including the new youtube-embed scenarios 1-3), lint clean, build OK. +- psf-memo-db (carried-in merge): 331 unit passing, lint clean. + +## Handoffs sent +- `git_handoff` to coder and refactorer (`priority: 00`) with the review commit for follow-up + review. diff --git a/psf-memo-client/specs/youtube-embed.feature b/psf-memo-client/specs/youtube-embed.feature index 112832d..5c0ea15 100644 --- a/psf-memo-client/specs/youtube-embed.feature +++ b/psf-memo-client/specs/youtube-embed.feature @@ -1,3 +1,7 @@ +# acceptance-mutation-manifest-begin +# {"version":1,"tested_at":"2026-09-04T16:25:46.345890783Z","feature_name":"YouTube Embed","feature_path":"/home/trout/work/psf-memo/.worktrees/architect/psf-memo-client/specs/youtube-embed.feature","background_hash":"0d66780cb1b8e277f0ada40a8ffe336dec7a8eaf658f19d2ea344815fb9bf26c","implementation_hash":"unknown","scenarios":[]} +# acceptance-mutation-manifest-end + # Scenarios: YouTube Embed - 1, YouTube Embed - 2, YouTube Embed - 3 # # When a post's text contains a YouTube link, the client renders an embedded diff --git a/psf-memo-client/src/services/youtube-embed.js b/psf-memo-client/src/services/youtube-embed.js index 72f9956..d7a1c63 100644 --- a/psf-memo-client/src/services/youtube-embed.js +++ b/psf-memo-client/src/services/youtube-embed.js @@ -111,3 +111,7 @@ module.exports = { extractYouTubeVideoId, parsePostText } + +// mutate4javascript-manifest-begin +// {"version":1,"tested_at":"2026-09-04T16:24:43.962Z","module_hash":"102305b0a6a2998c7271bbfdecc272fbe426ecc2dac61f89f74cde410372c52f","functions":[{"id":"func/validVideoId","name":"validVideoId","line":14,"end_line":16,"hash":"cf16fbd8a480a0cffa21699ddb96d1c7ecf746ad06503b97022f7e4a7376ddff"},{"id":"func/parseCandidate","name":"parseCandidate","line":20,"end_line":28,"hash":"7b2588b1e113b390c14d08a99befb0a2bd844ed4b0057a577d6dc97c8f3c4d20"},{"id":"func/videoIdFromWatchUrl","name":"videoIdFromWatchUrl","line":31,"end_line":34,"hash":"dde2829ca98d70aae061202de77df0adb7fb774a5aca1ba080e062b157391672"},{"id":"func/videoIdFromShortUrl","name":"videoIdFromShortUrl","line":37,"end_line":40,"hash":"a0a6934232c55cfd7d80c825fd65f337c9fd35a5cbb09ab12b834c34b520624e"},{"id":"func/extractYouTubeVideoId","name":"extractYouTubeVideoId","line":46,"end_line":61,"hash":"9559c0264f22c094687249f330bb514a7514955c115384e292ddb9e16df8afe7"},{"id":"func/pushText","name":"pushText","line":66,"end_line":68,"hash":"abda2060349c814451b88fe350ca235069afdc769eaa3ffa90e9d214673c71b9"},{"id":"func/parsePostText","name":"parsePostText","line":75,"end_line":107,"hash":"ab68477ea4145245c47def1a3ac2a0cfd4ed62725000bd1d669b7bde160435c5"}]} +// mutate4javascript-manifest-end diff --git a/psf-memo-client/test/unit/youtube-embed.test.js b/psf-memo-client/test/unit/youtube-embed.test.js index 94358de..2b7e9f2 100644 --- a/psf-memo-client/test/unit/youtube-embed.test.js +++ b/psf-memo-client/test/unit/youtube-embed.test.js @@ -35,6 +35,14 @@ test('extractYouTubeVideoId returns null when the watch URL has no v value', () assert.equal(extractYouTubeVideoId('https://www.youtube.com/watch?v='), null) }) +test('extractYouTubeVideoId returns null for a watch URL with an invalid video id', () => { + assert.equal(extractYouTubeVideoId('https://www.youtube.com/watch?v=abc!def'), null) +}) + +test('extractYouTubeVideoId returns null for a youtube.com URL that is not /watch', () => { + assert.equal(extractYouTubeVideoId('https://www.youtube.com/not-watch?v=abc'), null) +}) + test('extractYouTubeVideoId returns null when the short URL has no path id', () => { assert.equal(extractYouTubeVideoId('https://youtu.be/'), null) })