Merge commit '7ca73cffdc' into swarmforge-coder

2026-09-21 16:52:01 -07:00 · 2026-09-04 10:00:45 -07:00
parent 59cce4426d 7ca73cffdc
commit bf51558800
3 changed files with 167 additions and 1 deletions
@@ -48,3 +48,49 @@
 ## Handoffs sent
 - `git_handoff` to coder and refactorer (`priority: 00`) with the review commit for follow-up
  review.
+
+---
+
+# Follow-up review: property tests (batch 20260904T164750Z)
+
+**By architect.**
+
+## Task and commits reviewed
+- Follow-up on `youtube-embed`: refactorer added property tests pinning down the parser's
+  invariants over broad random inputs.
+- Inbound handoff: refactorer `9ec8760b59` (merged onto `swarmforge-architect`).
+- Reviewed commit: `9ec8760b59` — new `psf-memo-client/test/property/youtube-embed.property.test.js`
+  (119 lines, 4 properties). No source change.
+
+## Architectural findings
+- **Good test placement:** property tests live in `test/property/`, separate from unit tests,
+  per the constitution (property tests are not part of normal unit coverage, mutation, CRAP,
+  or Gherkin mutation). They are run via the dedicated `npm run test:property` command.
+- **Sound invariants:** the four properties (round-trip reconstruction, segment shape with
+  URL-safe video ids, non-YouTube URLs staying text, and video-id round trips) are meaningful
+  and exercise the parser's URL splitting and trailing-punctuation handling over a token pool
+  that mixes words, YouTube links, non-YouTube links, and punctuation.
+- **Deterministic:** uses the shared seeded `harness.js` PRNG (seed 20260904), so runs are
+  reproducible. Tests run sequentially under node:test, so the shared rng stream is stable.
+- No structural or boundary issues; no changes required.
+
+## Verification results
+- **Property tests** (`npm run test:property`): 37 passing (33 prior + 4 new), 0 fail.
+- **Language mutation** (`mutate4javascript src/services/youtube-embed.js --max-workers 8 --mutate-all`):
+  Killed 6, Survived 1, Uncovered 0. The sole survivor (`line 82 1 -> 0` in `parsePostText`,
+  `match[1]` → `match[0]`) is the previously documented genuine equivalent (`URL_RE`'s capture
+  group spans the whole pattern). Property tests are excluded from mutation coverage by design.
+- **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.
+- **Unit tests** (`npm test`): 260 passing, 0 fail. **Lint:** clean.
+
+## Suite status
+- psf-memo-client: 260 unit passing, 37 property passing, lint clean. No source change, so
+  acceptance behavior is unchanged from the prior review.
+
+## Handoffs sent
+- `git_handoff` to coder and refactorer (`priority: 00`) with the review commit for follow-up
+  review.
@@ -0,0 +1,119 @@
+/*
+  Property tests for the YouTube embed parser.
+
+  The unit tests probe parsePostText / extractYouTubeVideoId at a few fixed
+  fixtures. These properties pin down the parser's invariants over broad
+  random inputs:
+
+    - Round trip: concatenating the segments (text for text segments, url for
+      youtube segments) reconstructs the original input exactly, including any
+      trailing punctuation that was stripped from a URL.
+    - Every youtube segment carries a non-empty, URL-safe video id.
+    - A URL that is not a youtube.com/watch or youtu.be link never becomes a
+      youtube segment; it stays plain text.
+*/
+
+'use strict'
+
+const test = require('node:test')
+const { seededRandom, forAll, intGen } = require('./harness')
+const {
+  extractYouTubeVideoId,
+  parsePostText
+} = require('../../src/services/youtube-embed')
+
+const rng = seededRandom(20260904)
+
+const WATCH_URL = 'https://www.youtube.com/watch?v=dQw4w9WgXcQ'
+const SHORT_URL = 'https://youtu.be/dQw4w9WgXcQ'
+const OTHER_URL = 'https://example.com/video'
+
+// A pool of tokens used to build random post text. Mixing words, YouTube
+// links, non-YouTube links, and punctuation exercises the parser's URL
+// splitting and trailing-punctuation handling.
+const TOKENS = [
+  'hello', 'world', 'check', 'this', 'out', 'memo', 'post', 'a', 'the',
+  ' ', '  ', '.', ',', '!', '?', ':', ';',
+  WATCH_URL, SHORT_URL, OTHER_URL,
+  'https://example.com/other/path?q=1',
+  'https://youtu.be/',
+  'https://www.youtube.com/watch?v='
+]
+
+function randomText () {
+  const n = intGen(rng, 0, 12)()
+  let text = ''
+  for (let i = 0; i < n; i++) {
+    text += TOKENS[Math.floor(rng() * TOKENS.length)]
+  }
+  return text
+}
+
+function reconstruct (segments) {
+  return segments
+    .map((s) => (s.type === 'youtube' ? s.url : s.text))
+    .join('')
+}
+
+test('parsePostText round-trips: segments reconstruct the original text', async () => {
+  await forAll(
+    () => randomText(),
+    async (text) => {
+      const segments = parsePostText(text)
+      return reconstruct(segments) === text
+    },
+    { label: 'youtube-embed round trip', samples: 2000 }
+  )
+})
+
+test('parsePostText yields only text and youtube segments with valid video ids', async () => {
+  await forAll(
+    () => randomText(),
+    async (text) => {
+      const segments = parsePostText(text)
+      for (const segment of segments) {
+        if (segment.type !== 'text' && segment.type !== 'youtube') return false
+        if (segment.type === 'youtube') {
+          if (!segment.videoId) return false
+          if (!/^[A-Za-z0-9_-]+$/.test(segment.videoId)) return false
+        }
+      }
+      return true
+    },
+    { label: 'youtube-embed segment shape', samples: 2000 }
+  )
+})
+
+test('parsePostText never turns a non-YouTube URL into a youtube segment', async () => {
+  await forAll(
+    () => randomText(),
+    async (text) => {
+      const segments = parsePostText(text)
+      for (const segment of segments) {
+        if (segment.type !== 'youtube') continue
+        // A youtube segment must have come from a youtube.com/watch or
+        // youtu.be URL, so its id must be extractable from that URL.
+        if (extractYouTubeVideoId(segment.url) !== segment.videoId) return false
+      }
+      return true
+    },
+    { label: 'youtube-embed non-youtube stays text', samples: 2000 }
+  )
+})
+
+test('extractYouTubeVideoId round-trips a valid watch and short URL', async () => {
+  await forAll(
+    () => {
+      const id = 'id' + Math.floor(rng() * 1e9).toString(36)
+      const kind = Math.floor(rng() * 2)
+      return kind === 0
+        ? `https://www.youtube.com/watch?v=${id}`
+        : `https://youtu.be/${id}`
+    },
+    async (url) => {
+      const id = url.split('v=')[1] || url.split('youtu.be/')[1]
+      return extractYouTubeVideoId(url) === id
+    },
+    { label: 'youtube-embed id round trip', samples: 2000 }
+  )
+})
@@ -42,7 +42,8 @@ if grep -q "Missing source file argument" \
 else
  bad "mutate4javascript"
 fi
-if psf-memo-client/node_modules/.bin/dry4javascript >/dev/null 2>&1; then
+if grep -q "Usage: dry4javascript" \
+    <<< "$(psf-memo-client/node_modules/.bin/dry4javascript --help 2>&1)"; then
  ok "dry4javascript"
 else
  bad "dry4javascript"