From af54b0ebe7092e041396d5b6073207d426ffa837 Mon Sep 17 00:00:00 2001 From: Chris Troutner Date: Fri, 18 Sep 2026 06:35:33 -0700 Subject: [PATCH] Record topic-metadata review and verification - Add the canonical per-component verification records (indexer, db, client) pinned to the review commit 3f05488, and the architect review summary. By architect. --- .../topic-metadata-client-verification.json | 46 ++++++ .../topic-metadata-db-verification.json | 39 +++++ docs/reviews/topic-metadata-summary.md | 149 ++++++++++++++++++ docs/reviews/topic-metadata-verification.json | 39 +++++ 4 files changed, 273 insertions(+) create mode 100644 docs/reviews/topic-metadata-client-verification.json create mode 100644 docs/reviews/topic-metadata-db-verification.json create mode 100644 docs/reviews/topic-metadata-summary.md create mode 100644 docs/reviews/topic-metadata-verification.json diff --git a/docs/reviews/topic-metadata-client-verification.json b/docs/reviews/topic-metadata-client-verification.json new file mode 100644 index 0000000..2bd9515 --- /dev/null +++ b/docs/reviews/topic-metadata-client-verification.json @@ -0,0 +1,46 @@ +{ + "schema_version": 1, + "task": "topic-metadata", + "component": "psf-memo-client", + "git_sha": "3f05488da024895005786de0b8dd6973045d7b46", + "branch": "swarmforge-architect", + "timestamp": "2026-09-18T13:34:46.555Z", + "commands": [ + { + "name": "unit", + "command": "npm test", + "exit": 0, + "duration_ms": 10359, + "summary": "441 pass / 0 fail" + }, + { + "name": "property", + "command": "npm run test:property", + "exit": 0, + "duration_ms": 12663, + "summary": "90 pass / 0 fail" + }, + { + "name": "acceptance", + "command": "npm run test:acceptance", + "exit": 0, + "duration_ms": 18818, + "summary": "all 33 acceptance suites passed" + }, + { + "name": "lint", + "command": "npm run lint", + "exit": 0, + "duration_ms": 2629, + "summary": "ok" + }, + { + "name": "build", + "command": "npm run build", + "exit": 0, + "duration_ms": 83937, + "summary": "ok" + } + ], + "result": "pass" +} diff --git a/docs/reviews/topic-metadata-db-verification.json b/docs/reviews/topic-metadata-db-verification.json new file mode 100644 index 0000000..cb26fff --- /dev/null +++ b/docs/reviews/topic-metadata-db-verification.json @@ -0,0 +1,39 @@ +{ + "schema_version": 1, + "task": "topic-metadata", + "component": "psf-memo-db", + "git_sha": "3f05488da024895005786de0b8dd6973045d7b46", + "branch": "swarmforge-architect", + "timestamp": "2026-09-18T13:32:38.005Z", + "commands": [ + { + "name": "unit", + "command": "npm test", + "exit": 0, + "duration_ms": 12653, + "summary": "392 passing" + }, + { + "name": "property", + "command": "npm run property", + "exit": 0, + "duration_ms": 52767, + "summary": "57 pass / 0 fail" + }, + { + "name": "acceptance", + "command": "npm run acceptance", + "exit": 0, + "duration_ms": 113747, + "summary": "all 16 acceptance suites passed" + }, + { + "name": "lint", + "command": "npm run lint", + "exit": 0, + "duration_ms": 3824, + "summary": "ok" + } + ], + "result": "pass" +} diff --git a/docs/reviews/topic-metadata-summary.md b/docs/reviews/topic-metadata-summary.md new file mode 100644 index 0000000..9065fb5 --- /dev/null +++ b/docs/reviews/topic-metadata-summary.md @@ -0,0 +1,149 @@ +# Topic Metadata Columns — Architect Review + +**By architect.** + +## Task and commits reviewed + +- Task: `topic-metadata` (refactorer handoff `merge_and_process refactorer dd11964174`). +- Merged `swarmforge-refactorer` (fast-forward). Topic-metadata commits: + - `a599b34` Spec topic metadata columns + - `8da0f09` Implement topic metadata columns (coder) + - `dd11964` Refactor topic metadata: cut CRAP, share test doubles, harden + property tests (refactorer) +- Architect review commit: `3f05488` (survivor-killing tests + tool-written + mutation manifests/stamps). Verification record `git_sha`: `3f05488da0`. +- Records/summary commit: (this commit). + +The feature adds `lastSeen` (epoch-ms of the newest topic post, 0 for +follow-only rooms) and `followerCount` to each `topicSummaries` record; exposes +them from `GET /topics`; rebuilds them in the backfill utility; and renders +four topic columns (name, relative time, post count, follower count) on the +client topics page. + +## Architectural findings + +- **UI/Core separation (good).** The React `Topics` component stays thin: it + delegates load/pagination to the testable `TopicDiscoveryPage` controller and + formats labels with the pure `relative-time.js` service (own unit tests). The + controller also exposes `getLastSeenLabel` for the acceptance/unit seam. The + component calls `relativeTime(topic.lastSeen, …)` directly rather than that + wrapper, but both share the same pure function, so the exercised logic is the + tested logic. Accepted as-is. +- **Dependency direction (good).** Indexer metadata maintenance is isolated in + `topic-indexing.js`; `topic-follow.js`/`topic-message.js` only decide *when* + to call `recordTopicFollow`/`recordTopicPost`. `backfill-topic-indexes.js` + takes injected LevelDB handles and holds no file-system concerns; the CLI + wrapper opens the real stores. `TopicQuery.listTopics` reads metadata from + `topicSummaries` and ordering/pagination from `topicRecency` with + `limit: offset + limit`, never iterating `rooms`. +- **Information hiding (good).** `getIfPresent`/`isNotFound` hide not-found + handling; `listTopics` defaults missing metadata fields to 0 for legacy + summaries. The `topicSummaries` record shape and `topicRecencyKey` are + duplicated between the indexer and the DB — unavoidable cross-deployable + protocol constants (the components share no module); both are pinned by + exact-key/field tests so they cannot drift silently. +- **Local quality (good).** The refactorer replaced the ternary follower delta + with `Number(isActive) - Number(wasActive)`, dropping `recordTopicFollow` from + CC 8 to CC 6. The indexer and DB share in-memory store doubles + (`test/support/memory-db.js`, `test/support/level-double.js`) instead of + re-creating them per test. +- **Observation (not changed).** `recordTopicFollow` recomputes + `followerCount` from `summary.followerCount ?? 0`. If a room's `roomsDb` + record survives but its `topicSummaryDb` summary is lost, an idempotent replay + (delta 0) would report 0 rather than the true active-follow count. This only + arises from a degraded store state that the backfill repairs, and no runtime + path reaches it, so it was left unchanged. + +## Fixes applied (kill language-mutation survivors) + +Added DB unit tests only; no production behavior changed. + +- `psf-memo-db/test/unit/adapters/topic-query.unit.js` + - `listTopics()` called with no arguments defaults `limit` 100 / `offset` 0 + (killed `offset = 0 -> 1`). + - A legacy summary without `postCount` reports 0 (killed `?? 0 -> 1`). + - A post without `blockHeight` orders after a height-1 post, pinning the + zero default (killed `blockHeight ?? 0 -> 1`). +- `psf-memo-db/test/unit/lib/backfill-topic-indexes.unit.js` + - A post without `blockHeight` summarizes at `lastHeight` 0 and recency + height 0 (killed `value.blockHeight ?? 0 -> 1`). + - `topicRecencyKey(undefined|null, room)` equals `topicRecencyKey(0, room)` + (killed `blockHeight ?? 0 -> 1`). + +These five survivors were the only real mutation gaps; all remaining survivors +are documented intrinsic equivalents. + +## Verification + +Records (`git_sha` `3f05488da024895005786de0b8dd6973045d7b46`): + +| File | Component | +|------|-----------| +| `docs/reviews/topic-metadata-verification.json` | psf-memo-indexer (canonical) | +| `docs/reviews/topic-metadata-db-verification.json` | psf-memo-db | +| `docs/reviews/topic-metadata-client-verification.json` | psf-memo-client | + +Suite status (all `pass`): + +- **psf-memo-indexer**: 113 unit, 12 property, 7 acceptance suites, lint ok. +- **psf-memo-db**: 392 unit (was 387; +5 review tests), 57 property, + 16 acceptance suites, lint ok. +- **psf-memo-client**: 441 unit, 90 property, 33 acceptance suites, lint ok, + build ok. + +Language mutation (`mutate4javascript`, `--max-workers 8`; `--mutate-all` +where the differential run under-selected the changed DB files): + +- Indexer: `topic-indexing.js` 26/26 killed; `topic-follow.js` 3/3 killed; + `topic-message.js` 3 killed / 1 survived. +- DB: `topic-query.js` 29/29 killed (after hardening); + `backfill-topic-indexes.js` 11 killed / 2 survived. +- Client: `relative-time.js` 8/8 killed; `topic-discovery-page.js` 2/2 killed. + +Total 82 killed, 3 survived. + +Documented mutation equivalents: + +- `psf-memo-indexer/src/use-cases/action-types/topic-message.js` line 30 + `pushDatas[0] -> pushDatas[1]`: `handlePost` normalizes a two-push payload and + always reads the text at index 1; index 0 is never inspected, so the action + prefix and the room are interchangeable there. +- `psf-memo-db/src/lib/backfill-topic-indexes.js` lines 43/45 `> -> >=`: on + equality the guard assigns the value it already holds, so the summary is + unchanged. + +DRY (`dry4javascript`, scoped to changed files): no candidates in the changed +production files. One candidate in tests — the identical `topicRecencyKey` +ordering property test in `psf-memo-db` and `psf-memo-indexer` — is +cross-component duplication mirroring the duplicated key function; not +reducible without a shared test package, consistent with the accepted protocol +constant. No change. + +CRAP (`crap4javascript`): every changed function is 100% covered and below the +8.0 threshold; highest CRAP/CC 6.0 (`recordTopicFollow`, `collectSummaries`, +`TopicQuery.getTopicPostTxids`, `relativeTime`). + +Soft Gherkin acceptance mutation (`gherkin-mutator --level soft --workers 8`): + +- DB `topic-metadata.feature`: 30 total, **30 killed**, 0 survived → the tool + wrote a `# mutation-stamp` plus the per-scenario manifest. +- Indexer `topic-metadata-indexing.feature`: 62 total, 14 killed, 48 survived. +- Client `topic-metadata-columns.feature`: 28 total, 16 killed, 12 survived. + +All survivors are intrinsic equivalents, not implementation gaps: single-character +case mutations of example values (`room`, `addr`, `txid`, `text`) are resolved +consistently on both the setup and assertion sides of their scenario; the +`height`/`firstHeight`/`secondHeight` mutations feed scenarios that do not +assert height; the `firstSeen`/`secondSeen` mutations do not cross the asserted +`max(seen)` (the killed mutations are the ones that do); and the client +`lastSeen` mutations stay inside the same relative-time bucket. These are +specifier-side feature-quality items (weak write/read connection), consistent +with prior reviews. The mutator wrote manifests into the indexer and client +features; those tool-written changes are committed as-is. + +## Handoffs + +- End-of-chain `git_handoff` to `specifier` (priority 50) to merge + `swarmforge-architect` into `master`, using the records/summary commit. +- No coder/refactorer follow-up: the review adds tests and tool-written + metadata only, with no production behavior change. diff --git a/docs/reviews/topic-metadata-verification.json b/docs/reviews/topic-metadata-verification.json new file mode 100644 index 0000000..f0aa9e0 --- /dev/null +++ b/docs/reviews/topic-metadata-verification.json @@ -0,0 +1,39 @@ +{ + "schema_version": 1, + "task": "topic-metadata", + "component": "psf-memo-indexer", + "git_sha": "3f05488da024895005786de0b8dd6973045d7b46", + "branch": "swarmforge-architect", + "timestamp": "2026-09-18T13:29:34.859Z", + "commands": [ + { + "name": "unit", + "command": "npm test", + "exit": 0, + "duration_ms": 12039, + "summary": "113 passing" + }, + { + "name": "property", + "command": "npm run property", + "exit": 0, + "duration_ms": 8080, + "summary": "12 pass / 0 fail" + }, + { + "name": "acceptance", + "command": "npm run acceptance", + "exit": 0, + "duration_ms": 2899, + "summary": "all 7 acceptance suites passed" + }, + { + "name": "lint", + "command": "npm run lint", + "exit": 0, + "duration_ms": 3556, + "summary": "ok" + } + ], + "result": "pass" +}