From 1ff2c020ac34df9f5b58bcf11ac30abd2f082e17 Mon Sep 17 00:00:00 2001 From: Chris Troutner Date: Tue, 15 Sep 2026 13:10:46 -0700 Subject: [PATCH] Review thread-query-bounds: hide postChildren representation, drop dead like scan Encapsulate the postChildren prefix scan in PostQuery.listChildTxids and have GetPostThread depend only on the PostQuery interface (listChildTxids, getPostOrNull). countRepliesForTxids now reuses listChildTxids. Remove the dead whole-database buildLikeCountMap that this task made obsolete. Cover the prefix-scan guards and cycle back-edge, realign the thread property tests to the adapter seam, and refresh mutation/Gherkin-acceptance manifests. By architect. --- docs/reviews/thread-query-bounds-summary.md | 103 +++++++++++++ .../specs/thread-query-performance.feature | 4 + psf-memo-db/src/adapters/post-query.js | 49 +++---- psf-memo-db/src/use-cases/get-post-thread.js | 42 +----- .../property/get-post-thread.property.test.js | 137 ++++++++++++------ .../test/property/like-count.property.test.js | 24 +-- .../test/unit/adapters/post-query.unit.js | 81 ++++++++--- .../unit/use-cases/get-post-thread.unit.js | 93 ++++-------- 8 files changed, 330 insertions(+), 203 deletions(-) create mode 100644 docs/reviews/thread-query-bounds-summary.md diff --git a/docs/reviews/thread-query-bounds-summary.md b/docs/reviews/thread-query-bounds-summary.md new file mode 100644 index 0000000..83c32f3 --- /dev/null +++ b/docs/reviews/thread-query-bounds-summary.md @@ -0,0 +1,103 @@ +# Review summary: thread-query-bounds + +**Architect review of the refactorer handoff for task `thread-query-bounds`.** + +## Commits reviewed +- `9db1137` (specifier): Add thread query performance specification + (`psf-memo-db/specs/thread-query-performance.feature`) and record both + implementation parts in the backlog. +- `3c4de9c` (coder): Implement bounded thread query for `/posts/:txid/thread` — + per-thread like counting via `countLikesForTxids` instead of the whole-DB + `buildLikeCountMap`, and a prefix-scan of `postChildren` per node instead of a + full-store walk. +- `386fea9` (refactorer): Add property tests for the thread query and cover + `GetPostThread` edge cases. + +Merged onto the architect worktree (`swarmforge-architect`) as `d422db0`. The +merge also fast-forwarded the architect branch to `master`'s client worktree +changes (new-post txid display, nav menu, tooltip, Dockerfile cleanup); those +were already on `master` and are outside this task, but their suites were run as +part of merge hardening. + +## Architectural findings and fixes applied +The core optimization is correct and worth keeping: thread work is now +proportional to the thread, not the database. The review found and fixed three +structural issues the handoff left behind. + +1. **Information hiding / dependency direction (the key fix).** The refactorer + moved the `postChildren` key format (`${parentTxid}:`, `:\uffff`) into the + `GetPostThread` use case, which also reached into + `postQuery.postChildrenDb.iterator(...)` and `postQuery.postsDb.get(...)` + directly. That couples a high-level use case to the LevelDB representation. + I added `PostQuery.listChildTxids(txid)` (adapter owns the key format and the + prefix bounds), made `GetPostThread` call `listChildTxids` and the existing + `PostQuery.getPostOrNull`, and made `countRepliesForTxids` delegate to + `listChildTxids` (removing the duplicated prefix-scan). `GetPostThread` now + depends only on the `PostQuery` interface. + +2. **Dead full-scan method removed (cohesion).** `PostQuery.buildLikeCountMap` + was the exact whole-database like scan this task eliminated; after the change + it was referenced only by tests. Removed it and its tests, consistent with + the `feed-query-performance` review precedent. `postTxidFromPostLike` and + `postLikeKey` were kept (still pinned by the key round-trip property test). + +3. **Uncovered guards now covered + property tests realigned.** The prefix-scan + guards (`child?.parentTxid !== txid`, `!child?.childTxid`) and the + `buildThreadNode` cycle guard were uncovered by the handoff tests. I moved the + prefix-bound assertions into `PostQuery` unit/property tests, added unit cases + for the parent-mismatch and missing-child guards, added a cycle test, and + repurposed the `like-count` property test to the live `countLikesForTxids` + path. + +The only production behavior change is the removal of the dead method; thread +results, ordering, and like counts are unchanged. + +## Verification results + +### Language mutation (`mutate4javascript`, full mutation, `--max-workers 8`) +- **`psf-memo-db/src/adapters/post-query.js`**: 37 killed, **0 survived**, 0 uncovered. +- **`psf-memo-db/src/use-cases/get-post-thread.js`**: 10 killed, **0 survived**, 0 uncovered. +- The differential/default run only selected 1 changed site after the manifest + refresh, so both files were re-run with `--mutate-all` to guarantee the new + `listChildTxids` and removed method were actually mutated. Manifests were + refreshed by the tool (no hand edits). + +### DRY (`dry4javascript`) +- Changed files (`post-query.js`, `get-post-thread.js`): **no duplicate + candidates**. The `postChildren` prefix-scan is now single-sourced in + `PostQuery.listChildTxids`. + +### CRAP / cyclomatic complexity (`crap4javascript`) +All changed functions are well below the 8.0 threshold and 100% covered. Highest: +`PostQuery.scanRecentPostTxidsAndCount` (CC 6, CRAP 6.0, pre-existing), +`GetPostThread.buildThreadNode` (CC 5, CRAP 5.0), `PostQuery.likeTxidFromPostLike` +/ `scanFollowingFeedTxidsAndCount` / `scanPostsByAddrTxidsAndCount` (CC 5, +CRAP 5.0). `PostQuery.listChildTxids` is CC 4, CRAP 4.0; `countRepliesForTxids` +is CC 2, CRAP 2.0. + +### Soft Gherkin acceptance mutation (`gherkin-mutator --level soft`) +- **`thread-query-performance.feature`**: 8 executed, **6 killed**, **2 survived**. + Both survivors are `max_entries` upper-bound mutations + (`1 -> 3` and `0 -> 7`). The assertion is `reads <= max_entries`, so raising + the bound can never fail: the fixture performs 1 and 0 `postChildren` reads + respectively, still below the larger bound. These are intrinsic equivalents / + weak example-to-assertion connections (the same class documented for + `feed-query-performance`), not implementation gaps; no change warranted. The + feature stamp is correctly withheld because survivors remain. + +## Suite status +- `psf-memo-db`: unit **357 passing**, property **44 passing**, acceptance + **11/11 generated files pass** (including `thread-query-performance`), + lint **pass**. +- `psf-memo-client` (merge hardening): unit **309 passing**, lint **pass**, + build **pass**. +- `psf-memo-indexer` (merge hardening): unit **85 passing**, lint **pass**. + +## Handoffs sent +- `git_handoff` to coder and refactorer (`priority: 00`) with the review commit + (adapter encapsulation, dead-code removal, test hardening, refreshed mutation + manifests, Gherkin acceptance-mutation manifest). +- No specifier handoff: no functional or spec change in the review commit + (structural refactor, tests, and tool-generated manifests only). + +By architect. diff --git a/psf-memo-db/specs/thread-query-performance.feature b/psf-memo-db/specs/thread-query-performance.feature index 9f5a374..e577693 100644 --- a/psf-memo-db/specs/thread-query-performance.feature +++ b/psf-memo-db/specs/thread-query-performance.feature @@ -1,3 +1,7 @@ +# acceptance-mutation-manifest-begin +# {"version":1,"tested_at":"2026-09-15T20:05:07.042976100Z","feature_name":"Thread query performance","feature_path":"/home/trout/work/psf-memo/.worktrees/architect/psf-memo-db/specs/thread-query-performance.feature","background_hash":"e0defd91132ae900d79c841fc5ebe05d91f6bd154a763b742a951e6d5c3626fe","implementation_hash":"sha256:f7082c686e63a5297f4d1964224d7256faf59f5eea13e9f6de4d73ad6401a2ed","scenarios":[]} +# acceptance-mutation-manifest-end + # Scenarios: Thread query performance - 1 # # GET /posts/:txid/thread currently does work proportional to the whole diff --git a/psf-memo-db/src/adapters/post-query.js b/psf-memo-db/src/adapters/post-query.js index 801150c..0f112da 100644 --- a/psf-memo-db/src/adapters/post-query.js +++ b/psf-memo-db/src/adapters/post-query.js @@ -52,8 +52,8 @@ class PostQuery { this.scanPostsByAddrTxids = this.scanPostsByAddrTxids.bind(this) this.loadPostsByTxids = this.loadPostsByTxids.bind(this) this.countRepliesForTxids = this.countRepliesForTxids.bind(this) + this.listChildTxids = this.listChildTxids.bind(this) this.countLikesForTxids = this.countLikesForTxids.bind(this) - this.buildLikeCountMap = this.buildLikeCountMap.bind(this) this.txidFromPostHeight = this.txidFromPostHeight.bind(this) this.txidFromAddrPostHeight = this.txidFromAddrPostHeight.bind(this) this.getPostOrNull = this.getPostOrNull.bind(this) @@ -108,18 +108,33 @@ class PostQuery { } } + // Prefix-scan the postChildren index for one parent, returning its child + // txids. The key is `${parentTxid}:${childTxid}`; the \uffff upper bound + // covers every child txid, and the parentTxid guard rejects entries whose + // parent is merely a string prefix of txid. + async listChildTxids (txid) { + const childTxids = [] + const end = ':\uffff' + + for await ( + const [, child] + of this.postChildrenDb.iterator({ gte: `${txid}:`, lte: `${txid}${end}` }) + ) { + if (child?.parentTxid !== txid) continue + if (!child?.childTxid) continue + + childTxids.push(child.childTxid) + } + + return childTxids + } + // Count replies for each txid by prefix-scanning postChildren. async countRepliesForTxids (txids) { const counts = new Map() - const end = ':\uffff' for (const txid of txids) { - const prefix = `${txid}:` - let count = 0 - for await (const [, child] of this.postChildrenDb.iterator({ gte: prefix, lte: `${txid}${end}` })) { - if (child?.parentTxid === txid) count++ - } - counts.set(txid, count) + counts.set(txid, (await this.listChildTxids(txid)).length) } return counts @@ -149,22 +164,6 @@ class PostQuery { return this.txidFromKeyParts(key) } - // Build a global like-count map from the postLikes secondary index, - // ignoring likes whose target post no longer exists. - async buildLikeCountMap () { - const counts = new Map() - - for await (const [key, value] of this.postLikesDb.iterator()) { - const postTxid = this.postTxidFromPostLike(key, value) - if (!postTxid) continue - const post = await this.getPostOrNull(postTxid) - if (!post) continue - counts.set(postTxid, (counts.get(postTxid) || 0) + 1) - } - - return counts - } - postTxidFromPostLike (key, value) { if (value && typeof value.postTxid === 'string') return value.postTxid const parts = String(key).split(':') @@ -324,5 +323,5 @@ class PostQuery { export default PostQuery // mutate4javascript-manifest-begin -// {"version":1,"tested_at":"2026-09-05T01:46:35.715Z","module_hash":"6be7884d4d46fc0538f332dd781a68cfa6a939d5ebb94d186d18847eaa6e7633","functions":[{"id":"func/PostQuery.constructor","name":"PostQuery.constructor","line":17,"end_line":64,"hash":"61d79608f787f096d0c7196452c2f0b37ffff36e19e4ab8f3628652f7e1c0e6a"},{"id":"func/PostQuery.padHeight","name":"PostQuery.padHeight","line":66,"end_line":68,"hash":"be6c442a4d3d86ab3b60314756b7f7c0592479c21cb3b2e1273dcf139a84fb00"},{"id":"func/PostQuery.postHeightKey","name":"PostQuery.postHeightKey","line":70,"end_line":72,"hash":"2d4dff9464aa4c1e805da5de2ba314fbd856530c046705237ea848d5feff8c7c"},{"id":"func/PostQuery.addrPostHeightKey","name":"PostQuery.addrPostHeightKey","line":74,"end_line":76,"hash":"48579f08593cee36cc2f107c2526ac9def687b354cc5e54089defa3fd37117eb"},{"id":"func/PostQuery.postLikeKey","name":"PostQuery.postLikeKey","line":78,"end_line":80,"hash":"5d16caac2cf88932702b28f9d95927183f8904948eb8c16c7e8c672cd3a0780c"},{"id":"func/PostQuery.txidFromPostHeight","name":"PostQuery.txidFromPostHeight","line":82,"end_line":85,"hash":"0fd135c51089dcc9bd2f166bb9f28faaa6a03d7eb84cf03cb9b36e97cdb3139c"},{"id":"func/PostQuery.txidFromAddrPostHeight","name":"PostQuery.txidFromAddrPostHeight","line":87,"end_line":89,"hash":"314d5432292a78b273e3165343d3e09276600cbaf239f76bcd1b36fe5fcf5e10"},{"id":"func/PostQuery.txidFromKeyParts","name":"PostQuery.txidFromKeyParts","line":92,"end_line":95,"hash":"59fb8173599070095d87e5d6f56eeb999f79e75e4d3f3e435515e9db8ac71868"},{"id":"func/PostQuery.loadReplyTxids","name":"PostQuery.loadReplyTxids","line":97,"end_line":99,"hash":"74621495a3affc6ef8688b9d6a814a91c99b22c86c348d261c952aad25df1661"},{"id":"func/PostQuery.isReply","name":"PostQuery.isReply","line":101,"end_line":109,"hash":"be2e3729bd5f05cbfbab3630678eb5c389bd04c616d53b1e0c48c852f4cb25b1"},{"id":"func/PostQuery.countRepliesForTxids","name":"PostQuery.countRepliesForTxids","line":112,"end_line":126,"hash":"16268cd2a0f8db020a099b704c3d5a3cea5daf6a0936dd1f8d85c38809f006aa"},{"id":"func/PostQuery.countLikesForTxids","name":"PostQuery.countLikesForTxids","line":129,"end_line":144,"hash":"54e6e3e6467e72d9dd033c9861b3b9ef5e426a76aed7a13e8ac1a0f0389468a3"},{"id":"func/PostQuery.likeTxidFromPostLike","name":"PostQuery.likeTxidFromPostLike","line":146,"end_line":150,"hash":"7e07a9c278a9abae8f646b4f1950f88760f2d5c6445600a42fa42f36d029a45a"},{"id":"func/PostQuery.buildLikeCountMap","name":"PostQuery.buildLikeCountMap","line":154,"end_line":166,"hash":"a82ca3b46d68426edbe25f176c4a6019ea5fb6abea4c5d5e84aff3b381f63c61"},{"id":"func/PostQuery.postTxidFromPostLike","name":"PostQuery.postTxidFromPostLike","line":168,"end_line":172,"hash":"da7f8d0c63dbc074fb25da1a31dde5075c2c3469b84a5c700bd4a2961879d8e9"},{"id":"func/PostQuery.getPostOrNull","name":"PostQuery.getPostOrNull","line":175,"end_line":177,"hash":"d06ce38cd8bde722482a749ff58740b174a054900519e67d2045f29a18fce3f7"},{"id":"func/PostQuery.scanRecentPostTxids","name":"PostQuery.scanRecentPostTxids","line":179,"end_line":182,"hash":"004bb510a7463333b7aba90f732e84aa3c16243b0c891d9697068d18723380f3"},{"id":"func/PostQuery.isEligibleRecentPost","name":"PostQuery.isEligibleRecentPost","line":185,"end_line":189,"hash":"92d427447b5d23b22ac34829419382457b06d8a74908cbe1bd19e65cab21d571"},{"id":"func/PostQuery.scanRecentPostTxidsAndCount","name":"PostQuery.scanRecentPostTxidsAndCount","line":196,"end_line":224,"hash":"d6a4a8fbecdb9b27b23917160e414db4e31195385a468b2d766cdbc6dba4a197"},{"id":"func/PostQuery.scanPostsByAddrTxidsAndCount","name":"PostQuery.scanPostsByAddrTxidsAndCount","line":230,"end_line":258,"hash":"a872d7a1f71ce1ba6a1dee46a820989cbbb7287dfc433051be1e15890dbc010b"},{"id":"func/PostQuery.scanPostsByAddrTxids","name":"PostQuery.scanPostsByAddrTxids","line":261,"end_line":264,"hash":"3498f2b9615e79b9472a5c7be26520c78db7c1661071c90a882f981d8acca477"},{"id":"func/PostQuery.loadPostsByTxids","name":"PostQuery.loadPostsByTxids","line":266,"end_line":282,"hash":"86b05107f3ecc240b2e734217cedf29ef44c48474bf31c1c8f75f2e4b4f84bea"},{"id":"func/PostQuery.scanFollowingFeedTxidsAndCount","name":"PostQuery.scanFollowingFeedTxidsAndCount","line":287,"end_line":311,"hash":"1ef4b7fb555d82c33971d3e0a4d2f630c42eb86261a78cd34765a5e3736e5946"},{"id":"func/PostQuery.isFolloweePost","name":"PostQuery.isFolloweePost","line":316,"end_line":321,"hash":"7825f4509ef5103d2340ecb1434b483b02c7b435a263777ca89e4a0d345f2be3"}]} +// {"version":1,"tested_at":"2026-09-15T19:56:31.806Z","module_hash":"09acd99b64b1cedb4e35403eac5c5f60207f8b55a47f4db36f295158150be77b","functions":[{"id":"func/PostQuery.constructor","name":"PostQuery.constructor","line":17,"end_line":64,"hash":"389e45e5bd381a002b5e98bde2c32805e5adc930290d78b85848ff2caae596d0"},{"id":"func/PostQuery.padHeight","name":"PostQuery.padHeight","line":66,"end_line":68,"hash":"be6c442a4d3d86ab3b60314756b7f7c0592479c21cb3b2e1273dcf139a84fb00"},{"id":"func/PostQuery.postHeightKey","name":"PostQuery.postHeightKey","line":70,"end_line":72,"hash":"2d4dff9464aa4c1e805da5de2ba314fbd856530c046705237ea848d5feff8c7c"},{"id":"func/PostQuery.addrPostHeightKey","name":"PostQuery.addrPostHeightKey","line":74,"end_line":76,"hash":"48579f08593cee36cc2f107c2526ac9def687b354cc5e54089defa3fd37117eb"},{"id":"func/PostQuery.postLikeKey","name":"PostQuery.postLikeKey","line":78,"end_line":80,"hash":"5d16caac2cf88932702b28f9d95927183f8904948eb8c16c7e8c672cd3a0780c"},{"id":"func/PostQuery.txidFromPostHeight","name":"PostQuery.txidFromPostHeight","line":82,"end_line":85,"hash":"0fd135c51089dcc9bd2f166bb9f28faaa6a03d7eb84cf03cb9b36e97cdb3139c"},{"id":"func/PostQuery.txidFromAddrPostHeight","name":"PostQuery.txidFromAddrPostHeight","line":87,"end_line":89,"hash":"314d5432292a78b273e3165343d3e09276600cbaf239f76bcd1b36fe5fcf5e10"},{"id":"func/PostQuery.txidFromKeyParts","name":"PostQuery.txidFromKeyParts","line":92,"end_line":95,"hash":"59fb8173599070095d87e5d6f56eeb999f79e75e4d3f3e435515e9db8ac71868"},{"id":"func/PostQuery.loadReplyTxids","name":"PostQuery.loadReplyTxids","line":97,"end_line":99,"hash":"74621495a3affc6ef8688b9d6a814a91c99b22c86c348d261c952aad25df1661"},{"id":"func/PostQuery.isReply","name":"PostQuery.isReply","line":101,"end_line":109,"hash":"be2e3729bd5f05cbfbab3630678eb5c389bd04c616d53b1e0c48c852f4cb25b1"},{"id":"func/PostQuery.listChildTxids","name":"PostQuery.listChildTxids","line":115,"end_line":130,"hash":"e2016c239c309e4e8771788c788941e308dad11103bc1247d971961ff532cea8"},{"id":"func/PostQuery.countRepliesForTxids","name":"PostQuery.countRepliesForTxids","line":133,"end_line":141,"hash":"874b637df6cb92b898c8f58340639794cdadd053227bd020812b1a7c8f2106c9"},{"id":"func/PostQuery.countLikesForTxids","name":"PostQuery.countLikesForTxids","line":144,"end_line":159,"hash":"54e6e3e6467e72d9dd033c9861b3b9ef5e426a76aed7a13e8ac1a0f0389468a3"},{"id":"func/PostQuery.likeTxidFromPostLike","name":"PostQuery.likeTxidFromPostLike","line":161,"end_line":165,"hash":"7e07a9c278a9abae8f646b4f1950f88760f2d5c6445600a42fa42f36d029a45a"},{"id":"func/PostQuery.postTxidFromPostLike","name":"PostQuery.postTxidFromPostLike","line":167,"end_line":171,"hash":"da7f8d0c63dbc074fb25da1a31dde5075c2c3469b84a5c700bd4a2961879d8e9"},{"id":"func/PostQuery.getPostOrNull","name":"PostQuery.getPostOrNull","line":174,"end_line":176,"hash":"d06ce38cd8bde722482a749ff58740b174a054900519e67d2045f29a18fce3f7"},{"id":"func/PostQuery.scanRecentPostTxids","name":"PostQuery.scanRecentPostTxids","line":178,"end_line":181,"hash":"004bb510a7463333b7aba90f732e84aa3c16243b0c891d9697068d18723380f3"},{"id":"func/PostQuery.isEligibleRecentPost","name":"PostQuery.isEligibleRecentPost","line":184,"end_line":188,"hash":"92d427447b5d23b22ac34829419382457b06d8a74908cbe1bd19e65cab21d571"},{"id":"func/PostQuery.scanRecentPostTxidsAndCount","name":"PostQuery.scanRecentPostTxidsAndCount","line":195,"end_line":223,"hash":"d6a4a8fbecdb9b27b23917160e414db4e31195385a468b2d766cdbc6dba4a197"},{"id":"func/PostQuery.scanPostsByAddrTxidsAndCount","name":"PostQuery.scanPostsByAddrTxidsAndCount","line":229,"end_line":257,"hash":"a872d7a1f71ce1ba6a1dee46a820989cbbb7287dfc433051be1e15890dbc010b"},{"id":"func/PostQuery.scanPostsByAddrTxids","name":"PostQuery.scanPostsByAddrTxids","line":260,"end_line":263,"hash":"3498f2b9615e79b9472a5c7be26520c78db7c1661071c90a882f981d8acca477"},{"id":"func/PostQuery.loadPostsByTxids","name":"PostQuery.loadPostsByTxids","line":265,"end_line":281,"hash":"86b05107f3ecc240b2e734217cedf29ef44c48474bf31c1c8f75f2e4b4f84bea"},{"id":"func/PostQuery.scanFollowingFeedTxidsAndCount","name":"PostQuery.scanFollowingFeedTxidsAndCount","line":286,"end_line":310,"hash":"1ef4b7fb555d82c33971d3e0a4d2f630c42eb86261a78cd34765a5e3736e5946"},{"id":"func/PostQuery.isFolloweePost","name":"PostQuery.isFolloweePost","line":315,"end_line":320,"hash":"7825f4509ef5103d2340ecb1434b483b02c7b435a263777ca89e4a0d345f2be3"}]} // mutate4javascript-manifest-end diff --git a/psf-memo-db/src/use-cases/get-post-thread.js b/psf-memo-db/src/use-cases/get-post-thread.js index 71d35ba..1b65351 100644 --- a/psf-memo-db/src/use-cases/get-post-thread.js +++ b/psf-memo-db/src/use-cases/get-post-thread.js @@ -16,8 +16,6 @@ class GetPostThread { this.buildThreadNode = this.buildThreadNode.bind(this) this.collectThreadTxids = this.collectThreadTxids.bind(this) this.attachLikeCounts = this.attachLikeCounts.bind(this) - this.fetchPostOrNull = this.fetchPostOrNull.bind(this) - this.loadChildTxids = this.loadChildTxids.bind(this) this.compareReplies = this.compareReplies.bind(this) } @@ -67,38 +65,6 @@ class GetPostThread { } } - async fetchPostOrNull (txid) { - try { - return await this.adapters.postQuery.postsDb.get(txid) - } catch (err) { - if (err.notFound || err.code === 'LEVEL_NOT_FOUND') { - return null - } - - throw err - } - } - - // Prefix-scan the postChildren index for this parent instead of walking the - // whole store, so thread loading stays proportional to the thread size. - async loadChildTxids (txid) { - const childTxids = [] - const prefix = `${txid}:` - const end = ':\uffff' - - for await ( - const [, child] - of this.adapters.postQuery.postChildrenDb.iterator({ gte: prefix, lte: `${txid}${end}` }) - ) { - if (child?.parentTxid !== txid) continue - if (!child?.childTxid) continue - - childTxids.push(child.childTxid) - } - - return childTxids - } - compareReplies (a, b) { const blockDifference = (a.blockHeight ?? 0) - (b.blockHeight ?? 0) @@ -110,15 +76,17 @@ class GetPostThread { return (a.seen ?? 0) - (b.seen ?? 0) } + // The postChildren representation and its prefix-scan bounds live in the + // PostQuery adapter, so this use case depends only on the adapter interface. async buildThreadNode (txid, visited = new Set()) { if (visited.has(txid)) return null visited.add(txid) - const post = await this.fetchPostOrNull(txid) + const post = await this.adapters.postQuery.getPostOrNull(txid) if (!post) return null - const childTxids = await this.loadChildTxids(txid) + const childTxids = await this.adapters.postQuery.listChildTxids(txid) const replies = [] @@ -147,5 +115,5 @@ class GetPostThread { export default GetPostThread // mutate4javascript-manifest-begin -// {"version":1,"tested_at":"2026-08-27T03:24:13.418Z","module_hash":"3270d9244ee4517c8d64770645d4b38e8d1937a4bb13beea79d0611960305aea","functions":[{"id":"func/GetPostThread.constructor","name":"GetPostThread.constructor","line":6,"end_line":21,"hash":"b23e82ab887fb68d199f23b4c6d94f6417556cbf7f7eeaeff7efdb5cce74f3f0"},{"id":"func/GetPostThread.execute","name":"GetPostThread.execute","line":23,"end_line":46,"hash":"968030072d4946cd4d00832fb334dc25d1286b24c1bc71252d30de6e3faebd38"},{"id":"func/GetPostThread.attachLikeCounts","name":"GetPostThread.attachLikeCounts","line":48,"end_line":55,"hash":"f7ba4717fd0cd0ba0c6cd3f7571c071a00bfc666076023a323d4848d6d4ba720"},{"id":"func/GetPostThread.fetchPostOrNull","name":"GetPostThread.fetchPostOrNull","line":57,"end_line":67,"hash":"b853fa9e3b121a3a0fae8c6828865039add3ec5950ae4cfa2db252f9db09446c"},{"id":"func/GetPostThread.loadChildTxids","name":"GetPostThread.loadChildTxids","line":69,"end_line":83,"hash":"9c1a7feff0dd92624b77127de70b2b13eea13854cf2e807b9c2c635b072a932f"},{"id":"func/GetPostThread.compareReplies","name":"GetPostThread.compareReplies","line":85,"end_line":94,"hash":"60393ba5137feda1df76d54f2b5c4da0728cc0dd556d72748ff3ba69328ba9a9"},{"id":"func/GetPostThread.buildThreadNode","name":"GetPostThread.buildThreadNode","line":96,"end_line":127,"hash":"e5ca3213bad9d22d9a87a2cf24f55cc5ffc4a6acd2d0e6c026ade991b490f43f"}]} +// {"version":1,"tested_at":"2026-09-15T19:59:55.632Z","module_hash":"bd62a2406b8d4458ad4aee839d4eae4dbc5d70e35bc1439974b33aa5e7785a3c","functions":[{"id":"func/GetPostThread.constructor","name":"GetPostThread.constructor","line":6,"end_line":20,"hash":"0ef50404634f2ede06763d4c200988ab6c1d2d260b9a649e226d9b18263d6af1"},{"id":"func/GetPostThread.execute","name":"GetPostThread.execute","line":22,"end_line":48,"hash":"50af0b17eca5ab1b5a17cb9a0dbdd44e11730a0c38089eff40bd50363274cd49"},{"id":"func/GetPostThread.collectThreadTxids","name":"GetPostThread.collectThreadTxids","line":50,"end_line":57,"hash":"a69b4b97ea19185e0e5fffb6e1cac8b6dee8721bd7a3702edda6845b400ab2a0"},{"id":"func/GetPostThread.attachLikeCounts","name":"GetPostThread.attachLikeCounts","line":59,"end_line":66,"hash":"f7ba4717fd0cd0ba0c6cd3f7571c071a00bfc666076023a323d4848d6d4ba720"},{"id":"func/GetPostThread.compareReplies","name":"GetPostThread.compareReplies","line":68,"end_line":77,"hash":"60393ba5137feda1df76d54f2b5c4da0728cc0dd556d72748ff3ba69328ba9a9"},{"id":"func/GetPostThread.buildThreadNode","name":"GetPostThread.buildThreadNode","line":81,"end_line":112,"hash":"47bb08b598c94fd0d78a238785c3b47f724fd892f57f79c5c43a2b4ef7f52454"}]} // mutate4javascript-manifest-end diff --git a/psf-memo-db/test/property/get-post-thread.property.test.js b/psf-memo-db/test/property/get-post-thread.property.test.js index 4f35adf..f12e4b8 100644 --- a/psf-memo-db/test/property/get-post-thread.property.test.js +++ b/psf-memo-db/test/property/get-post-thread.property.test.js @@ -1,15 +1,16 @@ /* - Property tests for the bounded thread query in GetPostThread. + Property tests for the bounded thread query. The unit tests pin the thread endpoint to a fixed fixture. These properties exercise broad random thread shapes and assert the feature's invariants: - closure / conservation: execute returns exactly the reachable posts and requests like counts for exactly those txids, once each. - - bounded work: it loads at most one post per visited txid and never falls - back to the whole-database buildLikeCountMap scan. - - bounded reads: every postChildren iteration is a prefix-bounded scan for - the parent being expanded (gte ":" / lte ":\uffff"). + - bounded work: it loads at most one post per visited txid and asks the + adapter for child txids at most once per present txid, never falling back + to a whole-store scan. + - bounded reads: PostQuery.listChildTxids prefix-scans only the requested + parent range (gte ":" / lte ":\uffff"). - ordering: replies are sorted by blockHeight ascending, then seen ascending. - acyclicity: back-edges and shared children terminate and appear at most once in the returned thread. @@ -19,15 +20,53 @@ import test from 'node:test' import { seededRandom, forAll, intGen, txidGen } from './harness.js' import GetPostThread from '../../src/use-cases/get-post-thread.js' +import PostQuery from '../../src/adapters/post-query.js' const rng = seededRandom(20260915) +// Build a GetPostThread around a mock postQuery adapter, exposing call records. +// The mock implements only the PostQuery interface the use case is allowed to +// depend on; the postChildren key format belongs to the adapter. +function makeEnvironment ({ posts, edges }) { + const childrenByParent = new Map() + for (const { parentTxid, childTxid } of edges) { + if (!childrenByParent.has(parentTxid)) childrenByParent.set(parentTxid, []) + childrenByParent.get(parentTxid).push(childTxid) + } + + const env = { + postsGetCount: 0, + childListCalls: [], + likeCalls: [] + } + + const postQuery = { + async getPostOrNull (txid) { + env.postsGetCount++ + return posts[txid] || null + }, + async listChildTxids (txid) { + env.childListCalls.push(txid) + return childrenByParent.get(txid) || [] + }, + async countLikesForTxids (txids) { + env.likeCalls.push([...txids]) + const counts = new Map() + txids.forEach((txid, index) => counts.set(txid, index % 3)) + return counts + } + } + + return { env, uut: new GetPostThread({ adapters: { postQuery } }) } +} + // In-memory postChildren store honoring the LevelDB gte/lte prefix contract, // recording each iterator call so the bounds can be asserted. -function makePostChildrenDb (edges, calls) { +function makePrefixScanChildrenDb (edges, calls) { const store = new Map() for (const edge of edges) { store.set(`${edge.parentTxid}:${edge.childTxid}`, { + txid: edge.childTxid, parentTxid: edge.parentTxid, childTxid: edge.childTxid }) @@ -48,39 +87,16 @@ function makePostChildrenDb (edges, calls) { } } -// Build a GetPostThread around a mock postQuery adapter, exposing call records. -function makeEnvironment ({ posts, edges }) { - const env = { - postsGetCount: 0, - iteratorCalls: [], - likeCalls: [], - buildLikeCountMapCalled: false - } - - const postQuery = { - postsDb: { - async get (txid) { - env.postsGetCount++ - if (posts[txid]) return posts[txid] - const err = new Error('not found') - err.notFound = true - throw err - } - }, - postChildrenDb: makePostChildrenDb(edges, env.iteratorCalls), - async countLikesForTxids (txids) { - env.likeCalls.push([...txids]) - const counts = new Map() - txids.forEach((txid, index) => counts.set(txid, index % 3)) - return counts - }, - async buildLikeCountMap () { - env.buildLikeCountMapCalled = true - return new Map() - } - } - - return { env, uut: new GetPostThread({ adapters: { postQuery } }) } +function makeChildQuery (edges, calls) { + return new PostQuery({ + postsDb: {}, + postHeightsDb: {}, + addrPostHeightsDb: {}, + postParentsDb: {}, + postChildrenDb: makePrefixScanChildrenDb(edges, calls), + likesDb: {}, + postLikesDb: {} + }) } // A random directed graph over n posts. Node 0 is the root and edges may point @@ -185,7 +201,6 @@ test('execute returns exactly the reachable thread and counts likes for it only' if (new Set(returned).size !== returned.length) return false if (!sameSet(returned, expected.present)) return false - if (env.buildLikeCountMapCalled) return false if (env.likeCalls.length !== 1) return false if (!sameSet(env.likeCalls[0], expected.present)) return false if (new Set(env.likeCalls[0]).size !== env.likeCalls[0].length) return false @@ -199,21 +214,24 @@ test('execute returns exactly the reachable thread and counts likes for it only' ) }) -test('every postChildren read is prefix-bounded to the expanded parent', async () => { +test('child txids are requested at most once per present thread node', async () => { await forAll( threadGen(), async ({ root, posts, edges }) => { const { env, uut } = makeEnvironment({ posts, edges }) await uut.execute({ txid: root }) + const expected = reachableFromRoot(root, posts, edges) - for (const options of env.iteratorCalls) { - if (typeof options?.gte !== 'string' || typeof options?.lte !== 'string') return false - if (!options.gte.endsWith(':')) return false - if (options.lte !== options.gte + '\uffff') return false + // Every child-list request targets a node that exists in the thread. + for (const txid of env.childListCalls) { + if (!expected.present.has(txid)) return false } + // Exactly one request per present txid, so no repeated whole-store work. + if (env.childListCalls.length !== expected.present.size) return false + if (new Set(env.childListCalls).size !== env.childListCalls.length) return false return true }, - { label: 'bounded postChildren reads' } + { label: 'bounded child-list requests' } ) }) @@ -228,3 +246,30 @@ test('thread replies are ordered by blockHeight then seen ascending', async () = { label: 'thread reply ordering' } ) }) + +test('listChildTxids prefix-scans a bounded parent range and returns exactly its children', async () => { + await forAll( + threadGen(), + async ({ posts, edges }) => { + const calls = [] + const query = makeChildQuery(edges, calls) + + for (const parent of Object.keys(posts)) { + const children = await query.listChildTxids(parent) + const expected = [...new Set( + edges.filter((e) => e.parentTxid === parent).map((e) => e.childTxid) + )].sort() + const actual = [...children].sort() + if (JSON.stringify(actual) !== JSON.stringify(expected)) return false + } + + for (const options of calls) { + if (typeof options?.gte !== 'string' || typeof options?.lte !== 'string') return false + if (!options.gte.endsWith(':')) return false + if (options.lte !== options.gte + '\uffff') return false + } + return true + }, + { label: 'bounded and exact child prefix scan' } + ) +}) diff --git a/psf-memo-db/test/property/like-count.property.test.js b/psf-memo-db/test/property/like-count.property.test.js index 59e3645..97dd8b9 100644 --- a/psf-memo-db/test/property/like-count.property.test.js +++ b/psf-memo-db/test/property/like-count.property.test.js @@ -5,8 +5,8 @@ list and thread use cases. These properties pin down the invariants that unit tests only probe at a few fixed fixtures: - - conservation: buildLikeCountMap counts exactly the likes whose post - exists in the posts store, ignoring orphaned likes. + - conservation: countLikesForTxids counts exactly the likes for each + requested post, ignoring likes on other posts. - round-trip: attachLikeCounts adds likeCount without mutating or dropping any input post. - pagination: assemblePostPage preserves pagination metadata and computes @@ -33,12 +33,17 @@ function mockPostsDb (posts) { } } -// A postLikes store that iterates [key, postLike] pairs keyed as postTxid:likeTxid. +// A postLikes store that iterates [key, postLike] pairs keyed as postTxid:likeTxid +// and honors the gte/lte prefix bounds the adapter sends. function mockPostLikesDb (likes) { return { - async * iterator () { + async * iterator (options = {}) { + const { gte, lte } = options for (const like of likes) { - yield [`${like.postTxid}:${like.txid}`, like] + const key = `${like.postTxid}:${like.txid}` + if (gte !== undefined && key < gte) continue + if (lte !== undefined && key > lte) continue + yield [key, like] } } } @@ -56,7 +61,7 @@ function makeQuery (posts, likes) { }) } -test('buildLikeCountMap counts likes per existing post and ignores orphans', async () => { +test('countLikesForTxids counts likes per requested post', async () => { await forAll( (i) => { const postCount = intGen(rng, 0, 8)() @@ -77,9 +82,10 @@ test('buildLikeCountMap counts likes per existing post and ignores orphans', asy return { posts, likes } }, async ({ posts, likes }) => { - const counts = await makeQuery(posts, likes).buildLikeCountMap() + const requested = Object.keys(posts) + const counts = await makeQuery(posts, likes).countLikesForTxids(requested) - const expected = new Map() + const expected = new Map(requested.map((txid) => [txid, 0])) for (const like of likes) { if (!posts[like.postTxid]) continue expected.set(like.postTxid, (expected.get(like.postTxid) || 0) + 1) @@ -91,7 +97,7 @@ test('buildLikeCountMap counts likes per existing post and ignores orphans', asy } return true }, - { label: 'buildLikeCountMap conservation' } + { label: 'countLikesForTxids conservation' } ) }) diff --git a/psf-memo-db/test/unit/adapters/post-query.unit.js b/psf-memo-db/test/unit/adapters/post-query.unit.js index acc4aa8..a70143d 100644 --- a/psf-memo-db/test/unit/adapters/post-query.unit.js +++ b/psf-memo-db/test/unit/adapters/post-query.unit.js @@ -526,6 +526,58 @@ describe('#PostQuery', () => { }) }) + describe('#listChildTxids', () => { + it('should prefix-scan postChildren for the parent and return its child txids', async () => { + async function * mockChildren () { + yield ['tx1:reply-a', { parentTxid: 'tx1', childTxid: 'reply-a' }] + yield ['tx1:reply-b', { parentTxid: 'tx1', childTxid: 'reply-b' }] + } + postChildrenDb.iterator + .withArgs(sinon.match({ gte: 'tx1:', lte: 'tx1:\uffff' })) + .returns(mockChildren()) + + const result = await uut.listChildTxids('tx1') + + assert.deepEqual(result, ['reply-a', 'reply-b']) + assert.isTrue(postChildrenDb.iterator.calledWith({ gte: 'tx1:', lte: 'tx1:\uffff' })) + }) + + it('should skip entries whose parentTxid does not match the requested parent', async () => { + async function * mockChildren () { + // A parent txid that is a string prefix of tx1 can land in the range. + yield ['tx1:reply-a', { parentTxid: 'tx1x', childTxid: 'reply-a' }] + yield ['tx1:reply-b', { parentTxid: 'tx1', childTxid: 'reply-b' }] + } + postChildrenDb.iterator + .withArgs(sinon.match({ gte: 'tx1:', lte: 'tx1:\uffff' })) + .returns(mockChildren()) + + const result = await uut.listChildTxids('tx1') + + assert.deepEqual(result, ['reply-b']) + }) + + it('should skip entries with no child txid', async () => { + async function * mockChildren () { + yield ['tx1:reply-a', { parentTxid: 'tx1' }] + yield ['tx1:reply-b', { parentTxid: 'tx1', childTxid: 'reply-b' }] + } + postChildrenDb.iterator + .withArgs(sinon.match({ gte: 'tx1:', lte: 'tx1:\uffff' })) + .returns(mockChildren()) + + const result = await uut.listChildTxids('tx1') + + assert.deepEqual(result, ['reply-b']) + }) + + it('should return an empty list when the parent has no children', async () => { + const result = await uut.listChildTxids('tx1') + + assert.deepEqual(result, []) + }) + }) + describe('#likeTxidFromPostLike', () => { it('should return the likeTxid from the value when present', () => { assert.equal(uut.likeTxidFromPostLike('tx1:like-a', { likeTxid: 'like-a' }), 'like-a') @@ -589,29 +641,14 @@ describe('#PostQuery', () => { }) }) - describe('#buildLikeCountMap', () => { - it('should count likes per post from the postLikes index', async () => { - async function * mockPostLikes () { - yield ['tx1:like-a', { postTxid: 'tx1', txid: 'like-a' }] - yield ['tx1:like-b', { postTxid: 'tx1', txid: 'like-b' }] - yield ['tx2:like-c', { postTxid: 'tx2', txid: 'like-c' }] - yield ['tx3:like-d', {}] - } - postLikesDb.iterator.returns(mockPostLikes()) - postsDb.get.callsFake(async (txid) => { - if (txid === 'tx1' || txid === 'tx2' || txid === 'tx3') { - return { addr: 'addr-a', text: 'x', seen: 1, blockHeight: 1 } - } - const err = new Error('not found') - err.notFound = true - throw err - }) + describe('#postTxidFromPostLike', () => { + it('should return the postTxid from the value when present', () => { + assert.equal(uut.postTxidFromPostLike('tx1:like-a', { postTxid: 'tx1' }), 'tx1') + }) - const result = await uut.buildLikeCountMap() - - assert.equal(result.get('tx1'), 2) - assert.equal(result.get('tx2'), 1) - assert.equal(result.get('tx3'), 1) + it('should parse the post txid from the key when the value is absent', () => { + assert.equal(uut.postTxidFromPostLike('tx1:like-a'), 'tx1') + assert.equal(uut.postTxidFromPostLike('tx1:like-a', {}), 'tx1') }) }) diff --git a/psf-memo-db/test/unit/use-cases/get-post-thread.unit.js b/psf-memo-db/test/unit/use-cases/get-post-thread.unit.js index c794c4e..2e049a7 100644 --- a/psf-memo-db/test/unit/use-cases/get-post-thread.unit.js +++ b/psf-memo-db/test/unit/use-cases/get-post-thread.unit.js @@ -7,6 +7,7 @@ describe('#GetPostThread', () => { let sandbox let postQuery let postsGetCounter + let childLists const mockPosts = { 'root-1': { addr: 'addr-a', text: 'root', seen: 100, blockHeight: 600100 }, @@ -14,32 +15,6 @@ describe('#GetPostThread', () => { 'reply-2': { addr: 'addr-b', text: 'reply b', seen: 80, blockHeight: 600080 } } - // An in-memory postChildren index that honors the gte/lte prefix bounds the - // production code sends, so a full-store scan is observably different from a - // bounded prefix scan. - function createPostChildrenDb (entries) { - return { - iterator: sandbox.stub().callsFake((options = {}) => { - const { gte, lte } = options - return (async function * () { - for (const [key, value] of entries) { - if (gte !== undefined && key < gte) continue - if (lte !== undefined && key > lte) continue - yield [key, value] - } - })() - }) - } - } - - function rootThreadChildren () { - return createPostChildrenDb([ - ['root-1:reply-1', { parentTxid: 'root-1', childTxid: 'reply-1' }], - ['root-1:reply-2', { parentTxid: 'root-1', childTxid: 'reply-2' }], - ['other-root:reply-x', { parentTxid: 'other-root', childTxid: 'reply-x' }] - ]) - } - beforeEach(() => { sandbox = sinon.createSandbox() postsGetCounter = { calls: 0 } @@ -48,23 +23,19 @@ describe('#GetPostThread', () => { 'reply-1': { addr: 'addr-a', text: 'reply', seen: 90, blockHeight: 600090 }, 'reply-2': { addr: 'addr-b', text: 'reply b', seen: 80, blockHeight: 600080 } }) + childLists = { 'root-1': ['reply-1', 'reply-2'] } postQuery = { - postsDb: { - get: sandbox.stub().callsFake(async (txid) => { - postsGetCounter.calls++ - if (mockPosts[txid]) return mockPosts[txid] - const err = new Error('not found') - err.notFound = true - throw err - }) - }, - postChildrenDb: rootThreadChildren(), + // The use case must depend only on the PostQuery interface; the + // postChildren key format and prefix bounds are the adapter's concern. + getPostOrNull: sandbox.stub().callsFake(async (txid) => { + postsGetCounter.calls++ + return mockPosts[txid] || null + }), + listChildTxids: sandbox.stub().callsFake(async (txid) => childLists[txid] || []), countLikesForTxids: sandbox.stub().resolves(new Map([ ['root-1', 4], ['reply-1', 2] - ])), - // The thread endpoint must not fall back to a whole-database like scan. - buildLikeCountMap: sandbox.stub().rejects(new Error('buildLikeCountMap must not be called')) + ])) } uut = new GetPostThread({ adapters: { postQuery } }) }) @@ -122,12 +93,6 @@ describe('#GetPostThread', () => { assert.deepEqual([...txids].sort(), ['reply-1', 'reply-2', 'root-1']) }) - it('should not build a whole-database like count map', async () => { - await uut.execute({ txid: 'root-1' }) - - assert.isTrue(postQuery.buildLikeCountMap.notCalled) - }) - it('should only load posts that belong to the thread', async () => { await uut.execute({ txid: 'root-1' }) @@ -135,32 +100,32 @@ describe('#GetPostThread', () => { assert.equal(postsGetCounter.calls, 3) }) - it('should prefix-scan postChildren for each thread node', async () => { + it('should request child txids from the adapter for each thread node', async () => { await uut.execute({ txid: 'root-1' }) - const bounds = postQuery.postChildrenDb.iterator.args.map(([options]) => options) - assert.deepInclude(bounds, { gte: 'root-1:', lte: 'root-1:\uffff' }) - assert.deepInclude(bounds, { gte: 'reply-1:', lte: 'reply-1:\uffff' }) - assert.deepInclude(bounds, { gte: 'reply-2:', lte: 'reply-2:\uffff' }) + const requested = postQuery.listChildTxids.args.map(([txid]) => txid).sort() + assert.deepEqual(requested, ['reply-1', 'reply-2', 'root-1']) }) - it('should never scan the whole postChildren store', async () => { - await uut.execute({ txid: 'root-1' }) + it('should prune a child post that no longer exists', async () => { + childLists['root-1'] = ['reply-1', 'missing-reply'] - for (const [options] of postQuery.postChildrenDb.iterator.args) { - assert.isString(options?.gte) - assert.isString(options?.lte) - } - }) - - it('should ignore postChildren entries outside the requested parent prefix', async () => { const result = await uut.execute({ txid: 'root-1' }) + assert.equal(result.post.replyCount, 1) + assert.deepEqual(result.post.replies.map((r) => r.txid), ['reply-1']) + }) + + it('should terminate on a reply cycle and keep each post once', async () => { + childLists['reply-1'] = ['root-1'] + + const result = await uut.execute({ txid: 'root-1' }) + + // root-1 -> reply-1 -> root-1 (cycle back-edge is pruned); reply-2 is a + // sibling with no children. assert.equal(result.post.replyCount, 2) - assert.deepEqual( - result.post.replies.map((r) => r.txid).sort(), - ['reply-1', 'reply-2'] - ) + const reply1 = result.post.replies.find((r) => r.txid === 'reply-1') + assert.equal(reply1.replyCount, 0) }) it('should sort replies by blockHeight ascending then seen ascending', async () => { @@ -257,7 +222,7 @@ describe('#GetPostThread', () => { it('should rethrow unexpected post lookup errors', async () => { const boom = new Error('disk failure') - postQuery.postsDb.get.rejects(boom) + postQuery.getPostOrNull.rejects(boom) try { await uut.execute({ txid: 'root-1' })