Repository navigation
feat: Add per-session draft AI context state - #707
Conversation
|
Warning Review limit reached
More reviews will be available in 2 minutes and 19 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds per-session ChangesDraft Context Session Lifecycle
Sequence DiagramsequenceDiagram
participant User
participant AssistantUI
participant RoomStore
participant AiSlice
participant Session
User->>AssistantUI: select context IDs
AssistantUI->>RoomStore: ai.setSessionDraftContextItemIds(sessionId, ids)
RoomStore->>AiSlice: setSessionDraftContextItemIds(sessionId, ids)
User->>AssistantUI: start analysis (target sessionId)
AssistantUI->>AiSlice: startAnalysis(sessionId)
AiSlice->>AiSlice: getRunContext(sessionId)
AiSlice->>RoomStore: getRunContext(store, sessionId)
RoomStore->>Session: read draftContextItemIds / runContext
AiSlice->>Session: store runContext (captured from draft)
AiSlice->>Session: set draftContextItemIds = undefined
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1951ded4dc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| draftSession.runContext = getRunContext?.(); | ||
| draftSession.draftContextItemIds = undefined; |
There was a problem hiding this comment.
When startAnalysis is invoked for a session that is not the current session (for example from another open/background tab or a programmatic caller), getRunContext still receives no sessionId, so hosts using the new per-session draft state have to infer from getCurrentSession() and can capture the wrong session's draft context. This line then clears the target session's draftContextItemIds, losing its selection even though runContext may have been built from another session or as undefined; pass the target sessionId into context capture or build the context from draftSession before clearing it.
Useful? React with 👍 / 👎.
![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 current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/ai-core/__tests__/AiSlice.modelSelection.test.ts`:
- Around line 299-300: The variable declaration for store uses let but is never
reassigned; change the declaration in the test to use const instead of let where
store is initialized with createTestStore (the identifier store and the call
createTestStore are the targets) so ESLint prefer-const is satisfied; update the
variable declaration to const store = createTestStore({...}) and remove the
separate let declaration.
In `@packages/ai-core/src/AiSlice.ts`:
- Around line 1195-1198: The run-context capture is calling getRunContext?.()
without the session identity which can capture another session's context; update
the call to pass the current session id (sessionId) into getRunContext so
draftSession.runContext = getRunContext?.(sessionId) (or the equivalent
parameter name used in startAnalysis) to ensure the captured context is scoped
to the target draftSession; adjust any call sites and types for getRunContext to
accept the sessionId parameter if needed.
🪄 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
Run ID: 721b4cb9-8fb9-490b-871e-ddc1ac47400a
📒 Files selected for processing (3)
packages/ai-config/src/schema/AnalysisSessionSchema.tspackages/ai-core/__tests__/AiSlice.modelSelection.test.tspackages/ai-core/src/AiSlice.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4265a1d74f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (session?.draftContextItemIds) { | ||
| return session.draftContextItemIds.length > 0 | ||
| ? session.draftContextItemIds | ||
| : (options.implicitItemIds ?? []); |
There was a problem hiding this comment.
When the user removes every selected item, setSessionDraftContextItemIds stores [] to represent an explicit manual empty selection, but this branch replaces that empty draft with implicitItemIds. In an empty/new session with a current artifact, clearing the selector and then starting analysis will still send the implicit artifact as context, regressing the previous manual no-context path; a defined draft should be honored even when it is empty.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/ai-core/src/contextSelection.ts`:
- Around line 1-49: Add JSDoc comments to the four public functions
isAnalysisSessionEmpty, getRunContextItemIds, getVisibleSessionContextItemIds,
and getEffectiveSessionContextItemIds describing their purpose, inputs, and
return values; explicitly document that session.draftContextItemIds ===
undefined means "no explicit draft configured; fall back to runContext/implicit
IDs", whereas an empty array means "explicitly set to empty (hide all items)";
for getEffectiveSessionContextItemIds describe the fallback order
(draftContextItemIds when present and non-empty → explicit empty array returns
provided implicitItemIds if specified → otherwise runContext ids → finally
options.implicitItemIds) and include a short example usage for each function
showing typical input and expected output; keep descriptions generic and
composable (no app-specific examples) and reference the AiRunContext and
AnalysisSessionSchema types in the JSDoc `@param/`@returns.
🪄 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
Run ID: fe4f725e-6255-4b55-9feb-6425a48e795d
📒 Files selected for processing (13)
apps/sqlrooms-cli-ui/src/components/AssistantContextSelector.tsxapps/sqlrooms-cli-ui/src/components/assistantUtils.tsapps/sqlrooms-cli-ui/src/components/useAssistantContextItems.tsapps/sqlrooms-cli-ui/src/components/useAutoContextReplacement.tsapps/sqlrooms-cli-ui/src/context/createArtifactContextAiTools.tsapps/sqlrooms-cli-ui/src/context/getRunContext.tsapps/sqlrooms-cli-ui/src/store-types.tsapps/sqlrooms-cli-ui/src/store.tspackages/ai-core/__tests__/AiSlice.modelSelection.test.tspackages/ai-core/src/AiSlice.tspackages/ai-core/src/contextSelection.tspackages/ai-core/src/index.tspackages/ai/src/index.ts
💤 Files with no reviewable changes (3)
- apps/sqlrooms-cli-ui/src/context/createArtifactContextAiTools.ts
- apps/sqlrooms-cli-ui/src/components/useAutoContextReplacement.ts
- apps/sqlrooms-cli-ui/src/store-types.ts
| import {getAiRunContextItems} from '@sqlrooms/ai-config'; | ||
| import type {AiRunContext, AnalysisSessionSchema} from '@sqlrooms/ai-config'; | ||
|
|
||
| export function isAnalysisSessionEmpty( | ||
| session: AnalysisSessionSchema | undefined, | ||
| ) { | ||
| if (!session) return true; | ||
| const hasMessages = session.uiMessages.length > 0; | ||
| const hasCompletedResults = session.analysisResults.some( | ||
| (result) => result.id !== '__pending__', | ||
| ); | ||
| return !hasMessages && !hasCompletedResults; | ||
| } | ||
|
|
||
| export function getRunContextItemIds( | ||
| runContext: AiRunContext | undefined, | ||
| ): string[] { | ||
| return getAiRunContextItems(runContext).map((item) => item.id); | ||
| } | ||
|
|
||
| export function getVisibleSessionContextItemIds( | ||
| session: AnalysisSessionSchema | undefined, | ||
| ): string[] { | ||
| if (session?.draftContextItemIds) { | ||
| return session.draftContextItemIds; | ||
| } | ||
|
|
||
| if (!isAnalysisSessionEmpty(session)) { | ||
| return getRunContextItemIds(session?.runContext); | ||
| } | ||
|
|
||
| return []; | ||
| } | ||
|
|
||
| export function getEffectiveSessionContextItemIds( | ||
| session: AnalysisSessionSchema | undefined, | ||
| options: {implicitItemIds?: string[]} = {}, | ||
| ): string[] { | ||
| if (session?.draftContextItemIds) { | ||
| return session.draftContextItemIds.length > 0 | ||
| ? session.draftContextItemIds | ||
| : (options.implicitItemIds ?? []); | ||
| } | ||
|
|
||
| const runContextIds = getRunContextItemIds(session?.runContext); | ||
| return runContextIds.length > 0 | ||
| ? runContextIds | ||
| : (options.implicitItemIds ?? []); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add JSDoc documentation for public exports.
These helper functions are exported from a packages/ module and will be part of the public API. Per coding guidelines, public exports should be documented. Add JSDoc comments explaining:
- The purpose of each function
- The meaning of empty array vs undefined for
draftContextItemIds - The fallback behavior in
getEffectiveSessionContextItemIds - Example usage
As per coding guidelines, "Public exports in packages/ should be kept generic, composable, documented, and free of app/example-specific assumptions."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/ai-core/src/contextSelection.ts` around lines 1 - 49, Add JSDoc
comments to the four public functions isAnalysisSessionEmpty,
getRunContextItemIds, getVisibleSessionContextItemIds, and
getEffectiveSessionContextItemIds describing their purpose, inputs, and return
values; explicitly document that session.draftContextItemIds === undefined means
"no explicit draft configured; fall back to runContext/implicit IDs", whereas an
empty array means "explicitly set to empty (hide all items)"; for
getEffectiveSessionContextItemIds describe the fallback order
(draftContextItemIds when present and non-empty → explicit empty array returns
provided implicitItemIds if specified → otherwise runContext ids → finally
options.implicitItemIds) and include a short example usage for each function
showing typical input and expected output; keep descriptions generic and
composable (no app-specific examples) and reference the AiRunContext and
AnalysisSessionSchema types in the JSDoc `@param/`@returns.
Source: Coding guidelines
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fa36af25e8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return session.draftContextItemIds; | ||
| } | ||
|
|
||
| if (!isAnalysisSessionEmpty(session)) { |
There was a problem hiding this comment.
For a brand-new session with no draft selection, this returns an empty visible selection, but apps/sqlrooms-cli-ui/src/context/getRunContext.ts still passes the current artifact as implicitItemIds when starting analysis. That means the selector can look empty while the current artifact is silently sent as context, and because it is not shown as selected the user cannot remove it before the first run; the previous auto-context path made that implicit artifact visible.
Useful? React with 👍 / 👎.
Summary:
This adds session-owned draft context state for AI conversations. Instead of hosts keeping context selector state globally and syncing it on session switches, each AI session can now store its own draft context item IDs.
Changes:
draftContextItemIdstoAnalysisSessionSchema.getSessionDraftContextItemIdssetSessionDraftContextItemIdsdraftContextItemIdswhenstartAnalysiscaptures context intorunContext.Why:
This lets host apps model composer context the same way they already model prompt/session state: as per-session draft state. It avoids cross-session context leakage and removes the need for app-level synchronization effects.
Validation:
AiSlice.modelSelection.test.tspassed.tsc --noEmitpassed forai-configandai-core.Summary by CodeRabbit
New Features
Tests