From c2bfbc3cfb74ee21391903a72c63c6f3135cf1ab Mon Sep 17 00:00:00 2001 From: Chris Troutner Date: Wed, 16 Sep 2026 07:40:15 -0700 Subject: [PATCH] Record post-image-rendering review and verification Commit the review summary and the client verification record (git_sha 19e57be, the review commit) alongside the review. All five client commands pass. By architect. --- docs/reviews/post-image-rendering-summary.md | 116 ++++++++++++++++++ .../post-image-rendering-verification.json | 46 +++++++ 2 files changed, 162 insertions(+) create mode 100644 docs/reviews/post-image-rendering-summary.md create mode 100644 docs/reviews/post-image-rendering-verification.json diff --git a/docs/reviews/post-image-rendering-summary.md b/docs/reviews/post-image-rendering-summary.md new file mode 100644 index 0000000..34fbe7c --- /dev/null +++ b/docs/reviews/post-image-rendering-summary.md @@ -0,0 +1,116 @@ +# Review summary: post-image-rendering + +**Architect review of the refactorer handoff for task `post-image-rendering`.** + +## Commits reviewed +- `53d7439` (specifier): Spec post image rendering + (`psf-memo-client/specs/post-image-rendering.feature`). +- `3971e07` (coder): Render image URLs inline in posts — pure + `isImageUrl`/`imageAltText` helpers in `src/services/post-links.js`, a + `PostImage` presentational component plus failed-image state in + `src/components/post-feed/post-content.js`, acceptance handlers/adapter, and + unit tests. +- `e77e174` (refactorer): Refactor post image rendering and cover image helpers + — extracted the pure `addFailedImage` set transition into + `src/services/failed-images.js`, DRY'd the acceptance no-link/no-image + assertions through `assertFeedHasNoElement`, added property tests for image + detection/alt text/rendering. + +Merged onto the architect worktree (`swarmforge-architect`) by fast-forwarding +to `e77e174`. + +**Architect review commit: `19e57be`** (code + manifests + unit test). This +summary and the verification record are committed directly on top of it, so +`git diff 19e57be HEAD` touches only `docs/`. The record's `git_sha` is +`19e57be`, the commit that contains the review source changes. + +## Architectural findings and fixes applied +The structure is sound and matches the codebase's layered layout. UI/Core +separation, the dependency rule, and information hiding all hold: + +- **UI/Core separation:** `parsePostLinks`, `isImageUrl`, and `imageAltText` + are pure text/URL functions with no React or network dependency, and + `addFailedImage` is a pure `Set` transition free of React and the DOM. All + four are directly unit- and property-testable. `PostImage`/`PostContent` are + presentational (`React.createElement`, no JSX, no IO), so the same module + drives the browser build and the Node acceptance adapter. +- **Dependency rule:** the component depends inward on the pure services + (`post-links`, `failed-images`, `youtube-embed`); none of those services + depends outward on the component, the DOM, or the acceptance layer. +- **Information hiding:** the services expose only + `parsePostLinks`/`isImageUrl`/`imageAltText`/`addFailedImage`; `PostImage` + hides the anchor/img markup and the fallback behavior; the `Set` + representation of failed images stays inside `PostContent`/`failed-images`. + The acceptance seam is the `initialFailedImages` prop, the minimal + synchronous injection needed because static server rendering cannot emit + image `error` events. +- **Local quality:** the `PostImage` fallback anchor repeats the plain-link + anchor attributes. `dry4javascript` reports no candidate (below its + structural threshold), so extracting a shared `PostLink` component would add + indirection without measured duplication gain; left as-is. + +One issue left by the handoff was fixed: + +1. **Unkilled language mutant (edge cases).** `mutate4javascript` left one + survivor in `isImageUrl` at line 83 (`if (typeof url !== 'string') return + false`; `false -> true`). The non-string guard was never exercised. Added + `isImageUrl rejects non-string input` (`null`, `undefined`, `12345`, `{}`) + to `test/unit/post-links.test.js`, which kills it. + +Tool-written artifacts committed with the review: the embedded +`mutate4javascript` manifests in `post-links.js`/`post-content.js` (now track +`isImageUrl`, `imageAltText`, and `PostImage`) and the `gherkin-mutator` +acceptance-mutation stamp at the top of `post-image-rendering.feature` +(scenarios 1 and 3 fully killed; scenario 2 withheld because of the survivor +below). + +## Verification results + +### Language mutation (`mutate4javascript`, `--max-workers 8`) +- **`src/services/post-links.js`**: 20 killed, **0 survived**, 0 uncovered + after the fix (1 survivor before). +- **`src/components/post-feed/post-content.js`**: 2 killed, **0 survived**, + 0 uncovered. +- **`src/services/failed-images.js`**: 0 mutation sites (no supported + operators), 0 uncovered. + +### DRY (`dry4javascript`) +- Changed files (`post-links.js`, `failed-images.js`, `post-content.js`): + **no duplicate candidates**. +- `acceptance/lib/handlers.js`: no new candidates involving the added + handlers or `assertFeedHasNoElement`; the pre-existing route/controller + boilerplate duplicates are untouched. + +### CRAP / cyclomatic complexity (`crap4javascript`) +All changed functions are below the 8.0 threshold: +`PostContent` (CC 6, 97.1% covered, CRAP 6.0), `parsePostLinks` (CC 6, 100%, +6.0), `isBareDomainBoundary` (CC 4, 100%, 4.0), `imageAltText` (CC 3, 100%, +3.0), `isImageUrl` (CC 3, 100%, 3.0), `addFailedImage` (CC 2, 100%, 2.0), +`PostImage` (CC 2, 100%, 2.0), `pushText` (CC 2, 100%, 2.0). + +### Soft Gherkin acceptance mutation (`gherkin-mutator --level soft`) +- **`post-image-rendering.feature`**: 26 executed, **25 killed**, **1 + survived**, 0 errors. +- Survivor: scenario 2, example 1, `read https://example.com/page now` -> + `reaD https://example.com/page now`. Scenario 2 asserts the link and the + absence of an image, not the surrounding prose, so a case mutation of the + setup-only word `read` cannot change the outcome. This is a weak + example-to-assertion connection (the same class as the documented + post-link-formatting trailing-prose survivors), not an implementation gap — + scenario 1 asserts its surrounding text case-sensitively, which is why its + mutants died. Not chased. + +## Suite status +`swarmforge/scripts/verify.sh client --record +docs/reviews/post-image-rendering-verification.json --task post-image-rendering` +-> **pass (5/5)**: unit **346 passing**, property **58 passing**, acceptance +**all 27 suites passing**, lint **pass**, build **pass**. + +## Handoffs sent +- End-of-chain `git_handoff` to the specifier (`priority: 50`, task + `post-image-rendering`) with the review commit so it can merge + `swarmforge-architect` into `master`. +- No coder/refactorer handoff: the review is a test addition plus tool-written + manifests, with no follow-up work for them. + +By architect. diff --git a/docs/reviews/post-image-rendering-verification.json b/docs/reviews/post-image-rendering-verification.json new file mode 100644 index 0000000..477ae6b --- /dev/null +++ b/docs/reviews/post-image-rendering-verification.json @@ -0,0 +1,46 @@ +{ + "schema_version": 1, + "task": "post-image-rendering", + "component": "psf-memo-client", + "git_sha": "19e57be1ecf7c1493ccaa21959bb076677f7f8e2", + "branch": "swarmforge-architect", + "timestamp": "2026-09-16T14:35:46.808Z", + "commands": [ + { + "name": "unit", + "command": "npm test", + "exit": 0, + "duration_ms": 7806, + "summary": "346 pass / 0 fail" + }, + { + "name": "property", + "command": "npm run test:property", + "exit": 0, + "duration_ms": 7728, + "summary": "58 pass / 0 fail" + }, + { + "name": "acceptance", + "command": "npm run test:acceptance", + "exit": 0, + "duration_ms": 12360, + "summary": "all 27 acceptance suites passed" + }, + { + "name": "lint", + "command": "npm run lint", + "exit": 0, + "duration_ms": 2476, + "summary": "ok" + }, + { + "name": "build", + "command": "npm run build", + "exit": 0, + "duration_ms": 59592, + "summary": "ok" + } + ], + "result": "pass" +}