perf: count universes and works without materializing every record (#2729)#2732
Merged
Conversation
…2729) The Wordsmith skill scored Create/Writers Room engagement with `listUniverses().length + listWorks().length`. Both are whole-collection reads for a tally: listUniverses() selects and sanitizes every universe JSONB record AND loads the entire universe-run history before sorting, and listWorks() runs two queries to rebuild every work's manifest with its drafts[]. That cost is paid on every explicit /character read and on every saveCharacter() response, and it grows with the user's library. Adds countUniverses()/countWorks() across both backends and points the skill at them. The value is unchanged for every existing case. countUniverses reads the `universes.deleted` mirror column rather than the JSONB flag listUniverses() filters on. Verified the two cannot diverge: writeRaw and the one-time file import are the only writers, and each binds `record.deleted === true` into the column in the same statement that writes `data` — the same predicate sanitizeSoftDeleteFields computes for the flag the service filters on. So `WHERE deleted = FALSE` is exact, and it hits the idx_universes_live partial index that exists for this scan. Ephemeral universes are deliberately still counted (listUniverses filters only on `deleted`), and a tombstone is not — the bug a naive listIds().length would introduce, since listIds() returns live + ephemeral + tombstones and leaves filtering to the service. The file backend must read each record to see its `deleted` flag, so its count has no cheap form; it still skips the run-history load and the sanitize+sort. Writers Room's file count shares one live-set reader with listWorks so the listing and the tally can't drift apart.
…#2729) Reviewer flagged that the column is nullable, so the two forms differ on a NULL. Keeping deleted = FALSE: it is the form the idx_universes_live partial index can serve and it matches every sibling live-set query. Neither writer can produce a NULL, so the distinction is unreachable — noting it so the choice reads as considered rather than overlooked.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The Wordsmith skill scored Create/Writers Room engagement with
listUniverses().length + listWorks().length— two whole-collection reads for a tally.listUniverses()selects and sanitizes every universe JSONB record and loads the entire universe-run history before sorting;listWorks()runs two queries to rebuild every work's manifest with itsdrafts[]. That cost is paid on every explicit/characterread and everysaveCharacter()response, and it grows with the user's library.Adds
countUniverses()/countWorks()across both backends (Postgres + the file escape hatch), threaded through the existing store facades, and points Wordsmith'scompute()at them. The skill's value is unchanged for every existing case.Finding: is
universes.deletedauthoritative and in sync with the JSONB flag?Yes — they cannot diverge, so
WHERE deleted = FALSEis exact. This was the issue's crux, so stating the verification explicitly:listUniverses()filters!u.deletedon the sanitized record, where the flag comes fromsanitizeSoftDeleteFields(server/lib/syncWire.js) — computed asraw?.deleted === true.universestable has exactly two writers, and each binds the column in the same statement that writesdata, from the samerecordobject, with the identical predicaterecord.deleted === true:writeRaw(server/services/universeBuilder/db.js)server/scripts/migrateUniversesToDB.js)UPDATEthat touchesdeletedwithout rewritingdata(verified by grepping all SQL against the table).So the column is an exact mirror, and it additionally hits the
idx_universes_livepartial index that exists for precisely this "live set" scan. No divergence to fix.Two related correctness points the count had to get right:
(await listIds()).lengthwould introduce, sincelistIds()is documented as returning live + ephemeral + tombstones and leaving filtering to the service. Pinned by tests on both backends.listUniverses()filters only ondeleted, so filteringephemeralhere would undercount. Pinned by a test.Known boundary (documented in code, deliberately not replicated in SQL): a row whose
datacan't survivesanitizeTemplate(blankname, non-stringid) is dropped by listUniverses's.filter(Boolean)but still counted. It's unreachable through the service — create/update/sync all sanitize before writing — and only a corrupt legacy record imported verbatim could reach it. Replicating the sanitizer's drop rules in SQL would couple the count tosanitizeTemplate's internals and silently drift when they change; a ±1 on an already-corrupt record isn't worth that. Both backends share these semantics, so they agree with each other.writers_room_works.deletedneeded no such analysis: it's a real column (not a JSONB mirror) and it's the same predicatelistWorks()already filters on.Step 5 (
saveCharacter(data, { withSkills })) — evaluated, deliberately not shippedThe issue asked to consider it. It has no beneficiary, so the option would be dead weight:
saveCharacter()(addXP,takeDamage,takeRest,addEvent,syncJiraXP,syncTaskXP,updateCharacterFields,/reset) originates fromserver/routes/character.js— there are zero background/service callers (verified by grep).client/src/pages/CharacterSheet.jsx, which doessetChar(result.character). PassingwithSkills: falseanywhere would blank the skills card, so every call site would passtrue.COUNT(*)/stat reads, not a full library scan.enrichCharacteralready accepts{ withSkills }(from #2725), so the seam remains if a non-UI caller ever appears.Test plan
server/services/universeBuilder/db.test.js— new:countUniversescounts the live set, excludes the tombstone whilelistIds()still returns 3, counts ephemeral, agrees with the JSONB-flag filterlistRaw()applies,includeDeletedvariant, and follows an un-delete. Ran againstportos_testvia the DB config (verified executed, not skipped).server/services/writersRoom/db.test.js— new:countWorksagrees withlistWorks().lengthand excludes a tombstone thatlistWorkIds({ includeDeleted: true })still returns. Same, againstportos_test.server/services/universeBuilder.test.js— new (file backend, service level):countUniversesagrees withlistUniverses().lengthbefore and after a soft-delete, andincludeDeletedmirrorslistUniverses({ includeDeleted: true }).server/services/writersRoom/local.test.js— new (file backend, service level):countWorksagrees withlistWorks().lengthacross a soft-delete.server/services/characterSkills.test.js— mocks updated to the count helpers; all 27 pass, Wordsmith's values unchanged.services/installState.test.js(install-root resolution) are pre-existing onorigin/mainand untouched by this diff — confirmed by stashing this branch's changes and re-running them red on a clean tree.No DB guard was weakened; the DB-backed suites ran only via the
portos_testconfig.Closes #2729