feat(social-listening): stable mentions paging across rebuilds (GH-1918) - #2384
feat(social-listening): stable mentions paging across rebuilds (GH-1918)#2384audigregorie wants to merge 5 commits into
Conversation
Signed-off-by: Audi Young <audi.mycloud@gmail.com>
PR SummaryMedium Risk Overview Backend: Feed SQL uses Frontend: Two-phase window fills chain phase 2 off phase 1’s token; Breaking: Reviewed by Cursor Bugbot for commit bb19774. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Essentials Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
🚀 Deployment StatusYour branch has been deployed to: https://ui-pr-2384.dev.v2.cluster.linuxfound.info Deployment Details:
The deployment will be automatically removed when this PR is closed. |
There was a problem hiding this comment.
Pull request overview
Replaces offset-based social-listening pagination with stable Snowflake keyset cursors across hourly rebuilds.
Changes:
- Adds strict cursor token parsing and shared pagination contracts.
- Implements compound-key Snowflake pagination and token chaining.
- Updates Angular feed state, counts, watermark handling, and tests.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
packages/shared/src/interfaces/social-listening.interface.ts |
Defines cursor request and response contracts. |
packages/shared/src/constants/social-listening.constants.ts |
Removes the server offset ceiling. |
apps/lfx-one/src/server/services/social-listening.service.ts |
Implements keyset SQL and token issuance. |
apps/lfx-one/src/server/services/social-listening.service.spec.ts |
Tests cursor predicates and page boundaries. |
apps/lfx-one/src/server/helpers/strict-query-param.helper.ts |
Adds strict scalar query parsing. |
apps/lfx-one/src/server/helpers/social-listening-params.helper.ts |
Parses and validates pagination tokens. |
apps/lfx-one/src/server/helpers/social-listening-params.helper.spec.ts |
Tests token validation and page-size handling. |
apps/lfx-one/src/server/helpers/committee-activity-query.helper.ts |
Reuses the strict query reader. |
apps/lfx-one/src/server/controllers/social-listening.controller.ts |
Passes cursor pagination into the service. |
apps/lfx-one/src/app/modules/dashboards/social-listening/social-listening.component.ts |
Chains feed windows and detects cursor exhaustion. |
apps/lfx-one/src/app/modules/dashboards/social-listening/social-listening.component.spec.ts |
Tests cursor-chain UI behavior. |
apps/lfx-one/src/app/modules/dashboards/social-listening/components/mentions-list/mentions-list.component.ts |
Updates total-count presentation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Signed-off-by: Audi Young <audi.mycloud@gmail.com>
Signed-off-by: Audi Young <audi.mycloud@gmail.com>
Signed-off-by: Audi Young <audi.mycloud@gmail.com>
Signed-off-by: Audi Young <audi.mycloud@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
apps/lfx-one/src/server/helpers/social-listening-params.helper.ts:177
- The token is base64url-encoded JSON with shape validation only, so tampering is not detected: a caller can encode any other valid
{ ts, key }and it will be accepted. This contradicts both this comment and the PR's claim that tampered tokens return 400. Either authenticate the payload (for example, with a server-side MAC) or narrow the documented contract to malformed/wrong-shaped tokens.
|
Hey @audigregorie, thanks for the thorough writeup on this one — the cursor-based redesign is a real fix for a genuine bug class (offset drift under hourly rebuilds), and the NULL-timestamp handling in the keyset predicate is the kind of detail that's easy to get subtly wrong and you clearly worked through it carefully. This is a re-review round: the five Copilot findings from earlier rounds (stale-high total at feed end, Three independent reviewers covered Security/Privacy, Correctness/Performance/Tests, and Style/API/Docs across the full Issue count
Bot reconciliation
Findings
Decision: ✅ Approved with minor comments |
dealako
left a comment
There was a problem hiding this comment.
Approving. Full write-up: #2384 (comment)
No blocking or privacy findings. Two low-severity items (a page_token length-cap gap, a logging nit) noted in the summary comment — neither blocks merge.
Summary
The social-listening mentions feed paged by numeric
offsetover a Snowflake table that is rebuilt by an hourly full refresh — absolute row positions shift mid-session, so deep Load More paging could skip or repeat rows, and a hard cap stopped browsing past ~100,000 rows. This PR switches the feed to keyset (cursor) pagination: each page returns an opaquepage_tokenderived from the last shown row's content, and the next page continues strictly after that row, so paging stays stable across rebuilds and the cursor chain serves the feed to its end.Resolves #1918
Behavior changes
Technical changes
packages/shared/src/interfaces/social-listening.interface.ts, packages/shared/src/constants/social-listening.constants.ts— Shared contractsSocialListeningFeedCursor— the(MENTION_TS, _KEY)compound sort key of the last shown row, content-relative so it survives full-refresh rebuildsSocialListeningPaginationParamsis now{ pageSize, cursor? };SocialListeningFeedRequestsendspage_size/page_tokenon the wire;SocialListeningFeedResponsereturns an optionalpage_token(absent = feed exhausted)MENTION_MAX_FEED_OFFSET— cursor paging needs no depth capapps/lfx-one/src/server/helpers/social-listening-params.helper.ts— Feed param parsingparseSocialListeningPaginationreadspage_size(must be an integer, clamped 1–100;?page_size=parses to 0 and clamps to 1 deterministically) and decodespage_tokeninto a keyset cursor; malformed or tampered tokens return 400 instead of silently restarting the feedtsis kept verbatim (no timezone reinterpretation) so the bound value round-trips byte-for-byteisValidFeedTimestamp(shape regex +Date.parseround-trip), now shared byreadBeforeTsand the cursortsapps/lfx-one/src/server/helpers/strict-query-param.helper.ts, apps/lfx-one/src/server/helpers/committee-activity-query.helper.ts— Strict query readergetStrictStringQueryParam: a repeated query param (?page_token=a&page_token=b, which Express'sqsparser turns into an array) now 400s instead of being treated as absent and silently restarting paginationvalidation.helper.tsapps/lfx-one/src/server/services/social-listening.service.ts, apps/lfx-one/src/server/controllers/social-listening.controller.ts— Keyset feed queryLIMIT/OFFSETwith a keyset predicate selecting rows strictly after the cursor underMENTION_TS DESC, _KEY DESC, fetchingpage_size + 1rows — the extra row is the hasMore signal and the issued token rides the last kept rowpageSize/cursorthrough and logshas_cursorinstead of raw offsetsapps/lfx-one/src/app/modules/dashboards/social-listening/social-listening.component.ts, apps/lfx-one/src/app/modules/dashboards/social-listening/components/mentions-list/mentions-list.component.ts— Feed UIpage_token(feedChainToken) instead of computed offsets — a window waits while its predecessor fills and treats a tokenless completed window as the feed's endservableTotalis the uncapped count total;hasMoreend-detection reads the missing token; "Data as of" is pinned to window 0's first stamp so a mid-scan rebuild can't contradict rows already on screenapps/lfx-one/src/server/helpers/social-listening-params.helper.spec.ts, apps/lfx-one/src/server/services/social-listening.service.spec.ts, apps/lfx-one/src/app/modules/dashboards/social-listening/social-listening.component.spec.ts— Testspage_sizeclamping, and strict-reader 400sBreaking changes
The
GET /api/social-listening/mentions-feedquery contract changed:limit/offsetwere replaced bypage_size/page_token. The endpoint is consumed only by this app's own frontend, which is updated in the same PR, so there is no cross-deploy ordering concern.