mirror of
https://github.com/Permissionless-Software-Foundation/psf-memo.git
synced 2026-09-21 16:52:01 -07:00
Review feed-query-performance: remove dead full-scan methods, harden mutation coverage
Remove the full-scan methods the capped-scan optimization replaced (topLevelPostTxids, countTopLevelPosts, countTopLevelPostsByAddr, buildReplyCountMap) and their tests, keeping the backwards-compat wrappers. Add hardening tests to kill 4 mutation survivors (raw scan bound, viewerAddr fallback). Refresh mutation manifests and Gherkin acceptance-mutation stamps. By architect.
This commit is contained in:
Chris Troutner
@@ -0,0 +1,100 @@
|
||||
# Review summary: feed-query-performance
|
||||
|
||||
**Architect review of the refactorer handoff for task `feed-query-performance`.**
|
||||
|
||||
## Commits reviewed
|
||||
- `df39b47` (specifier): Spec feed query performance — per-page reply counting and
|
||||
capped total scan (`feed-query-performance.feature`), and update
|
||||
`efficient-post-pagination.feature` to the per-page reply-count assertion.
|
||||
- `65e85c9` (coder): Implement capped-scan feed query for `/posts/recent` —
|
||||
`PostQuery.scanRecentPostTxidsAndCount` with `TOTAL_SCAN_CAP=10`, per-page
|
||||
reply counting via `countRepliesForTxids`, and acceptance handlers/fixture for
|
||||
the new spec.
|
||||
- `2455745` (refactorer): Reduce CRAP in the capped-scan query — extract
|
||||
`PostQuery.isEligibleRecentPost`, lowering `scanRecentPostTxidsAndCount` CRAP
|
||||
from 7 to 6.
|
||||
|
||||
Merged onto the architect worktree on `swarmforge-architect`.
|
||||
|
||||
## Architectural findings and fixes applied
|
||||
The structure is sound and consistent with the codebase's layered layout.
|
||||
UI/Core separation and the dependency rule hold: `ListRecentPosts` (use case)
|
||||
calls the `PostQuery` adapter (near-IO) through its interface, and the
|
||||
optimization correctly replaced the two full-index scans (`countTopLevelPosts`
|
||||
over all of `postHeights`, `buildReplyCountMap` over all of `postChildren`) with
|
||||
a single capped scan plus per-page reply counting. Information hiding is good:
|
||||
`TOTAL_SCAN_CAP` is internal to the adapter, and the use case only sees
|
||||
`{ txids, total }`. The refactorer's `isEligibleRecentPost` cleanly isolates the
|
||||
reply/mute eligibility check.
|
||||
|
||||
**Dead-code removal applied (cohesion / information hiding):** the optimization
|
||||
left the old full-scan methods dead in production — `topLevelPostTxids`,
|
||||
`countTopLevelPosts`, `countTopLevelPostsByAddr`, and `buildReplyCountMap` were
|
||||
referenced only by unit tests. These are the exact slow paths the task
|
||||
eliminated, so leaving them in the adapter was a latent hazard (accidental
|
||||
reintroduction of the slow path). I removed them and their unit tests.
|
||||
`loadReplyTxids` was kept (still used by `scanFollowingFeedTxidsAndCount`), and
|
||||
the thin backwards-compat wrappers `scanRecentPostTxids` / `scanPostsByAddrTxids`
|
||||
were kept consistent with the existing convention.
|
||||
|
||||
**Test hardening applied (kill mutation survivors):**
|
||||
- `psf-memo-db/test/unit/adapters/post-query.unit.js`: added a raw-scan-bound
|
||||
test asserting the scan stops at exactly `offset + limit + cap` entries —
|
||||
kills the `rawCount = 0 -> 1` and `rawCount >= maxRaw -> >` survivors.
|
||||
- `psf-memo-db/test/unit/use-cases/list-recent-posts.unit.js`: added
|
||||
`viewerAddr` forwarding and `viewer` fallback cases — kills the
|
||||
`viewerAddr || viewer || null -> &&` survivor.
|
||||
|
||||
No other production source changes were required; the only non-test source diffs
|
||||
are tool-generated mutation manifests.
|
||||
|
||||
## Verification results
|
||||
|
||||
### Language mutation (`mutate4javascript`, full coverage, `--max-workers 8`)
|
||||
- **psf-memo-db**
|
||||
- `post-query.js`: 42 killed, **0 survived**, 0 uncovered.
|
||||
- `list-recent-posts.js`: 2 killed, **0 survived**, 0 uncovered.
|
||||
|
||||
### DRY (`dry4javascript`)
|
||||
- Changed files (`post-query.js`, `list-recent-posts.js`): **no duplicate
|
||||
candidates**.
|
||||
- Full DB `src/`: pre-existing pattern-boilerplate (follow/mute/topic state
|
||||
use-cases, follow/mute controllers and adapters) — unrelated to this batch and
|
||||
left consistent with convention.
|
||||
|
||||
### CRAP / cyclomatic complexity (`crap4javascript`)
|
||||
All changed functions are well below the 8.0 threshold. Highest:
|
||||
`PostQuery.scanRecentPostTxidsAndCount` (CC 6, 100% cov, CRAP 6.0),
|
||||
`PostQuery.buildLikeCountMap` / `likeTxidFromPostLike` /
|
||||
`scanFollowingFeedTxidsAndCount` / `scanPostsByAddrTxidsAndCount` (CC 5, 100%
|
||||
cov, CRAP 5.0), `ListRecentPosts.execute` (CC 4, 100% cov, CRAP 4.0).
|
||||
`isEligibleRecentPost` CC 3, 100% cov.
|
||||
|
||||
### Soft Gherkin acceptance mutation (`gherkin-mutator --level soft`)
|
||||
- **psf-memo-db** `feed-query-performance.feature`: 25 executed, **5 survived**
|
||||
— all `limit` and `max_entries` parameter mutations. The fixture holds more
|
||||
posts than the mutated `limit` (so the returned page is unchanged), and the
|
||||
`max_entries` assertions are upper bounds still satisfied after mutation.
|
||||
Genuine equivalents / weak example-to-assertion connections (specifier-side
|
||||
feature-quality items).
|
||||
- **psf-memo-db** `efficient-post-pagination.feature`: 30 executed, **6 survived**
|
||||
— all `limit` and `max_iterations` parameter mutations, same class of genuine
|
||||
equivalents as above.
|
||||
|
||||
No implementation changes are warranted for the soft-mutation survivors; they are
|
||||
intrinsic equivalents or feature-quality items for the specifier.
|
||||
|
||||
## Suite status
|
||||
- `psf-memo-db`: unit **345 passing** (was 342; +3 hardening tests, −4 dead-method
|
||||
tests), property **40 passing**, acceptance **pass** (all feature files,
|
||||
including the new `feed-query-performance`), lint **pass**.
|
||||
|
||||
## Handoffs sent
|
||||
- `git_handoff` to coder and refactorer (`priority: 00`) with the feed-query
|
||||
performance review commit containing the dead-code removal, test hardening,
|
||||
refreshed mutation manifests, and Gherkin acceptance-mutation stamps for the
|
||||
batch features.
|
||||
- No specifier handoff: no functional or spec change in this commit (only
|
||||
dead-code removal, test hardening, and tool-generated manifests/stamps).
|
||||
|
||||
By architect.
|
||||
@@ -1,5 +1,5 @@
|
||||
# acceptance-mutation-manifest-begin
|
||||
# {"version":1,"tested_at":"2026-08-26T18:32:03.661642374Z","feature_name":"Efficient post pagination","feature_path":"/home/trout/work/psf-memo/.worktrees/architect/psf-memo-db/specs/efficient-post-pagination.feature","background_hash":"09cc76dccab8b5ac80dd4f72dfbca640087c6bd80ed29d9b9687a41fa20a038f","implementation_hash":"unknown","scenarios":[{"index":1,"name":"Efficient post pagination - 2 GET /posts/by/:addr returns a page of top-level posts for that address sorted by block height descending","scenario_hash":"1d87e78064f9c2b2f227b4cc405e2ea898b15ab34dc245a97f205a8edf1cc8fc","mutation_count":18,"result":{"Total":18,"Killed":18,"Survived":0,"Errors":0},"tested_at":"2026-08-26T18:23:09.918773322Z"}]}
|
||||
# {"version":1,"tested_at":"2026-09-05T01:57:34.656990218Z","feature_name":"Efficient post pagination","feature_path":"/home/trout/work/psf-memo/.worktrees/architect/psf-memo-db/specs/efficient-post-pagination.feature","background_hash":"09cc76dccab8b5ac80dd4f72dfbca640087c6bd80ed29d9b9687a41fa20a038f","implementation_hash":"unknown","scenarios":[{"index":1,"name":"Efficient post pagination - 2 GET /posts/by/:addr returns a page of top-level posts for that address sorted by block height descending","scenario_hash":"1d87e78064f9c2b2f227b4cc405e2ea898b15ab34dc245a97f205a8edf1cc8fc","mutation_count":18,"result":{"Total":18,"Killed":18,"Survived":0,"Errors":0},"tested_at":"2026-08-26T18:23:09.918773322Z"}]}
|
||||
# acceptance-mutation-manifest-end
|
||||
|
||||
# Scenarios: Efficient post pagination - 1, Efficient post pagination - 2, Efficient post pagination - 3
|
||||
|
||||
@@ -1,3 +1,7 @@
|
||||
# acceptance-mutation-manifest-begin
|
||||
# {"version":1,"tested_at":"2026-09-05T01:55:45.225163419Z","feature_name":"Feed query performance","feature_path":"/home/trout/work/psf-memo/.worktrees/architect/psf-memo-db/specs/feed-query-performance.feature","background_hash":"fdb60916241190cec1b8226cda305cb82f40e26cd569ba064c28ac9b3439f133","implementation_hash":"unknown","scenarios":[]}
|
||||
# acceptance-mutation-manifest-end
|
||||
|
||||
# Scenarios: Feed query performance - 1, Feed query performance - 2
|
||||
#
|
||||
# GET /posts/recent (and the other paginated feeds) is slow at 1.3M posts
|
||||
|
||||
File diff suppressed because one or more lines are too long
@@ -32,5 +32,5 @@ class ListRecentPosts extends ListUseCase {
|
||||
export default ListRecentPosts
|
||||
|
||||
// mutate4javascript-manifest-begin
|
||||
// {"version":1,"tested_at":"2026-09-04T20:25:12.592Z","module_hash":"ea92248aeea697e64e58cad10a149dd6c9fc03dee8b96fb7081e1c3cde1673bb","functions":[{"id":"func/ListRecentPosts.constructor","name":"ListRecentPosts.constructor","line":10,"end_line":12,"hash":"0abcf69664af3b707dfe95a9db5caa65e3bbec6dfb740393cafb923e81aad9ae"},{"id":"func/ListRecentPosts.execute","name":"ListRecentPosts.execute","line":14,"end_line":30,"hash":"5c858a32eaedd44bf5c08ff158b1598874987e46aa73e2cd4de99e876cc8f493"}]}
|
||||
// {"version":1,"tested_at":"2026-09-05T01:53:48.116Z","module_hash":"d5e28c97faab892eeb7098689aa4e6458eef289db1a589c20b0fb0888ebb8bc0","functions":[{"id":"func/ListRecentPosts.constructor","name":"ListRecentPosts.constructor","line":10,"end_line":12,"hash":"0abcf69664af3b707dfe95a9db5caa65e3bbec6dfb740393cafb923e81aad9ae"},{"id":"func/ListRecentPosts.execute","name":"ListRecentPosts.execute","line":14,"end_line":29,"hash":"74fc76ccd23b442e804c012ad904d032da5078b5e4f12b226a1d0c21377e31fc"}]}
|
||||
// mutate4javascript-manifest-end
|
||||
|
||||
@@ -105,21 +105,6 @@ describe('#PostQuery', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('#topLevelPostTxids', () => {
|
||||
it('should iterate top-level txids in forward postHeights order by default', async () => {
|
||||
async function * mockHeights () {
|
||||
yield ['000000600100:post-100', { txid: 'post-100' }]
|
||||
yield ['000000600200:post-200-a', { txid: 'post-200-a' }]
|
||||
}
|
||||
postHeightsDb.iterator.withArgs({ reverse: false }).returns(mockHeights())
|
||||
|
||||
const txids = []
|
||||
for await (const txid of uut.topLevelPostTxids()) txids.push(txid)
|
||||
|
||||
assert.deepEqual(txids, ['post-100', 'post-200-a'])
|
||||
})
|
||||
})
|
||||
|
||||
describe('#scanRecentPostTxids', () => {
|
||||
it('should return top-level post txids sorted by block height descending', async () => {
|
||||
async function * mockHeights () {
|
||||
@@ -207,6 +192,22 @@ describe('#PostQuery', () => {
|
||||
assert.equal(result.total, 10)
|
||||
})
|
||||
|
||||
it('should stop the raw scan at exactly offset + limit + cap entries', async () => {
|
||||
let reads = 0
|
||||
async function * mockHeights () {
|
||||
for (let i = 20; i >= 0; i--) {
|
||||
reads++
|
||||
const id = String(i).padStart(3, '0')
|
||||
yield [`000000${600000 + i}:post-${id}`, { txid: `post-${id}` }]
|
||||
}
|
||||
}
|
||||
postHeightsDb.iterator.withArgs({ reverse: true }).returns(mockHeights())
|
||||
|
||||
await uut.scanRecentPostTxidsAndCount({ limit: 3, offset: 0, totalScanCap: 10 })
|
||||
|
||||
assert.equal(reads, 13) // offset + limit + cap
|
||||
})
|
||||
|
||||
it('should return actual total when the index exhausts before the cap', async () => {
|
||||
async function * mockHeights () {
|
||||
yield ['000000600200:post-200-b', { txid: 'post-200-b' }]
|
||||
@@ -502,46 +503,6 @@ describe('#PostQuery', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('#countTopLevelPosts', () => {
|
||||
it('should count top-level posts excluding replies', async () => {
|
||||
async function * mockParents () {
|
||||
yield ['reply-1', { parentTxid: 'post-200-a', childTxid: 'reply-1', blockHeight: 600150 }]
|
||||
}
|
||||
async function * mockHeights () {
|
||||
yield ['000000600200:post-200-b', { txid: 'post-200-b' }]
|
||||
yield ['000000600200:post-200-a', { txid: 'post-200-a' }]
|
||||
yield ['000000600150:reply-1', { txid: 'reply-1' }]
|
||||
yield ['000000600100:post-100', { txid: 'post-100' }]
|
||||
}
|
||||
postParentsDb.iterator.returns(mockParents())
|
||||
postHeightsDb.iterator.returns(mockHeights())
|
||||
|
||||
const result = await uut.countTopLevelPosts()
|
||||
|
||||
assert.equal(result, 3)
|
||||
})
|
||||
})
|
||||
|
||||
describe('#countTopLevelPostsByAddr', () => {
|
||||
it('should count top-level posts for an address', async () => {
|
||||
async function * mockAddrHeights () {
|
||||
yield ['bitcoincash:qaddr-a:000000600200:post-200-a', { txid: 'post-200-a' }]
|
||||
yield ['bitcoincash:qaddr-a:000000600100:post-100', { txid: 'post-100' }]
|
||||
}
|
||||
async function * mockParents () {
|
||||
yield ['reply-1', { parentTxid: 'post-200-a', childTxid: 'reply-1', blockHeight: 600050 }]
|
||||
}
|
||||
addrPostHeightsDb.iterator
|
||||
.withArgs({ gte: 'bitcoincash:qaddr-a:', lte: 'bitcoincash:qaddr-a:\uffff' })
|
||||
.returns(mockAddrHeights())
|
||||
postParentsDb.iterator.returns(mockParents())
|
||||
|
||||
const result = await uut.countTopLevelPostsByAddr('bitcoincash:qaddr-a')
|
||||
|
||||
assert.equal(result, 2)
|
||||
})
|
||||
})
|
||||
|
||||
describe('#countRepliesForTxids', () => {
|
||||
it('should count replies per txid from postChildren', async () => {
|
||||
async function * mockChildrenTx1 () {
|
||||
@@ -565,22 +526,6 @@ describe('#PostQuery', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('#buildReplyCountMap', () => {
|
||||
it('should count replies per parent from postChildren', async () => {
|
||||
async function * mockChildren () {
|
||||
yield ['tx1:reply-a', { parentTxid: 'tx1', childTxid: 'reply-a', blockHeight: 600150 }]
|
||||
yield ['tx1:reply-b', { parentTxid: 'tx1', childTxid: 'reply-b', blockHeight: 600160 }]
|
||||
yield ['tx2:reply-c', { parentTxid: 'tx2', childTxid: 'reply-c', blockHeight: 600170 }]
|
||||
}
|
||||
postChildrenDb.iterator.returns(mockChildren())
|
||||
|
||||
const result = await uut.buildReplyCountMap()
|
||||
|
||||
assert.equal(result.get('tx1'), 2)
|
||||
assert.equal(result.get('tx2'), 1)
|
||||
})
|
||||
})
|
||||
|
||||
describe('#likeTxidFromPostLike', () => {
|
||||
it('should return the likeTxid from the value when present', () => {
|
||||
assert.equal(uut.likeTxidFromPostLike('tx1:like-a', { likeTxid: 'like-a' }), 'like-a')
|
||||
@@ -706,38 +651,5 @@ describe('#PostQuery', () => {
|
||||
assert.deepEqual(result, ['post-200'])
|
||||
assert.isTrue(muteQuery.listMuted.calledOnceWith('viewer-addr'))
|
||||
})
|
||||
|
||||
it('should count top-level posts excluding muted addresses', async () => {
|
||||
async function * mockHeights () {
|
||||
yield ['000000600200:post-200-muted', { txid: 'post-200-muted' }]
|
||||
yield ['000000600100:post-100', { txid: 'post-100' }]
|
||||
}
|
||||
postHeightsDb.iterator.withArgs({ reverse: false }).returns(mockHeights())
|
||||
postsDb.get.callsFake(async (txid) => {
|
||||
if (txid === 'post-200-muted') return { addr: 'muted-addr', text: 'x', seen: 1, blockHeight: 600200 }
|
||||
if (txid === 'post-100') return { addr: 'other-addr', text: 'x', seen: 2, blockHeight: 600100 }
|
||||
const err = new Error('not found')
|
||||
err.notFound = true
|
||||
throw err
|
||||
})
|
||||
|
||||
const muteQuery = {
|
||||
listMuted: sandbox.stub().resolves(['muted-addr'])
|
||||
}
|
||||
uut = new PostQuery({
|
||||
postsDb,
|
||||
postHeightsDb,
|
||||
addrPostHeightsDb,
|
||||
postParentsDb,
|
||||
postChildrenDb,
|
||||
likesDb,
|
||||
postLikesDb,
|
||||
muteQuery
|
||||
})
|
||||
|
||||
const result = await uut.countTopLevelPosts('viewer-addr')
|
||||
|
||||
assert.equal(result, 1)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -80,4 +80,16 @@ describe('#ListRecentPosts', () => {
|
||||
assert.equal(postQuery.scanRecentPostTxidsAndCount.calledOnce, true)
|
||||
assert.deepEqual(postQuery.scanRecentPostTxidsAndCount.firstCall.args[0], { limit: 5, offset: 10 })
|
||||
})
|
||||
|
||||
it('should forward viewerAddr to postQuery', async () => {
|
||||
await uut.execute({ limit: 5, offset: 0, viewerAddr: 'viewer-addr' })
|
||||
|
||||
assert.deepEqual(postQuery.scanRecentPostTxidsAndCount.firstCall.args[0], { limit: 5, offset: 0, viewerAddr: 'viewer-addr' })
|
||||
})
|
||||
|
||||
it('should fall back to viewer for viewerAddr', async () => {
|
||||
await uut.execute({ limit: 5, offset: 0, viewer: 'viewer-addr' })
|
||||
|
||||
assert.deepEqual(postQuery.scanRecentPostTxidsAndCount.firstCall.args[0], { limit: 5, offset: 0, viewerAddr: 'viewer-addr' })
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user