- Add the canonical per-component verification records (indexer, db, client)
pinned to the review commit 3f05488, and the architect review summary.
By architect.
7.5 KiB
Topic Metadata Columns — Architect Review
By architect.
Task and commits reviewed
- Task:
topic-metadata(refactorer handoffmerge_and_process refactorer dd11964174). - Merged
swarmforge-refactorer(fast-forward). Topic-metadata commits:a599b34Spec topic metadata columns8da0f09Implement topic metadata columns (coder)dd11964Refactor topic metadata: cut CRAP, share test doubles, harden property tests (refactorer)
- Architect review commit:
3f05488(survivor-killing tests + tool-written mutation manifests/stamps). Verification recordgit_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
Topicscomponent stays thin: it delegates load/pagination to the testableTopicDiscoveryPagecontroller and formats labels with the purerelative-time.jsservice (own unit tests). The controller also exposesgetLastSeenLabelfor the acceptance/unit seam. The component callsrelativeTime(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.jsonly decide when to callrecordTopicFollow/recordTopicPost.backfill-topic-indexes.jstakes injected LevelDB handles and holds no file-system concerns; the CLI wrapper opens the real stores.TopicQuery.listTopicsreads metadata fromtopicSummariesand ordering/pagination fromtopicRecencywithlimit: offset + limit, never iteratingrooms. - Information hiding (good).
getIfPresent/isNotFoundhide not-found handling;listTopicsdefaults missing metadata fields to 0 for legacy summaries. ThetopicSummariesrecord shape andtopicRecencyKeyare 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), droppingrecordTopicFollowfrom 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).
recordTopicFollowrecomputesfollowerCountfromsummary.followerCount ?? 0. If a room'sroomsDbrecord survives but itstopicSummaryDbsummary 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.jslistTopics()called with no arguments defaultslimit100 /offset0 (killedoffset = 0 -> 1).- A legacy summary without
postCountreports 0 (killed?? 0 -> 1). - A post without
blockHeightorders after a height-1 post, pinning the zero default (killedblockHeight ?? 0 -> 1).
psf-memo-db/test/unit/lib/backfill-topic-indexes.unit.js- A post without
blockHeightsummarizes atlastHeight0 and recency height 0 (killedvalue.blockHeight ?? 0 -> 1). topicRecencyKey(undefined|null, room)equalstopicRecencyKey(0, room)(killedblockHeight ?? 0 -> 1).
- A post without
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.js26/26 killed;topic-follow.js3/3 killed;topic-message.js3 killed / 1 survived. - DB:
topic-query.js29/29 killed (after hardening);backfill-topic-indexes.js11 killed / 2 survived. - Client:
relative-time.js8/8 killed;topic-discovery-page.js2/2 killed.
Total 82 killed, 3 survived.
Documented mutation equivalents:
psf-memo-indexer/src/use-cases/action-types/topic-message.jsline 30pushDatas[0] -> pushDatas[1]:handlePostnormalizes 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.jslines 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-stampplus 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_handofftospecifier(priority 50) to mergeswarmforge-architectintomaster, using the records/summary commit. - No coder/refactorer follow-up: the review adds tests and tool-written metadata only, with no production behavior change.