fix(message): restore indexed reserved session request lookup - #1391
Merged
Conversation
…identities The canonical session lookup for reserved identities had drifted away from the messageSessionIdentity expression index, causing queries to miss the optimised index path. The condition now always anchors on the expression-index column while still allowing owner-scoped lookups to match legacy null-identity rows via the session_id fallback. Unscoped reserved lookups remain narrow and do not pick up unrelated null-identity rows, preserving reserved identity isolation.
…lback Extend the reserved identity SQL contract test to verify that the generated where clause uses coalesce when resolving owner-scoped session requests for legacy clients.
Add encoded-identity regression cases to the v1 session requests route test, verifying that URL-encoded session identities (pfx: and sid: prefixed) resolve correctly through the endpoint. Rewrite the repository-level session request query tests to compile the generated SQL via PgDialect instead of string matching, and parameterize them across both pfx: and sid: identity prefixes. The assertions now verify the exact coalesce expression, identity guards, and parameter bindings for both owner-scoped legacy fallback and unscoped canonical lookups.
Contributor
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
📝 WalkthroughWalkthrough本次变更更新会话 identity 查询条件。指定用户时允许空 Changes会话 identity 查询流程
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Code Review Summary
No significant issues identified in this PR.
PR Size: S
- Lines changed: 97
- Files changed: 3
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Clean
- Type safety - Clean
- Documentation accuracy - Clean
- Test coverage - Adequate
- Code clarity - Good
Automated review by Codex AI
Contributor
There was a problem hiding this comment.
- Reviewed PR
#1391end-to-end against the diff, repo standards, and the six requested review perspectives. - Applied the
size/Slabel and posted the GitHub review summary on the PR. - No issues cleared the reporting threshold after context/false-positive validation, so I did not add inline review comments.
- I could not run the targeted Vitest commands in this runner because
bun/bunxare not installed, so the review outcome is based on static diff and code-path analysis.
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
COALESCE(session_identity, session_id)predicate for reservedpfx:/sid:session request timelines so PostgreSQL can useidx_message_request_session_identity_created_atsession_identity = identityguard for unscoped lookups, preserving reserved namespace isolation from same-named physical Session IDssession_identity IS NULLProduction symptom
GET /api/v1/sessions/{encoded-pfx-identity}/requests?page=1&pageSize=20&order=descreturned400 session.action_failed.The route and query parameters are valid. The action wraps internal failures as this 400 response, while the underlying failure is the session request query timing out on a large
message_requesttable.Root cause
The reserved-identity request-list path generated only:
The existing index starts with:
The bare predicate could not become the expression index's
Index Cond, so an admin/unscoped request could scan the large table untilstatement_timeout.The repaired predicate is:
For owner-scoped legacy compatibility, the second term is:
Local PostgreSQL
EXPLAINchanged from a filter-only plan to an index scan with:Validation
bun run build: passed before the final rebasebun run lint: passed after rebasebun run typecheck: passed after rebasegit diff --check origin/dev...HEAD: passedThe intervening
devcommit only changes dashboard usage-log time-filter UI and tests. The focused session tests, lint, and typecheck were rerun after rebase.Deployment note
This PR does not change the action's broad error classification. Production recovery requires this PR to be merged and deployed; the reported remote URL has not been re-tested against a deployment from this branch.
Greptile Summary
This PR restores the expression-index predicate for reserved session request lookups while preserving owner-scoped legacy-row support and preventing reserved identities from aliasing physical sessions.
pfx:andsid:identities.Confidence Score: 5/5
The PR appears safe to merge, with no actionable correctness, security, or compatibility regressions identified.
The new predicate is logically equivalent to the prior lookup for all nullable column combinations, matches the existing PostgreSQL expression index, and preserves the deliberate distinction between owner-scoped legacy fallback and unscoped canonical lookup.
Important Files Changed
pfx:andsid:lookups.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Reserved session identity] --> B[Match COALESCE of session identity and session ID] B --> C{Owner scoped?} C -->|Yes| D[Require matching identity or legacy NULL identity] C -->|No| E[Require explicit matching session identity] D --> F[Apply owner and request filters] E --> F F --> G[Count and return session requests]Reviews (1): Last reviewed commit: "style(message): normalize comment punctu..." | Re-trigger Greptile
Context used: