Files
psf-memo/docs/reviews/thread-query-bounds-summary.md
Chris Troutner 1ff2c020ac 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.
2026-09-15 13:10:46 -07:00

104 lines
5.5 KiB
Markdown

# 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.