mirror of
https://github.com/Permissionless-Software-Foundation/psf-memo.git
synced 2026-09-21 16:52:01 -07:00
Merge architect profile-wallet-connect review
By coder.
This commit is contained in:
@@ -0,0 +1,70 @@
|
||||
# Profile Wallet Connect — Architectural Review Summary
|
||||
|
||||
**Task:** profile-wallet-connect
|
||||
**Commits reviewed:** `c294617` (profile Follow-button wiring fix), `8592db3`
|
||||
(gherkin-parser wrapper), `5c6bffd6de` (getViewerAddress property tests),
|
||||
merged on the architect branch
|
||||
**By:** architect
|
||||
|
||||
## Scope
|
||||
|
||||
Reviewed the refactorer's `profile-wallet-connect` feature across
|
||||
`psf-memo-client`: the new pure selector `src/services/profile-wallet.js`, its
|
||||
unit and property test suites, the profile page `index.js` wiring fix (Follow /
|
||||
Mute button address source), and a new `swarmforge/scripts/gherkin-parser`
|
||||
convenience wrapper.
|
||||
|
||||
## Architectural findings and fixes applied
|
||||
|
||||
- **`getViewerAddress` is a clean single-responsibility selector.** It prefers
|
||||
the reactive `bchWalletState.cashAddress`, falls back to
|
||||
`wallet.walletInfo.cashAddress`, and returns `null` when neither is present.
|
||||
Pure, dependency-free, trivially testable. The property-test oracle
|
||||
(`expectedGet`) and the four unit tests trace exactly to the source's
|
||||
precedence rule — traced line-by-line and consistent.
|
||||
- **`index.js` wiring fix is sound.** The Follow/Mute button address now comes
|
||||
from `getViewerAddress` (reactive-then-wallet) instead of only
|
||||
`wallet.walletInfo.cashAddress`, so the button appears once the reactive
|
||||
wallet state loads. `myAddr`, `wallet`, and `appProfiles` were extracted as
|
||||
locals and added to the effect dependency array, so the profile re-loads when
|
||||
the address becomes available. Component delegates address selection to the
|
||||
pure helper; dependency direction is correct (core helper ← component, no
|
||||
reverse coupling).
|
||||
- **Fixed a broken convenience script.** `swarmforge/scripts/gherkin-parser`
|
||||
delegated to `.swarmforge/tools/aps`, a path that did not exist anywhere in
|
||||
the monorepo (the established APS checkout is `./tmp/aps-spec`, per
|
||||
`docs/architect-startup.md` and `architect-startup.sh`). Corrected it to
|
||||
`tmp/aps-spec` and verified it now emits valid parser JSON.
|
||||
- **Mutation manifest added for the new module.** The language mutation tool
|
||||
recorded a differential-mutation baseline for `profile-wallet.js`
|
||||
(getViewerAddress fully covered, 2/2 mutants killed). Preserved so future
|
||||
runs can detect source drift.
|
||||
- Test/property separation respected: property tests live in
|
||||
`test/property/`, reuse the shared client `harness.js`, and are run via the
|
||||
dedicated `test:property` command.
|
||||
|
||||
## Verification results
|
||||
|
||||
- **Client unit tests:** **243 passing**, lint clean.
|
||||
- **Client property tests** (`test:property`): **33 passing** (incl. 3 new
|
||||
`getViewerAddress` precedence/fallback/absence properties).
|
||||
- **Language mutation** (`mutate4javascript`, `--max-workers 8`,
|
||||
`src/services/profile-wallet.js`): **Killed 2, Survived 0, Uncovered 0.**
|
||||
- **DRY (`dry4javascript`):** `No duplicate candidates found`.
|
||||
- **Soft Gherkin acceptance mutation** (`follow-user.feature --level soft`):
|
||||
**Total 6, Killed 0, Survived 6, Errors 0.** All six are single-character
|
||||
case mutations of the example `addr` (a cash address used identically on the
|
||||
setup and assertion sides of each scenario) — intrinsic equivalents, not
|
||||
chased.
|
||||
|
||||
## Suite status
|
||||
|
||||
- `psf-memo-client`: unit **243 passing**, property **33 passing**, lint clean.
|
||||
- The standalone React smoke test (`src/App.test.js`) is not part of the
|
||||
project's `npm test` glob and requires react-scripts/jsdom; it is unrelated to
|
||||
this change (same as the memo-db.js/adapter exclusion precedent).
|
||||
|
||||
## Handoffs sent
|
||||
|
||||
- `git_handoff` to coder and refactorer (`priority: 00`) with the merge/review
|
||||
commit for follow-up review.
|
||||
@@ -17,3 +17,7 @@ function getViewerAddress (appData) {
|
||||
}
|
||||
|
||||
module.exports = { getViewerAddress }
|
||||
|
||||
// mutate4javascript-manifest-begin
|
||||
// {"version":1,"tested_at":"2026-09-03T23:17:03.194Z","module_hash":"4ae57c493ddf87d2eed0fc3d1241829fb5a563a0dacdced4523170d7eb069640","functions":[{"id":"func/getViewerAddress","name":"getViewerAddress","line":13,"end_line":17,"hash":"5e0666eb50f9c9ad8ebd829551410179ba91b82c7f5fb862fa6e0c0613e6de47"}]}
|
||||
// mutate4javascript-manifest-end
|
||||
|
||||
@@ -0,0 +1,111 @@
|
||||
/*
|
||||
Property tests for the profile page wallet-address selector.
|
||||
|
||||
The unit tests check getViewerAddress at a few fixed shapes. These
|
||||
properties pin the precedence invariant over a broad input space:
|
||||
|
||||
- precedence: whenever the reactive bchWalletState.cashAddress is a truthy
|
||||
value it is returned, regardless of the wallet-field address.
|
||||
- fallback: when the reactive state carries no truthy address, the wallet
|
||||
object's walletInfo.cashAddress is used when present.
|
||||
- absence: when neither source has an address, the result is null, never a
|
||||
partial/empty object.
|
||||
*/
|
||||
|
||||
'use strict'
|
||||
|
||||
const test = require('node:test')
|
||||
const { seededRandom, forAll, intGen } = require('./harness')
|
||||
const { getViewerAddress } = require('../../src/services/profile-wallet')
|
||||
|
||||
const rng = seededRandom(20260903)
|
||||
const CHARS = 'qpzry9x8gf2tvdw0s3jn54khce6mua7l'
|
||||
|
||||
// Random cash-address-shaped string (never empty so it passes as truthy).
|
||||
function addressGen () {
|
||||
const len = intGen(rng, 40, 60)()
|
||||
let out = 'bitcoincash:q'
|
||||
for (let i = 0; i < len; i++) {
|
||||
out += CHARS[Math.floor(rng() * CHARS.length)]
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
// Pick a "reactive state address" which may be falsy (undefined/null/'') or a
|
||||
// real address string.
|
||||
function stateAddrGen () {
|
||||
const roll = rng()
|
||||
if (roll < 0.2) return undefined
|
||||
if (roll < 0.4) return null
|
||||
if (roll < 0.55) return ''
|
||||
return addressGen()
|
||||
}
|
||||
|
||||
// Build a wallet field: absent, present without an address, or present with a
|
||||
// randomly chosen address.
|
||||
function walletGen () {
|
||||
const roll = rng()
|
||||
if (roll < 0.25) return undefined
|
||||
if (roll < 0.4) return { walletInfo: {} }
|
||||
return { walletInfo: { cashAddress: addressGen() } }
|
||||
}
|
||||
|
||||
function buildAppData (stateAddr, wallet) {
|
||||
return {
|
||||
bchWalletState: { cashAddress: stateAddr },
|
||||
wallet,
|
||||
profiles: {}
|
||||
}
|
||||
}
|
||||
|
||||
function expectedGet (stateAddr, wallet) {
|
||||
if (stateAddr) return stateAddr
|
||||
if (wallet && wallet.walletInfo && wallet.walletInfo.cashAddress) {
|
||||
return wallet.walletInfo.cashAddress
|
||||
}
|
||||
return null
|
||||
}
|
||||
|
||||
function fixtureGen () {
|
||||
return () => ({
|
||||
stateAddr: stateAddrGen(),
|
||||
wallet: walletGen()
|
||||
})
|
||||
}
|
||||
|
||||
const gen = fixtureGen()
|
||||
|
||||
test('getViewerAddress matches the reactive-then-wallet precedence rule', async () => {
|
||||
await forAll(
|
||||
gen,
|
||||
({ stateAddr, wallet }) => {
|
||||
const appData = buildAppData(stateAddr, wallet)
|
||||
return getViewerAddress(appData) === expectedGet(stateAddr, wallet)
|
||||
},
|
||||
{ label: 'getViewerAddress precedence' }
|
||||
)
|
||||
})
|
||||
|
||||
test('getViewerAddress prefers the reactive state whenever it is truthy', async () => {
|
||||
await forAll(
|
||||
gen,
|
||||
({ stateAddr, wallet }) => {
|
||||
if (!stateAddr) return true
|
||||
const appData = buildAppData(stateAddr, wallet)
|
||||
return getViewerAddress(appData) === stateAddr
|
||||
},
|
||||
{ label: 'getViewerAddress reactive preference invariant' }
|
||||
)
|
||||
})
|
||||
|
||||
test('getViewerAddress returns null when no wallet address is available', async () => {
|
||||
await forAll(
|
||||
gen,
|
||||
({ stateAddr, wallet }) => {
|
||||
if (expectedGet(stateAddr, wallet) !== null) return true
|
||||
const appData = buildAppData(stateAddr, wallet)
|
||||
return getViewerAddress(appData) === null
|
||||
},
|
||||
{ label: 'getViewerAddress null-on-absent invariant' }
|
||||
)
|
||||
})
|
||||
@@ -1,6 +1,6 @@
|
||||
#!/usr/bin/env bash
|
||||
# Wrapper for the Babashka APS gherkin-parser task.
|
||||
# Delegates to the project-local APS checkout under .swarmforge/tools/aps.
|
||||
# Delegates to the project-local APS checkout under ./tmp/aps-spec.
|
||||
set -euo pipefail
|
||||
if [ "$#" -ne 2 ]; then
|
||||
echo "usage: gherkin-parser <feature-file> <json-output>" >&2
|
||||
@@ -9,6 +9,6 @@ fi
|
||||
FEATURE_FILE="$(realpath -m "$1")"
|
||||
JSON_OUTPUT="$(realpath -m "$2")"
|
||||
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
|
||||
APS_DIR="$(cd "$SCRIPT_DIR/../../.swarmforge/tools/aps" && pwd)"
|
||||
APS_DIR="$(cd "$SCRIPT_DIR/../../tmp/aps-spec" && pwd)"
|
||||
cd "$APS_DIR"
|
||||
exec bb gherkin-parser "$FEATURE_FILE" "$JSON_OUTPUT"
|
||||
|
||||
Reference in New Issue
Block a user