diff --git a/docs/architect-process-notes.md b/docs/architect-process-notes.md index 7c0aca5..795b4cb 100644 --- a/docs/architect-process-notes.md +++ b/docs/architect-process-notes.md @@ -18,6 +18,21 @@ distinct from per-task verification results, which live in ## Tooling behavior / runtime +- **Pure modules can legitimately report 0 mutation sites.** `mutate4javascript` + only targets arithmetic, comparison, equality, boolean, logical, and `0<->1` + constant sites. A module built from `!` guards, ternaries, and template + literals (e.g. `block-explorer.js`, `like-result.js`) scans as `Total mutation + sites: 0` with `Killed: 0, Survived: 0`. Run `--scan` to confirm the zero is + structural and not a skipped/under-selected run before treating it as a pass. + +- **Run `dry4javascript` scoped to the changed files/dirs, not the whole client.** + A broad `src test acceptance` run reports hundreds of pre-existing duplicate + blocks (475 in the client on 2026-09-16) — mostly `acceptance/lib/handlers.js` + step-handler boilerplate and repeated older-suite test setup — which buries the + one or two task-local candidates. Scope the run to the changed production + files, tests, and adapters; the broad run is only a noise floor, consistent + with prior reviews. + - **Bare `dry4javascript` runs the full test suite (~1m50s).** It is a DRY analysis that invokes tests, so running it with no arguments is a slow full-suite run, not a fast readiness probe. Never use it as a startup smoke diff --git a/docs/reviews/like-result-modal-summary.md b/docs/reviews/like-result-modal-summary.md new file mode 100644 index 0000000..9132a8c --- /dev/null +++ b/docs/reviews/like-result-modal-summary.md @@ -0,0 +1,149 @@ +# like-result-modal — Architect Review + +Task: `like-result-modal` +Component: `psf-memo-client` +Base: `3486da3` (last merged architect review); inbound refactorer commit `e978516` + +## What was reviewed + +Inbound refactorer batch (priority 50), merged onto `swarmforge-architect` by +fast-forwarding `3486da3` -> `e978516`. The linear chain reviewed: + +- **`efe7a6c`** — specifier: *Record post-options-menu completion in backlog and briefing*. +- **`3af0d42`** — specifier: *Add like broadcast result specification*. Adds + `psf-memo-client/specs/like-broadcast-result.feature` (3 scenario outlines). +- **`a0040f9`** — coder: *Implement like broadcast result modal*. Keeps the + like/tip modal open after a successful like and shows the success message, the + like txid, and a block-explorer link. Adds the pure `LikeTipPage` result state, + the presentational `like-result.js`, the acceptance render adapter, step + handlers, and unit tests. +- **`e978516`** — refactorer: *Refactor like result modal and share the block + explorer link*. Extracts `src/services/block-explorer.js` as the single source + of the `bch.loping.net/tx` URL shared by the New Post result modal, the post + options menu, and the like/tip result; DRYs the new acceptance steps behind + `renderLikeBroadcastResult`; adds property tests. + +**Architect review commit: `e071881ea0`** — the `mutate4javascript` footer +manifests, the soft `gherkin-mutator` acceptance-mutation manifest stamp, and the +hardening changes below. The summary and verification record are committed on +top, so `git diff e071881ea0 HEAD` touches only `docs/`. The record's `git_sha` +is `e071881ea0`, the commit that contains the verified source state. + +## Architectural findings and fixes applied + +The refactorer's structure is sound: a framework-free controller, a +server-renderable presentational component, a pure shared URL module, and a +separate acceptance render adapter. The hardening work was mutation- and +duplication-driven. + +1. **UI/Core separation.** `src/services/block-explorer.js` is a pure leaf + (`if (!txid) return ''` + a template literal) with no React, DOM, or IO. + `LikeTipPage` owns the broadcast-result state machine and is exercised with no + browser; `LikeResult` only maps props to markup and is reached by the + acceptance adapter through `acceptance/lib/render-like-result.js`, which + server-renders the actual component. +2. **Dependency rule.** `new-post.js`, `post-options.js`, and `like-tip-page.js` + depend inward on `block-explorer.js`; nothing in the pure modules reaches out + to React, the router, or the wallet. No framework or persistence structure + leaks across the boundary. +3. **Information hiding / DRY.** The block-explorer base URL and link shape now + have exactly one definition. The old `EXPLORER_TX_BASE` literals in + `new-post.js`, `post-options.js`, and `like-tip-page.js` are gone; the + `EXPLORER_TX_BASE`/`explorerUrl`/`explorerTxUrl` names remain as thin aliases + for existing acceptance/test callers. +4. **Mutation survivors (like-tip-page).** Initial run: **13 killed, 8 survived, + 0 uncovered** across 21 sites. Every survivor was a missing assertion, not a + design fault: + - the `false` initial `tipping`/`modalOpen`, and the `deps.postTxid`/ + `deps.authorAddress` constructor seeds, were never asserted; + - `open`'s `<` dust comparison had no boundary case (balance exactly at 3000); + - the `open` success result's `true` was unchecked; + - `_parseTip`'s second `||` (null/undefined) was uncovered; + - `_handleSubmitFailure`'s `err.message || String(err)` survived because a + *broadcast* failure is re-derived by the superclass, so the value is only + observable on a *validation* failure. + Added unit coverage for each; re-run: **21 killed, 0 survived, 0 uncovered**. +5. **Mutation survivor (new-post).** The `NewPostPage` constructor's + `this.showResultModal = false` (line 32) was a pre-existing survivor. Added an + initial-state assertion; re-run: **10 killed, 0 survived, 0 uncovered**. +6. **Test duplication.** The first DRY pass flagged two structurally identical + test pairs in `like-tip-page.test.js`. Extracted a `submitLike()` helper and a + `balance` option on `makePage()`; the remaining pair (dismiss vs. reopen) is + semantically distinct and is left as structural test boilerplate. +7. **Result-modal controller duplication (documented, not changed).** + `LikeTipPage` mirrors the `showResultModal`/`lastResult`/`submit`/ + `dismissResult` shape of `NewPostPage`, with a different show/dismiss policy. + A shared result-modal controller would touch both controllers, their unit + tests, and acceptance handlers; per `docs/architect-process-notes.md` that is + a broad cross-module refactor beyond a single review, so it is recorded as a + follow-up candidate rather than folded into this task. Likewise, the React + `like-tip-modal.js` keeps its own transient `showResult`/`resultTxid` state; + it is the environmentally unsuitable shell that node unit and acceptance + mutation deliberately do not target, matching the existing component pattern. + +## Verification results + +### Language mutation (`mutate4javascript`, `--mutate-all`, `--max-workers 8`) + +| File | Sites | Killed | Survived | Uncovered | +|------|------:|-------:|---------:|----------:| +| `src/services/like-tip-page.js` | 21 | 21 | 0 | 0 | +| `src/services/new-post.js` | 10 | 10 | 0 | 0 | +| `src/services/post-options.js` | 10 | 10 | 0 | 0 | +| `src/services/block-explorer.js` | 0 | 0 | 0 | 0 | +| `src/components/post-feed/like-result.js` | 0 | 0 | 0 | 0 | + +`block-explorer.js` and `like-result.js` contain only `!`, a ternary, and a +template literal — constructs `mutate4javascript` does not target — so `--scan` +reports 0 sites; this is structural, not a skipped run. The two JSX/ESM +components (`like-tip-modal.js`, `post-feed-item.js`) are outside the node unit +test boundary and are covered end-to-end by acceptance. + +### DRY (`dry4javascript`) + +Focused run over the changed source, tests, and adapters: the only task-local +duplicate left is the semantically distinct dismiss/reopen test pair noted above. +A broad run over `src test acceptance` reports 475 blocks, all pre-existing +`acceptance/lib/handlers.js` step-handler boilerplate and repeated test setup +across older suites — none involves the new production modules. + +### CRAP / cyclomatic complexity (`crap4javascript`) + +All changed functions at or below the 8.0 threshold with **100% coverage**, e.g. +`LikeTipPage._parseTip` (CC 5, CRAP 5.0), `LikeTipPage.open` (CC 4, CRAP 4.0), +`LikeTipPage.getBroadcastMessage` (CC 3, CRAP 3.0), `NewPostPage.submit`/ +`dismissResult` (CC 3, CRAP 3.0), `blockExplorerTxUrl` (CC 2, CRAP 2.0), +`LikeResult` (CC 2, CRAP 2.0). + +### Soft Gherkin acceptance mutation (`gherkin-mutator --level soft`) + +`like-broadcast-result.feature`: **7 executed, 4 killed, 3 survived, 0 errors**. +The three survivors are consistent-value intrinsic equivalents — each mutated +example value is used consistently on both the setup and assertion sides: + +- `tip: 600 -> 601` and `tip: 25000 -> 25007` (Scenario 2): the tip is entered + and then asserted against the same `` parameter. +- Scenario 3 `liked_txid` with an injected `x`: Scenario 3's purpose is the + dismiss-closes-modal behavior; it reuses `` on the setup and click + steps but makes no txid-validity or broadcast assertion (Scenarios 1 and 2, + which do assert the broadcast, killed the same class of mutation). + +The tool stamped Scenario 0 (all mutations killed) and left Scenarios 1–2 +unstamped, as designed. + +### Suite status + +`swarmforge/scripts/verify.sh client --record +docs/reviews/like-result-modal-verification.json --task like-result-modal` +-> **pass (5/5)**: +unit **387 pass / 0 fail**, property **75 pass / 0 fail**, acceptance **all 29 +suites passed**, lint **ok**, build **ok**. Record `git_sha` = `e071881ea0`. + +## Handoffs sent + +- End-of-chain `git_handoff` to the specifier (task `like-result-modal`) with the + review commit so it can merge `swarmforge-architect` into `master`. +- No coder/refactorer handoff: the review is test hardening plus manifest refresh + with no follow-up work for those roles. + +By architect. diff --git a/docs/reviews/like-result-modal-verification.json b/docs/reviews/like-result-modal-verification.json new file mode 100644 index 0000000..4ad4f40 --- /dev/null +++ b/docs/reviews/like-result-modal-verification.json @@ -0,0 +1,46 @@ +{ + "schema_version": 1, + "task": "like-result-modal", + "component": "psf-memo-client", + "git_sha": "e071881ea06f9a7951352e5242319d21f7ba7997", + "branch": "swarmforge-architect", + "timestamp": "2026-09-16T19:31:39.772Z", + "commands": [ + { + "name": "unit", + "command": "npm test", + "exit": 0, + "duration_ms": 8374, + "summary": "387 pass / 0 fail" + }, + { + "name": "property", + "command": "npm run test:property", + "exit": 0, + "duration_ms": 9402, + "summary": "75 pass / 0 fail" + }, + { + "name": "acceptance", + "command": "npm run test:acceptance", + "exit": 0, + "duration_ms": 13394, + "summary": "all 29 acceptance suites passed" + }, + { + "name": "lint", + "command": "npm run lint", + "exit": 0, + "duration_ms": 2266, + "summary": "ok" + }, + { + "name": "build", + "command": "npm run build", + "exit": 0, + "duration_ms": 66200, + "summary": "ok" + } + ], + "result": "pass" +}