Skip to content

Commit 7d20e78

Browse files
mariuspruvotclaude
andcommitted
feat(comprehension): add SSE question streaming and chat panel (Story 3.3)
Implements the end-to-end Socratic question generation flow: PydanticAI streaming provider, SSE endpoint with DB-phase / HTTP-phase split, metadata-only question persistence (hash, never verbatim text), and a live-updating chat panel with reduced-motion support. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 039b72b commit 7d20e78

36 files changed

Lines changed: 4354 additions & 157 deletions

_bmad-output/implementation-artifacts/sprint-status.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@
4545
# 'awaiting-manual-qa', unless invoked with `--skip-manual-qa-gate <reason>`.
4646

4747
generated: 2026-04-09
48-
last_updated: 2026-04-10 # 3-3 → in-progress (dev-story: question generation + SSE streaming)
48+
last_updated: 2026-04-11 # 3-3 → review (dev-story complete: question generation + SSE streaming)
4949
project: helprs
5050
project_key: NOKEY
5151
tracking_system: file-system
@@ -114,7 +114,7 @@ development_status:
114114
# replaced "Story 3.3 backlog" prose with grep-able TODO(story-3.3).
115115
# make lint && vitest = clean + 24 web tests green (24 = 25 −3 +2).
116116
3-2-split-view-session-ui-and-diff-viewer: done
117-
3-3-question-generation-and-sse-streaming: in-progress
117+
3-3-question-generation-and-sse-streaming: review
118118
3-4-answer-submission-and-feedback-with-code-links: backlog
119119
3-5-role-adaptation-beyond-diff-and-large-pr-handling: backlog
120120
epic-3-retrospective: optional
Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
"""add questions table and total_questions
2+
3+
Revision ID: a1b2c3d4e5f6
4+
Revises: 9036c7377667
5+
Create Date: 2026-04-10 14:00:00.000000
6+
7+
Story 3.3: introduces the ``questions`` table (metadata-only — no
8+
``text`` column; see FR35/NFR14) and adds ``sessions.total_questions``
9+
so the session detail endpoint + SSE stream know up-front how many
10+
questions to ask. Existing rows get ``total_questions = 0`` via the
11+
``server_default``.
12+
"""
13+
from collections.abc import Sequence
14+
15+
import sqlalchemy as sa
16+
17+
from alembic import op
18+
19+
# revision identifiers, used by Alembic.
20+
revision: str = "a1b2c3d4e5f6"
21+
down_revision: str | None = "9036c7377667"
22+
branch_labels: str | Sequence[str] | None = None
23+
depends_on: str | Sequence[str] | None = None
24+
25+
26+
def upgrade() -> None:
27+
# ---- sessions.total_questions (additive, nullable=False + default) ----
28+
op.add_column(
29+
"sessions",
30+
sa.Column(
31+
"total_questions",
32+
sa.Integer(),
33+
nullable=False,
34+
server_default=sa.text("0"),
35+
),
36+
)
37+
38+
# ---- questions table -------------------------------------------------
39+
op.create_table(
40+
"questions",
41+
sa.Column("session_id", sa.Uuid(), nullable=False),
42+
sa.Column("number", sa.Integer(), nullable=False),
43+
sa.Column("topic", sa.String(length=32), nullable=False),
44+
sa.Column("text_hash", sa.String(length=64), nullable=False),
45+
sa.Column("id", sa.Uuid(), nullable=False),
46+
sa.Column(
47+
"created_at",
48+
sa.DateTime(timezone=True),
49+
server_default=sa.text("now()"),
50+
nullable=False,
51+
),
52+
sa.Column(
53+
"updated_at",
54+
sa.DateTime(timezone=True),
55+
server_default=sa.text("now()"),
56+
nullable=False,
57+
),
58+
sa.ForeignKeyConstraint(
59+
["session_id"],
60+
["sessions.id"],
61+
ondelete="CASCADE",
62+
),
63+
sa.PrimaryKeyConstraint("id"),
64+
sa.UniqueConstraint(
65+
"session_id",
66+
"number",
67+
name="uq_questions_session_number",
68+
),
69+
)
70+
op.create_index(
71+
"ix_questions_session_id",
72+
"questions",
73+
["session_id"],
74+
unique=False,
75+
)
76+
77+
78+
def downgrade() -> None:
79+
op.drop_index("ix_questions_session_id", table_name="questions")
80+
op.drop_table("questions")
81+
op.drop_column("sessions", "total_questions")

apps/api/src/helprs/core/database.py

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
"""Async SQLAlchemy engine and session factory."""
22

33
import uuid
4-
from collections.abc import AsyncGenerator
4+
from collections.abc import AsyncGenerator, AsyncIterator
5+
from contextlib import asynccontextmanager
56
from datetime import datetime
67

78
from sqlalchemy import DateTime, func
@@ -10,6 +11,64 @@
1011

1112
from helprs.core.config import get_settings
1213

14+
# Module-level slot holding the active session factory. Populated by
15+
# ``helprs.main`` during lifespan startup and read by ``get_db_context``
16+
# (a plain async context manager — not a FastAPI dependency — used by
17+
# streaming endpoints that need to open a fresh short-lived session
18+
# inside a stream generator AFTER the request-scoped ``get_db`` session
19+
# has already been released).
20+
#
21+
# The FastAPI dep graph keeps using ``app.state.session_factory`` via
22+
# ``core.dependencies.get_db`` — ``get_db_context`` does NOT replace it;
23+
# it is additive, for code paths that live outside the request lifecycle.
24+
_session_factory_holder: async_sessionmaker[AsyncSession] | None = None
25+
26+
27+
def set_session_factory(factory: async_sessionmaker[AsyncSession]) -> None:
28+
"""Register the session factory for ``get_db_context`` to use.
29+
30+
Called once by the lifespan startup in ``helprs.main`` after the
31+
engine is created. Tests that need ``get_db_context`` outside the
32+
app lifespan can call this directly in a fixture.
33+
"""
34+
global _session_factory_holder
35+
_session_factory_holder = factory
36+
37+
38+
def clear_session_factory() -> None:
39+
"""Reset the module-level slot (test teardown)."""
40+
global _session_factory_holder
41+
_session_factory_holder = None
42+
43+
44+
@asynccontextmanager
45+
async def get_db_context() -> AsyncIterator[AsyncSession]:
46+
"""Open a short-lived AsyncSession outside the FastAPI dep graph.
47+
48+
Used by SSE streaming code that needs to commit small writes
49+
(per-question persistence) AFTER the request-scoped
50+
``get_db`` dependency has released its session. Holding the
51+
request-scoped session open across 10s+ LLM calls would exhaust
52+
the connection pool under load — this helper is the escape hatch.
53+
54+
Commits on clean exit and rolls back on exception. Never use
55+
``get_db_context`` for request-scoped DB work — the FastAPI
56+
``get_db`` dependency is the right tool there.
57+
"""
58+
if _session_factory_holder is None:
59+
raise RuntimeError(
60+
"get_db_context called before set_session_factory — "
61+
"this is a lifespan bug in helprs.main or a missing "
62+
"test fixture setup."
63+
)
64+
async with _session_factory_holder() as session:
65+
try:
66+
yield session
67+
await session.commit()
68+
except Exception:
69+
await session.rollback()
70+
raise
71+
1372

1473
class Base(AsyncAttrs, DeclarativeBase):
1574
"""Base model with common columns for all entities."""

apps/api/src/helprs/main.py

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,12 @@
88
from fastapi import APIRouter, FastAPI
99

1010
from helprs.core.config import get_settings
11-
from helprs.core.database import create_engine, create_session_factory
11+
from helprs.core.database import (
12+
clear_session_factory,
13+
create_engine,
14+
create_session_factory,
15+
set_session_factory,
16+
)
1217
from helprs.core.exceptions import DomainError, domain_exception_handler
1318
from helprs.core.middleware import configure_logging, configure_sentry, setup_middleware
1419

@@ -108,8 +113,13 @@ async def lifespan(app: FastAPI):
108113
engine = create_engine()
109114
reaper_task: asyncio.Task | None = None
110115
try:
116+
session_factory = create_session_factory(engine)
111117
app.state.engine = engine
112-
app.state.session_factory = create_session_factory(engine)
118+
app.state.session_factory = session_factory
119+
# Register the factory for ``get_db_context`` (used by the
120+
# SSE stream generator for per-question writes outside the
121+
# request-scoped dep graph — Story 3.3).
122+
set_session_factory(session_factory)
113123
app.state.replay_semaphore = asyncio.Semaphore(_REPLAY_CONCURRENCY)
114124
app.state.replay_tasks = set()
115125

@@ -141,6 +151,7 @@ async def lifespan(app: FastAPI):
141151
if tracked:
142152
await asyncio.gather(*tracked, return_exceptions=True)
143153

154+
clear_session_factory()
144155
await engine.dispose()
145156

146157
app = FastAPI(
@@ -160,6 +171,7 @@ async def lifespan(app: FastAPI):
160171
api_router = APIRouter(prefix="/api/v1")
161172

162173
from helprs.modules.comprehension.presentation.routers import router as comprehension_router
174+
from helprs.modules.comprehension.presentation.sse import sse_router as comprehension_sse_router
163175
from helprs.modules.identity.router import router as identity_router
164176
from helprs.modules.installation.router import router as installation_router
165177
from helprs.modules.webhook.router import router as webhook_router
@@ -168,6 +180,11 @@ async def lifespan(app: FastAPI):
168180
api_router.include_router(installation_router)
169181
api_router.include_router(webhook_router)
170182
api_router.include_router(comprehension_router)
183+
# Story 3.3: SSE streaming for Socratic question generation. Mounted
184+
# AFTER the detail router so FastAPI's route matcher considers the
185+
# more specific ``/{session_id}/stream`` path alongside the existing
186+
# ``/{session_id}`` detail route.
187+
api_router.include_router(comprehension_sse_router)
171188

172189
app.include_router(api_router)
173190

apps/api/src/helprs/modules/comprehension/application/commands.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,3 +24,9 @@ class StartSessionCommand:
2424
# field in a frozen dataclass still lets callers mutate labels after
2525
# construction. Immutable by construction keeps the command safe.
2626
pr_labels: tuple[str, ...]
27+
# Story 3.3: optional PR diff line count, used by
28+
# ``StartSessionHandler`` to size the session via
29+
# ``estimate_question_count``. ``None`` for legacy callers (existing
30+
# webhook-handler tests without an updated fixture) — the handler
31+
# falls back to ``5`` questions in that case.
32+
pr_diff_line_count: int | None = None

apps/api/src/helprs/modules/comprehension/application/handlers.py

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
from helprs.modules.comprehension.application.commands import StartSessionCommand
3030
from helprs.modules.comprehension.application.queries import GetSessionQuery, GetSessionResult
3131
from helprs.modules.comprehension.domain.entities import PRContext, Session
32+
from helprs.modules.comprehension.domain.services import estimate_question_count
3233
from helprs.modules.comprehension.infrastructure.repositories import SqlAlchemySessionRepository
3334
from helprs.modules.installation.service import (
3435
get_default_suppression_labels,
@@ -126,8 +127,16 @@ async def handle(self, cmd: StartSessionCommand) -> StartSessionResult:
126127
pr_diff_url=cmd.pr_diff_url,
127128
)
128129

130+
# TODO(story-3.5): replace with role-adaptive + large-PR sizing
131+
# (see FR39–FR41). Story 3.3 uses the minimal line-count
132+
# heuristic so the end-to-end flow ships first.
133+
total_questions = estimate_question_count(cmd.pr_diff_line_count) if cmd.pr_diff_line_count is not None else 5
134+
129135
if len(existing) == 0:
130-
author, reviewer = await repo.add_pair(pr_ctx=pr_ctx)
136+
author, reviewer = await repo.add_pair(
137+
pr_ctx=pr_ctx,
138+
total_questions=total_questions,
139+
)
131140
pair: tuple[Session, Session] = (author, reviewer)
132141
created = True
133142
comment_needed = True
@@ -156,7 +165,10 @@ async def handle(self, cmd: StartSessionCommand) -> StartSessionResult:
156165
orphan_session_id=str(orphan.id),
157166
)
158167
await repo.delete_one(session_id=orphan.id)
159-
author, reviewer = await repo.add_pair(pr_ctx=pr_ctx)
168+
author, reviewer = await repo.add_pair(
169+
pr_ctx=pr_ctx,
170+
total_questions=total_questions,
171+
)
160172
pair = (author, reviewer)
161173
created = True
162174
comment_needed = True
@@ -259,8 +271,11 @@ async def handle(self, query: GetSessionQuery) -> GetSessionResult:
259271
# exceptions".
260272
installation_token = await mint_installation_token(domain_session.github_installation_id, self._settings)
261273

274+
# Story 3.3: count persisted questions for the detail response.
275+
question_count = await repo.count_questions(session_id=query.session_id)
276+
262277
return GetSessionResult(
263278
session=domain_session,
264279
installation_token=installation_token,
265-
question_count=0, # Story 3.3 will count QuestionModel rows here
280+
question_count=question_count,
266281
)

apps/api/src/helprs/modules/comprehension/domain/entities.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,11 @@ class Session:
7575
pr_diff_url: str
7676
role: SessionRole
7777
status: SessionStatus
78+
# Story 3.3: number of questions this session plans to ask, set at
79+
# creation time by ``estimate_question_count``. Zero on legacy rows
80+
# created before the Story 3.3 migration — the SSE endpoint falls
81+
# back to ``5`` in that case.
82+
total_questions: int
7883
created_at: datetime
7984
updated_at: datetime
8085

0 commit comments

Comments
 (0)