Skip to content

Fix question prompt not rendering for active session - #325

Merged
chriswritescode-dev merged 2 commits into
chriswritescode-dev:mainfrom
thrawn-sh:fix/session-scoped-questions
Jul 28, 2026
Merged

Fix question prompt not rendering for active session#325
chriswritescode-dev merged 2 commits into
chriswritescode-dev:mainfrom
thrawn-sh:fix/session-scoped-questions

Conversation

@thrawn-sh

@thrawn-sh thrawn-sh commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

currentQuestion was the first pending question globally across all sessions. When another session had the first question in the flat list, the QuestionPrompt would not render for the session being viewed even though it had its own pending question.

Add getForSession() to the questions context and use it in SessionDetail to look up questions scoped to the current session.

Summary

Type of Change

  • Bug fix
  • New feature
  • Refactor
  • Documentation

Checklist

  • Code follows project style (no comments, named imports)
  • TypeScript types are properly defined
  • Tests added/updated (80% coverage target)
  • pnpm lint passes locally
  • pnpm typecheck passes locally

Summary by CodeRabbit

  • Bug Fixes
    • Improved question handling on session detail pages by retrieving the relevant pending/known question per specific session.
    • Updated question prompt rendering to depend on the resolved session question (and minimized state), rather than requiring session IDs to match.
  • Tests
    • Added and expanded coverage to verify pending questions resolve independently per session, including updated mocks for per-session question lookups.

@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 28da9128-3468-4f06-bfb3-c06c79b6b9dc

📥 Commits

Reviewing files that changed from the base of the PR and between 217d1ee and 1d2d29e.

📒 Files selected for processing (6)
  • frontend/src/contexts/EventContext.test.tsx
  • frontend/src/pages/__tests__/SessionDetail.assistant-loading.test.tsx
  • frontend/src/pages/__tests__/SessionDetail.polling.test.tsx
  • frontend/src/pages/__tests__/SessionDetail.question-prompt.test.tsx
  • frontend/src/pages/__tests__/SessionDetail.scroll-floating.test.tsx
  • frontend/src/pages/__tests__/SessionDetail.todo-header.test.tsx

📝 Walkthrough

Walkthrough

Changes

Question lookup flow

Layer / File(s) Summary
Expose per-session question lookup
frontend/src/contexts/EventContext.tsx, frontend/src/contexts/EventContext.test.tsx
Adds and tests questions.getForSession, which returns the first question associated with a session and is included in the memoized context value.
Use session-specific question state
frontend/src/pages/SessionDetail.tsx, frontend/src/pages/__tests__/SessionDetail.*.test.tsx
Derives the current question with the session-specific getter, updates QuestionPrompt rendering, and adds or updates tests for session scoping and hook mocks.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: chriswritescode-dev

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main bug fix: rendering the question prompt for the active session.
Description check ✅ Passed The description covers the bug, the fix, the change type, and checklist items, with only minor template formatting issues.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@frontend/src/pages/SessionDetail.tsx`:
- Around line 176-177: Guard the sessionId argument in the currentQuestion
initialization within SessionDetail so getQuestionForSession is called only when
sessionId is defined, returning null otherwise. Keep the existing
getQuestionForSession behavior unchanged for valid string session IDs.
🪄 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: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 3ef53df2-9fce-437f-aec4-687f9d06a220

📥 Commits

Reviewing files that changed from the base of the PR and between f8e361c and 6895354.

📒 Files selected for processing (2)
  • frontend/src/contexts/EventContext.tsx
  • frontend/src/pages/SessionDetail.tsx

Comment thread frontend/src/pages/SessionDetail.tsx Outdated
currentQuestion was the first pending question globally across all
sessions. When another session had the first question in the flat list,
the QuestionPrompt would not render for the session being viewed even
though it had its own pending question.

Add getForSession() to the questions context and use it in
SessionDetail to look up questions scoped to the current session.
@thrawn-sh
thrawn-sh force-pushed the fix/session-scoped-questions branch from 6895354 to 217d1ee Compare July 28, 2026 19:25
The SessionDetail useQuestions mocks did not expose getForSession, so
every page test threw on the new session-scoped lookup. Add it to each
mock and lock in the behaviour with regression coverage: the context
helper resolves questions per session while another session owns the
global current question, and SessionDetail renders the prompt for the
session being viewed.
@chriswritescode-dev
chriswritescode-dev merged commit eac0909 into chriswritescode-dev:main Jul 28, 2026
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants