Merge architect topics-most-recent-order review

By specifier.
This commit is contained in:
Chris Troutner
2026-09-04 07:33:22 -07:00
6 changed files with 120 additions and 17 deletions
@@ -0,0 +1,45 @@
# Review: topics-most-recent-order
**By architect.**
## Task and commits reviewed
- Task: `topics-most-recent-order` — order `GET /topics` by most recent post.
- Inbound handoff: refactorer `57915081c6` (merged fast-forward onto `swarmforge-architect`).
- Reviewed commits: `bc07eda` (spec), `f45f2f9` (implementation), `5791508` (property-test comment), plus the merged `f7647c7`/`5bc61c7` history.
## Architectural findings and fixes applied
- **Finding (information hiding):** the refactorer's `TopicQuery.listTopics()` returned
`{ room, postCount, lastHeight }`, and that shape flowed unchanged through the
`ListTopics` use case to the REST controller, so `lastHeight` leaked into the public
`/topics` API response. The client does not use it, the spec does not require it, and
the controller's documented contract only exposes `room` and `postCount`.
- **Fix:** `listTopics()` now sorts by `lastHeight` internally and then strips it from the
returned objects, so the adapter's public contract stays `{ room, postCount }` and the
ordering key is hidden at the adapter boundary. No use-case/controller change needed.
- **Hardening:** added two unit tests pinning the block-height-0 edge cases
(a post at height 0, and a post with no height field) so the ordering key's default
handling is covered.
## Verification results
- **Language mutation** (`mutate4javascript src/adapters/topic-query.js --max-workers 8 --mutate-all`):
Killed 21, Survived 1, Uncovered 0.
- Survivor `line 56 > -> >=` in `listTopics` is a **genuine equivalent**: the max of a
set of equal heights is unchanged regardless of which equal element is kept, so `>`
and `>=` produce identical ordering. Documented, not chased.
- **DRY** (`dry4javascript src/adapters/topic-query.js`): no duplicate candidates.
- **Soft Gherkin acceptance mutation** (`gherkin-mutator --level soft` on `topic-read.feature`):
new scenario Topic Read - 5 has 1 mutation, 1 killed, 0 survived. Two survivors in the
pre-existing pagination scenario (Topic Read - 3) are intrinsic `limit`-value equivalents
(mutating a limit that exceeds available posts does not change the result); pre-existing,
not introduced by this task.
- **Cyclomatic complexity:** no new branches beyond the existing sort comparator; no concern.
## Suite status
- psf-memo-db: 317 unit passing, 39 property passing, lint clean, topic-read acceptance
scenarios 1-5 all PASS (including the new ordering scenario).
- psf-memo-client: build OK, 243 tests passing, lint clean (verified because the merged
commit also carried the `app-body/index.js` Profile `appData` change from prior work).
## Handoffs sent
- `git_handoff` to coder and refactorer (`priority: 00`) with the review commit for
follow-up review.
+13 -2
View File
@@ -373,7 +373,7 @@ async function loadTopicsWithPosts (world) {
{ key: 'bitcoin:post-200', room: 'bitcoin', txid: 'post-200', type: 'post', blockHeight: 200 },
{ key: 'bitcoin:addr-f', room: 'bitcoin', addr: 'addr-f', type: 'follow', unfollow: false },
{ key: 'cash:post-250', room: 'cash', txid: 'post-250', type: 'post', blockHeight: 250 },
{ key: 'dev:post-100', room: 'dev', txid: 'post-100', type: 'post', blockHeight: 100 },
{ key: 'dev:post-400', room: 'dev', txid: 'post-400', type: 'post', blockHeight: 400 },
{ key: 'lone:addr-f', room: 'lone', addr: 'addr-f', type: 'follow', unfollow: false }
]
@@ -381,7 +381,7 @@ async function loadTopicsWithPosts (world) {
'post-300': { addr: 'addr-a', text: 'hello bitcoin', seen: 1, blockHeight: 300 },
'post-200': { addr: 'addr-b', text: 'bitcoin again', seen: 2, blockHeight: 200 },
'post-250': { addr: 'addr-a', text: 'cash rules', seen: 3, blockHeight: 250 },
'post-100': { addr: 'addr-c', text: 'dev stuff', seen: 4, blockHeight: 100 }
'post-400': { addr: 'addr-c', text: 'dev stuff', seen: 4, blockHeight: 400 }
}
for (const entry of roomEntries) {
@@ -868,6 +868,17 @@ const handlers = [
}
}
},
{
name: 'response lists topics in order',
pattern: /^the response lists topics in order (<expected_order>)$/,
run (m, example, world) {
const expected = resolveParam(m[1], example).split(',').map((s) => s.trim())
const actual = world.getLastResponse().topics.map((t) => t.room)
if (expected.join(',') !== actual.join(',')) {
throw new Error(`Expected topics ${expected.join(',')}, got ${actual.join(',')}`)
}
}
},
{
name: 'request topic posts',
pattern: /^the client requests \/topics\/([^/]+)\/posts(?: with limit (<limit>) and offset (<offset>))?$/,
+1 -1
View File
@@ -1,5 +1,5 @@
# acceptance-mutation-manifest-begin
# {"version":1,"tested_at":"2026-08-28T15:47:38.414053749Z","feature_name":"Topic Read","feature_path":"/home/trout/work/psf-memo/.worktrees/architect/psf-memo-db/specs/topic-read.feature","background_hash":"922fd45b8606f723073833f0aaca0e6d92b03a263a6b54c60490843e0ac41cc0","implementation_hash":"unknown","scenarios":[{"index":0,"name":"Topic Read - 1 GET /topics lists distinct topics with their post counts","scenario_hash":"a425a0cbf18c7e7fcb7d01c3571556d4c184b013ad7990e724d95d8d100eebb9","mutation_count":8,"result":{"Total":8,"Killed":8,"Survived":0,"Errors":0},"tested_at":"2026-08-28T15:39:33.364455310Z"},{"index":1,"name":"Topic Read - 2 GET /topics/:room/posts returns the posts for a topic sorted by block height descending","scenario_hash":"cffb702052b0df29aa87510e0c45dd01b83c4607e7ce915bc0e660d601f79cce","mutation_count":4,"result":{"Total":4,"Killed":4,"Survived":0,"Errors":0},"tested_at":"2026-08-28T15:39:33.364455310Z"}]}
# {"version":1,"tested_at":"2026-09-04T14:07:09.030166090Z","feature_name":"Topic Read","feature_path":"/home/trout/work/psf-memo/.worktrees/architect/psf-memo-db/specs/topic-read.feature","background_hash":"922fd45b8606f723073833f0aaca0e6d92b03a263a6b54c60490843e0ac41cc0","implementation_hash":"unknown","scenarios":[{"index":4,"name":"Topic Read - 5 GET /topics orders topics by most recent post","scenario_hash":"fcb48583a39be06c616d4d1636d06ac7e985a16fa7d9f66d9c9c43e95639812c","mutation_count":1,"result":{"Total":1,"Killed":1,"Survived":0,"Errors":0},"tested_at":"2026-09-04T14:07:09.030166090Z"},{"index":0,"name":"Topic Read - 1 GET /topics lists distinct topics with their post counts","scenario_hash":"a425a0cbf18c7e7fcb7d01c3571556d4c184b013ad7990e724d95d8d100eebb9","mutation_count":8,"result":{"Total":8,"Killed":8,"Survived":0,"Errors":0},"tested_at":"2026-08-28T15:39:33.364455310Z"},{"index":1,"name":"Topic Read - 2 GET /topics/:room/posts returns the posts for a topic sorted by block height descending","scenario_hash":"cffb702052b0df29aa87510e0c45dd01b83c4607e7ce915bc0e660d601f79cce","mutation_count":4,"result":{"Total":4,"Killed":4,"Survived":0,"Errors":0},"tested_at":"2026-08-28T15:39:33.364455310Z"}]}
# acceptance-mutation-manifest-end
# Scenarios: Topic Read - 1, Topic Read - 2, Topic Read - 3, Topic Read - 4, Topic Read - 5
+21 -8
View File
@@ -42,21 +42,34 @@ class TopicQuery {
}
async listTopics () {
const counts = new Map()
const topics = new Map()
for await (const [key, value] of this.roomsDb.iterator()) {
const room = this.roomFromKey(key, value)
if (!counts.has(room)) {
counts.set(room, 0)
if (!topics.has(room)) {
topics.set(room, { postCount: 0, lastHeight: 0 })
}
const topic = topics.get(room)
if (value?.type === 'post') {
counts.set(room, counts.get(room) + 1)
topic.postCount++
const height = value?.blockHeight ?? 0
if (height > topic.lastHeight) {
topic.lastHeight = height
}
}
}
return Array.from(counts.entries())
.map(([room, postCount]) => ({ room, postCount }))
.sort((a, b) => a.room.localeCompare(b.room))
return Array.from(topics.entries())
.map(([room, { postCount, lastHeight }]) => ({ room, postCount, lastHeight }))
.sort((a, b) => {
if (b.lastHeight !== a.lastHeight) {
return b.lastHeight - a.lastHeight
}
return a.room.localeCompare(b.room)
})
// lastHeight is an internal ordering key; keep it out of the public
// API contract so the response exposes only room and postCount.
.map(({ room, postCount }) => ({ room, postCount }))
}
async getTopicPostTxids (room, { limit, offset }) {
@@ -115,5 +128,5 @@ class TopicQuery {
export default TopicQuery
// mutate4javascript-manifest-begin
// {"version":1,"tested_at":"2026-08-28T19:16:59.454Z","module_hash":"7b5597f76aed02abe591e652087cb1c560dd17a1154a62299f50f60303aa6e87","functions":[{"id":"func/TopicQuery.constructor","name":"TopicQuery.constructor","line":14,"end_line":32,"hash":"08f209a40b4ffc2962e5f399e78936005f3286ec37123a043850b12e0c20833b"},{"id":"func/TopicQuery.roomFromKey","name":"TopicQuery.roomFromKey","line":34,"end_line":37,"hash":"4175916ac2bb8f9f102c70750f336ed980db2a30a1f8f35645534303408aaece"},{"id":"func/TopicQuery.txidFromKey","name":"TopicQuery.txidFromKey","line":39,"end_line":42,"hash":"4f5f5c456f1e74a60acff54d1d5e2d982be53886c2a77c6eff3293133efa9c87"},{"id":"func/TopicQuery.listTopics","name":"TopicQuery.listTopics","line":44,"end_line":60,"hash":"9c524ef62e22d7a434768dc431c360f9c81b0c2a321b162a614f7f7e1e011e08"},{"id":"func/TopicQuery.getTopicPostTxids","name":"TopicQuery.getTopicPostTxids","line":62,"end_line":80,"hash":"4e0fccf874560249152d04d13879c24fbae0271bf7835716789dfafcff7dbd04"},{"id":"func/TopicQuery.isFollowingRoom","name":"TopicQuery.isFollowingRoom","line":83,"end_line":92,"hash":"a0cf48cc6f4842adf65830b41d536cd6a078c7adf6b837e57259491157e2e97d"},{"id":"func/TopicQuery.listRoomFollowers","name":"TopicQuery.listRoomFollowers","line":95,"end_line":106,"hash":"351d71881e8b0953366056f223a6f06326e8ecd946277abc372ae5771c0b0f08"},{"id":"func/TopicQuery.followAddrFromValue","name":"TopicQuery.followAddrFromValue","line":108,"end_line":112,"hash":"2ce68c35f539e1fd7694774059b4b2afd096014c99a87c7538f3cbb70e26c3c7"}]}
// {"version":1,"tested_at":"2026-09-04T14:04:10.576Z","module_hash":"ea611a3cf767ec311982df6121e6051bda322d6392175cdd89ab2c545d7c1a7b","functions":[{"id":"func/TopicQuery.constructor","name":"TopicQuery.constructor","line":14,"end_line":32,"hash":"08f209a40b4ffc2962e5f399e78936005f3286ec37123a043850b12e0c20833b"},{"id":"func/TopicQuery.roomFromKey","name":"TopicQuery.roomFromKey","line":34,"end_line":37,"hash":"4175916ac2bb8f9f102c70750f336ed980db2a30a1f8f35645534303408aaece"},{"id":"func/TopicQuery.txidFromKey","name":"TopicQuery.txidFromKey","line":39,"end_line":42,"hash":"4f5f5c456f1e74a60acff54d1d5e2d982be53886c2a77c6eff3293133efa9c87"},{"id":"func/TopicQuery.listTopics","name":"TopicQuery.listTopics","line":44,"end_line":73,"hash":"b96bd821ff6eb1e14abe4418004f74418d3276666ca185c544c90d2095b08474"},{"id":"func/TopicQuery.getTopicPostTxids","name":"TopicQuery.getTopicPostTxids","line":75,"end_line":93,"hash":"4e0fccf874560249152d04d13879c24fbae0271bf7835716789dfafcff7dbd04"},{"id":"func/TopicQuery.isFollowingRoom","name":"TopicQuery.isFollowingRoom","line":96,"end_line":105,"hash":"a0cf48cc6f4842adf65830b41d536cd6a078c7adf6b837e57259491157e2e97d"},{"id":"func/TopicQuery.listRoomFollowers","name":"TopicQuery.listRoomFollowers","line":108,"end_line":119,"hash":"351d71881e8b0953366056f223a6f06326e8ecd946277abc372ae5771c0b0f08"},{"id":"func/TopicQuery.followAddrFromValue","name":"TopicQuery.followAddrFromValue","line":121,"end_line":125,"hash":"2ce68c35f539e1fd7694774059b4b2afd096014c99a87c7538f3cbb70e26c3c7"}]}
// mutate4javascript-manifest-end
@@ -7,7 +7,7 @@
- listTopics conservation: the sum of postCounts equals the number of
post entries in the rooms store, each room's count matches its own post
entries, and the topics are returned sorted by room name.
entries, and the topics are returned sorted by most recent post.
- getTopicPostTxids ordering + pagination: posts are returned newest-first
by block height, the total matches the room's post entries, and the
offset/limit slice is exact.
@@ -86,7 +86,7 @@ function fixtureGen () {
}
}
test('listTopics conserves post counts and returns rooms sorted by name', async () => {
test('listTopics conserves post counts and returns rooms sorted by most recent post', async () => {
await forAll(
fixtureGen(),
async ({ entries, rooms }) => {
@@ -97,8 +97,18 @@ test('listTopics conserves post counts and returns rooms sorted by name', async
const totalPosts = topics.reduce((sum, t) => sum + t.postCount, 0)
if (totalPosts !== postEntries.length) return false
const expectedRooms = [...new Set(entries.map((e) => e.value.room))].sort((a, b) => a.localeCompare(b))
if (JSON.stringify(topics.map((t) => t.room)) !== JSON.stringify(expectedRooms)) return false
const expectedTopics = [...new Set(entries.map((e) => e.value.room))]
.map((room) => {
const heights = entries
.filter((e) => e.value.room === room && e.value.type === 'post')
.map((e) => e.value.blockHeight ?? 0)
return { room, lastHeight: heights.length ? Math.max(...heights) : 0 }
})
.sort((a, b) => {
if (b.lastHeight !== a.lastHeight) return b.lastHeight - a.lastHeight
return a.room.localeCompare(b.room)
})
if (JSON.stringify(topics.map((t) => t.room)) !== JSON.stringify(expectedTopics.map((t) => t.room))) return false
for (const topic of topics) {
const roomPosts = postEntries.filter((e) => e.value.room === topic.room).length
@@ -115,10 +115,10 @@ describe('#TopicQuery', () => {
])
})
it('should sort topics by room name', async () => {
it('should sort topics by most recent post descending', async () => {
async function * mockRooms () {
yield ['zoo:post-1', { room: 'zoo', txid: 'post-1', type: 'post', blockHeight: 1 }]
yield ['alpha:post-1', { room: 'alpha', txid: 'post-1', type: 'post', blockHeight: 1 }]
yield ['alpha:post-1', { room: 'alpha', txid: 'post-1', type: 'post', blockHeight: 2 }]
}
roomsDb.iterator.returns(mockRooms())
@@ -126,6 +126,30 @@ describe('#TopicQuery', () => {
assert.deepEqual(result.map((t) => t.room), ['alpha', 'zoo'])
})
it('should treat a post at block height 0 as the least recent', async () => {
async function * mockRooms () {
yield ['a:post-1', { room: 'a', txid: 'post-1', type: 'post', blockHeight: 0 }]
yield ['b:post-1', { room: 'b', txid: 'post-1', type: 'post', blockHeight: 1 }]
}
roomsDb.iterator.returns(mockRooms())
const result = await uut.listTopics()
assert.deepEqual(result.map((t) => t.room), ['b', 'a'])
})
it('should treat a post with no block height as height 0', async () => {
async function * mockRooms () {
yield ['a:post-1', { room: 'a', txid: 'post-1', type: 'post' }]
yield ['b:post-1', { room: 'b', txid: 'post-1', type: 'post', blockHeight: 1 }]
}
roomsDb.iterator.returns(mockRooms())
const result = await uut.listTopics()
assert.deepEqual(result.map((t) => t.room), ['b', 'a'])
})
})
describe('#getTopicPostTxids', () => {