feat(profile): Sprint 16 §4.4 — SQL-aggregated stats in one round-trip#16
Open
dobrodob wants to merge 2 commits intofeat/sprint-16-c-deleted-by-auditfrom
Open
feat(profile): Sprint 16 §4.4 — SQL-aggregated stats in one round-trip#16dobrodob wants to merge 2 commits intofeat/sprint-16-c-deleted-by-auditfrom
dobrodob wants to merge 2 commits intofeat/sprint-16-c-deleted-by-auditfrom
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR introduces a new batch stats computation method Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
This was referenced Apr 19, 2026
dobrodob
force-pushed
the
feat/sprint-16-c-deleted-by-audit
branch
from
8dfc133 to
2f2a39d
Compare
dobrodob
force-pushed
the
feat/sprint-16-d-sql-stats
branch
from
c28564f to
94cef32
Compare
Member
Author
|
@coderabbitai review — this PR was marked as skipped earlier. Please review explicitly. |
|
🧠 Learnings used✅ Actions performedReview triggered.
|
|
🧠 Learnings used🧠 Learnings used✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/core/src/modules/profile/profile.service.ts`:
- Around line 292-329: computePublicStatsMany() currently selects from authors a
without filtering soft-deleted rows, so it can return stats for authors with
deleted_at set; update the SQL in profile.service.ts (the query used by
computePublicStatsMany / computePublicStats) to add "a.deleted_at IS NULL" to
the outer WHERE clause (the WHERE a.id = ANY(${authorIds}::int[])) so the
authors table scan excludes soft-deleted authors and aligns with the app's
soft-delete guidelines.
In `@apps/core/test/profile.integration.spec.ts`:
- Around line 451-489: The batch test for profiles.computePublicStatsMany only
asserts shoutsPublished — add seeding and assertions for the other aggregates
(followers_count, following_count, reactions_given) so the SQL join/alias paths
are exercised: create at least one follow edge (e.g., have user B follow A via
the follow/follows API or direct prisma follow create) and one reaction (e.g.,
create a reaction by some user on a shout via reactions.create or
prisma.reaction.create), ensure shouts are published as currently done, then
call profiles.computePublicStatsMany([a.id, b.id, c.id]) and assert for each
returned map row that shoutsPublished, followers_count, following_count, and
reactions_given have the expected values; also include the empty/unknown-ID
checks already present.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 63551fce-874f-4ff3-a788-b1d9a2a48e86
📒 Files selected for processing (4)
apps/core/src/modules/profile/profile.service.tsapps/core/test/profile.integration.spec.tsdocs/discours-patterns/44-sql-aggregated-stats.mddocs/discours-patterns/README.md
Comment on lines
+292
to
+329
| SELECT | ||
| a.id AS author_id, | ||
| COALESCE(sp.n, 0)::int AS shouts_published, | ||
| COALESCE(fl.n, 0)::int AS followers_count, | ||
| COALESCE(fo.n, 0)::int AS following_count, | ||
| COALESCE(rg.n, 0)::int AS reactions_given | ||
| FROM authors a | ||
| LEFT JOIN ( | ||
| SELECT sa.author_id AS aid, COUNT(*)::int AS n | ||
| FROM shouts s | ||
| JOIN shout_authors sa ON sa.shout_id = s.id | ||
| WHERE s.community_id = ${communityId} | ||
| AND s.deleted_at IS NULL | ||
| AND s.status = 'PUBLISHED'::"ShoutStatus" | ||
| AND sa.author_id = ANY(${authorIds}::int[]) | ||
| GROUP BY sa.author_id | ||
| ) sp ON sp.aid = a.id | ||
| LEFT JOIN ( | ||
| SELECT author_id AS aid, COUNT(*)::int AS n | ||
| FROM author_followers | ||
| WHERE author_id = ANY(${authorIds}::int[]) | ||
| GROUP BY author_id | ||
| ) fl ON fl.aid = a.id | ||
| LEFT JOIN ( | ||
| SELECT follower_id AS aid, COUNT(*)::int AS n | ||
| FROM author_followers | ||
| WHERE follower_id = ANY(${authorIds}::int[]) | ||
| GROUP BY follower_id | ||
| ) fo ON fo.aid = a.id | ||
| LEFT JOIN ( | ||
| SELECT created_by AS aid, COUNT(*)::int AS n | ||
| FROM reactions | ||
| WHERE deleted_at IS NULL | ||
| AND created_by = ANY(${authorIds}::int[]) | ||
| GROUP BY created_by | ||
| ) rg ON rg.aid = a.id | ||
| WHERE a.id = ANY(${authorIds}::int[]) | ||
| `; |
There was a problem hiding this comment.
Filter soft-deleted authors out of the batch query.
The outer authors scan only filters by id, so computePublicStatsMany() can still return stats for authors whose deleted_at is already set. Because computePublicStats() now delegates here, the single-author path inherits the same leak. Add a.deleted_at IS NULL to the outer WHERE.
Suggested fix
LEFT JOIN (
SELECT created_by AS aid, COUNT(*)::int AS n
FROM reactions
WHERE deleted_at IS NULL
AND created_by = ANY(${authorIds}::int[])
GROUP BY created_by
) rg ON rg.aid = a.id
- WHERE a.id = ANY(${authorIds}::int[])
+ WHERE a.id = ANY(${authorIds}::int[])
+ AND a.deleted_at IS NULL
`;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| SELECT | |
| a.id AS author_id, | |
| COALESCE(sp.n, 0)::int AS shouts_published, | |
| COALESCE(fl.n, 0)::int AS followers_count, | |
| COALESCE(fo.n, 0)::int AS following_count, | |
| COALESCE(rg.n, 0)::int AS reactions_given | |
| FROM authors a | |
| LEFT JOIN ( | |
| SELECT sa.author_id AS aid, COUNT(*)::int AS n | |
| FROM shouts s | |
| JOIN shout_authors sa ON sa.shout_id = s.id | |
| WHERE s.community_id = ${communityId} | |
| AND s.deleted_at IS NULL | |
| AND s.status = 'PUBLISHED'::"ShoutStatus" | |
| AND sa.author_id = ANY(${authorIds}::int[]) | |
| GROUP BY sa.author_id | |
| ) sp ON sp.aid = a.id | |
| LEFT JOIN ( | |
| SELECT author_id AS aid, COUNT(*)::int AS n | |
| FROM author_followers | |
| WHERE author_id = ANY(${authorIds}::int[]) | |
| GROUP BY author_id | |
| ) fl ON fl.aid = a.id | |
| LEFT JOIN ( | |
| SELECT follower_id AS aid, COUNT(*)::int AS n | |
| FROM author_followers | |
| WHERE follower_id = ANY(${authorIds}::int[]) | |
| GROUP BY follower_id | |
| ) fo ON fo.aid = a.id | |
| LEFT JOIN ( | |
| SELECT created_by AS aid, COUNT(*)::int AS n | |
| FROM reactions | |
| WHERE deleted_at IS NULL | |
| AND created_by = ANY(${authorIds}::int[]) | |
| GROUP BY created_by | |
| ) rg ON rg.aid = a.id | |
| WHERE a.id = ANY(${authorIds}::int[]) | |
| `; | |
| SELECT | |
| a.id AS author_id, | |
| COALESCE(sp.n, 0)::int AS shouts_published, | |
| COALESCE(fl.n, 0)::int AS followers_count, | |
| COALESCE(fo.n, 0)::int AS following_count, | |
| COALESCE(rg.n, 0)::int AS reactions_given | |
| FROM authors a | |
| LEFT JOIN ( | |
| SELECT sa.author_id AS aid, COUNT(*)::int AS n | |
| FROM shouts s | |
| JOIN shout_authors sa ON sa.shout_id = s.id | |
| WHERE s.community_id = ${communityId} | |
| AND s.deleted_at IS NULL | |
| AND s.status = 'PUBLISHED'::"ShoutStatus" | |
| AND sa.author_id = ANY(${authorIds}::int[]) | |
| GROUP BY sa.author_id | |
| ) sp ON sp.aid = a.id | |
| LEFT JOIN ( | |
| SELECT author_id AS aid, COUNT(*)::int AS n | |
| FROM author_followers | |
| WHERE author_id = ANY(${authorIds}::int[]) | |
| GROUP BY author_id | |
| ) fl ON fl.aid = a.id | |
| LEFT JOIN ( | |
| SELECT follower_id AS aid, COUNT(*)::int AS n | |
| FROM author_followers | |
| WHERE follower_id = ANY(${authorIds}::int[]) | |
| GROUP BY follower_id | |
| ) fo ON fo.aid = a.id | |
| LEFT JOIN ( | |
| SELECT created_by AS aid, COUNT(*)::int AS n | |
| FROM reactions | |
| WHERE deleted_at IS NULL | |
| AND created_by = ANY(${authorIds}::int[]) | |
| GROUP BY created_by | |
| ) rg ON rg.aid = a.id | |
| WHERE a.id = ANY(${authorIds}::int[]) | |
| AND a.deleted_at IS NULL | |
| `; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/core/src/modules/profile/profile.service.ts` around lines 292 - 329,
computePublicStatsMany() currently selects from authors a without filtering
soft-deleted rows, so it can return stats for authors with deleted_at set;
update the SQL in profile.service.ts (the query used by computePublicStatsMany /
computePublicStats) to add "a.deleted_at IS NULL" to the outer WHERE clause (the
WHERE a.id = ANY(${authorIds}::int[])) so the authors table scan excludes
soft-deleted authors and aligns with the app's soft-delete guidelines.
Comment on lines
+451
to
+489
| describe('computePublicStatsMany() batch', () => { | ||
| it( | ||
| 'returns stats for every author in one SQL round-trip', | ||
| inTenant(async () => { | ||
| const a = await register('batch-a@test.io'); | ||
| const b = await register('batch-b@test.io'); | ||
| const c = await register('batch-c@test.io'); | ||
|
|
||
| // A has 2 published shouts, B has 1, C has 0. We bypass | ||
| // ShoutService.transition and flip status directly — the state | ||
| // machine requires editor role, which is orthogonal to the stats | ||
| // calculation we're actually probing here. | ||
| const s1 = await shouts.create(a.id, { title: 'A1', body: 'x' }); | ||
| const s2 = await shouts.create(a.id, { title: 'A2', body: 'x' }); | ||
| const s3 = await shouts.create(b.id, { title: 'B1', body: 'x' }); | ||
| await prisma.shout.updateMany({ | ||
| where: { id: { in: [s1.id, s2.id, s3.id] } }, | ||
| data: { status: ShoutStatus.PUBLISHED, publishedAt: new Date() }, | ||
| }); | ||
|
|
||
| const batch = await profiles.computePublicStatsMany([a.id, b.id, c.id]); | ||
| expect(batch.size).toBe(3); | ||
| expect(batch.get(a.id)?.shoutsPublished).toBe(2); | ||
| expect(batch.get(b.id)?.shoutsPublished).toBe(1); | ||
| expect(batch.get(c.id)?.shoutsPublished).toBe(0); | ||
| // Missing-ID case — computePublicStatsMany should NOT invent rows. | ||
| const unknown = await profiles.computePublicStatsMany([999_999_999]); | ||
| expect(unknown.size).toBe(0); | ||
| }), | ||
| ); | ||
|
|
||
| it( | ||
| 'empty array returns empty map (no SQL round-trip)', | ||
| inTenant(async () => { | ||
| const batch = await profiles.computePublicStatsMany([]); | ||
| expect(batch.size).toBe(0); | ||
| }), | ||
| ); | ||
| }); |
There was a problem hiding this comment.
Exercise the other aggregate branches in this batch test.
This only validates shoutsPublished, so a bad join/alias in followers_count, following_count, or reactions_given would still pass. Since the implementation is raw SQL, I'd seed at least one follow edge and one reaction and assert all four fields per author here.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/core/test/profile.integration.spec.ts` around lines 451 - 489, The batch
test for profiles.computePublicStatsMany only asserts shoutsPublished — add
seeding and assertions for the other aggregates (followers_count,
following_count, reactions_given) so the SQL join/alias paths are exercised:
create at least one follow edge (e.g., have user B follow A via the
follow/follows API or direct prisma follow create) and one reaction (e.g.,
create a reaction by some user on a shout via reactions.create or
prisma.reaction.create), ensure shouts are published as currently done, then
call profiles.computePublicStatsMany([a.id, b.id, c.id]) and assert for each
returned map row that shoutsPublished, followers_count, following_count, and
reactions_given have the expected values; also include the empty/unknown-ID
checks already present.
dobrodob
added a commit
that referenced
this pull request
Apr 19, 2026
1. **`computePublicStatsMany` leaked stats for soft-deleted authors.**
The outer `FROM authors a WHERE a.id = ANY(...)` had no `deleted_at`
guard — a caller passing a mix of live and soft-deleted IDs would
receive rows for both. Added `AND a.deleted_at IS NULL` to the outer
WHERE. The single-author path (`computePublicStats`) inherits the
fix via delegation.
2. **Batch test only asserted `shoutsPublished`** — a bad alias/JOIN
on followers / following / reactionsGiven would pass silently.
Extended the spec:
- Seed follow edges (A→B, B→A, B→C) and reactions (A→s3, B→s1) so
each of the 4 aggregate branches has a non-zero case.
- Full `toEqual` assertion per author covers every field.
- New test: `excludes soft-deleted authors from the batch result`
— regression guard for fix #1.
Verification: 21/21 profile spec passing, tsc clean, biome clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
dobrodob
force-pushed
the
feat/sprint-16-c-deleted-by-audit
branch
from
2f2a39d to
eb2ae06
Compare
dobrodob
added a commit
that referenced
this pull request
Apr 19, 2026
1. **`computePublicStatsMany` leaked stats for soft-deleted authors.**
The outer `FROM authors a WHERE a.id = ANY(...)` had no `deleted_at`
guard — a caller passing a mix of live and soft-deleted IDs would
receive rows for both. Added `AND a.deleted_at IS NULL` to the outer
WHERE. The single-author path (`computePublicStats`) inherits the
fix via delegation.
2. **Batch test only asserted `shoutsPublished`** — a bad alias/JOIN
on followers / following / reactionsGiven would pass silently.
Extended the spec:
- Seed follow edges (A→B, B→A, B→C) and reactions (A→s3, B→s1) so
each of the 4 aggregate branches has a non-zero case.
- Full `toEqual` assertion per author covers every field.
- New test: `excludes soft-deleted authors from the batch result`
— regression guard for fix #1.
Verification: 21/21 profile spec passing, tsc clean, biome clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
dobrodob
force-pushed
the
feat/sprint-16-d-sql-stats
branch
from
1ff5e6b to
f5ac197
Compare
dobrodob
force-pushed
the
feat/sprint-16-c-deleted-by-audit
branch
from
eb2ae06 to
1eac3c4
Compare
dobrodob
added a commit
that referenced
this pull request
Apr 19, 2026
1. **`computePublicStatsMany` leaked stats for soft-deleted authors.**
The outer `FROM authors a WHERE a.id = ANY(...)` had no `deleted_at`
guard — a caller passing a mix of live and soft-deleted IDs would
receive rows for both. Added `AND a.deleted_at IS NULL` to the outer
WHERE. The single-author path (`computePublicStats`) inherits the
fix via delegation.
2. **Batch test only asserted `shoutsPublished`** — a bad alias/JOIN
on followers / following / reactionsGiven would pass silently.
Extended the spec:
- Seed follow edges (A→B, B→A, B→C) and reactions (A→s3, B→s1) so
each of the 4 aggregate branches has a non-zero case.
- Full `toEqual` assertion per author covers every field.
- New test: `excludes soft-deleted authors from the batch result`
— regression guard for fix #1.
Verification: 21/21 profile spec passing, tsc clean, biome clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
dobrodob
force-pushed
the
feat/sprint-16-d-sql-stats
branch
from
f5ac197 to
f444f19
Compare
Ports discoursio-backend-core/resolvers/stat.py's "stat object" pattern to
NestJS + Prisma. Replaces the 4-count Promise.all inside ProfileService
with a single raw-SQL query that LEFT JOINs four correlated subqueries per
author_id. Gains scale on list endpoints: 4N queries → 1 query regardless
of author count.
API reshape:
- computePublicStatsMany(authorIds: number[]) — new, primary: returns
Map<authorId, PublicProfileStats>. Empty array short-circuits (no SQL
round-trip).
- computePublicStats(authorId) — delegates to the batch version. Single-
author callers stay on the simple API; the batch runs for them too —
avoids two divergent code paths.
Test coverage:
- New describe('computePublicStatsMany() batch') block in profile spec:
seeds 3 authors with asymmetric activity, asserts stats per author and
that unknown IDs produce empty Map entries (no invented rows).
- All 18 pre-existing profile tests still pass — the getStatsForSlug
/ getPrivateStats paths delegate transparently.
Pattern doc: 44-sql-aggregated-stats.md. Covers BigInt-cast gotcha,
Postgres enum cast for 'PUBLISHED'::\"ShoutStatus\", empty-array short-
circuit, and index dependencies.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1. **`computePublicStatsMany` leaked stats for soft-deleted authors.**
The outer `FROM authors a WHERE a.id = ANY(...)` had no `deleted_at`
guard — a caller passing a mix of live and soft-deleted IDs would
receive rows for both. Added `AND a.deleted_at IS NULL` to the outer
WHERE. The single-author path (`computePublicStats`) inherits the
fix via delegation.
2. **Batch test only asserted `shoutsPublished`** — a bad alias/JOIN
on followers / following / reactionsGiven would pass silently.
Extended the spec:
- Seed follow edges (A→B, B→A, B→C) and reactions (A→s3, B→s1) so
each of the 4 aggregate branches has a non-zero case.
- Full `toEqual` assertion per author covers every field.
- New test: `excludes soft-deleted authors from the batch result`
— regression guard for fix #1.
Verification: 21/21 profile spec passing, tsc clean, biome clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
dobrodob
force-pushed
the
feat/sprint-16-c-deleted-by-audit
branch
from
1eac3c4 to
f9c5db1
Compare
dobrodob
force-pushed
the
feat/sprint-16-d-sql-stats
branch
from
f444f19 to
5a2488f
Compare
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
1 participant
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
Sprint 16 §4.4 — ports discoursio-backend-core/resolvers/stat.py's "stat object" pattern to NestJS + Prisma. Replaces the 4-count `Promise.all` inside `ProfileService` with a single raw-SQL query that LEFT JOINs four correlated subqueries per `author_id`.
PR 4 of 7 for Sprint 16 — stacked on top of #15.
The win
For a single-author profile page: 4 queries → 1. Small gain.
For a list endpoint with N authors (top-authors widget, topic page author cards):
4N queries → 1 query. Regardless of N.
API reshape
```typescript
async computePublicStatsMany(authorIds: number[]): Promise<Map<number, PublicProfileStats>> {
if (authorIds.length === 0) return new Map();
const rows = await this.prisma.$queryRaw<Array<...>>`
SELECT a.id AS author_id,
COALESCE(sp.n, 0)::int AS shouts_published,
COALESCE(fl.n, 0)::int AS followers_count,
COALESCE(fo.n, 0)::int AS following_count,
COALESCE(rg.n, 0)::int AS reactions_given
FROM authors a
LEFT JOIN (...) sp ON sp.aid = a.id -- published shouts per author
LEFT JOIN (...) fl ON fl.aid = a.id -- followers
LEFT JOIN (...) fo ON fo.aid = a.id -- following
LEFT JOIN (...) rg ON rg.aid = a.id -- reactions given
WHERE a.id = ANY(${authorIds}::int[])
`;
// ... fold rows into a Map
}
```
Test plan
Gotchas (in pattern doc)
Docs
🤖 Generated with Claude Code
Summary by CodeRabbit
Performance Improvements
Documentation