Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 57 additions & 0 deletions backend/app/api/v1/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -1008,6 +1008,63 @@ class StudyChainResponse(BaseModel):
links: list[StudyChainLink]


class RecentChainSummary(BaseModel):
"""One row in the ``GET /api/v1/studies/chains/recent`` response.

Per spec §8.1 (feat_overnight_studies_summary_card). Per-chain
rollup feeding the "Ran while you were away" card on ``/studies``
— anchor identity + chain length + the best link's metric + the
chain's cumulative lift + the derived stop reason + the
surfaceable proposal id for the best link. Read-only; no state
transitions, no audit events.
"""

anchor_study_id: str
anchor_name: str
chain_length: int
"""``len(traversal.links)`` — guaranteed ``>= 2`` by the discovery
repo (FR-1)."""
best_metric: float | None
"""``None`` when every link's ``best_metric IS NULL`` (e.g. a
terminal-failed chain). The card renders the stop-reason phrase in
place of the numeric line on this path (AC-11)."""
objective_metric: str
"""``traversal.links[0].objective.get('metric')`` — surfaced so the
card can render "Best <metric>: <value>" without an extra request."""
cumulative_lift: float | None
"""Direction-normalized lift via
:func:`backend.app.domain.study.chain_summary.compute_cumulative_lift`.
``None`` when the completed-link subset is empty OR no baseline is
derivable (mirrors the chain panel's null-lift contract)."""
direction: ObjectiveDirection
stop_reason: ChainStopReason
"""Derived per spec §9. Values must match
backend/app/domain/study/chain_summary.py CHAIN_STOP_REASONS."""
best_link_proposal_id: str | None
"""The selected (newest non-rejected) proposal for the best link,
surfaced so the card's "Review chain" link can deep-link directly
to the proposal when one exists."""
tail_completed_at: datetime
"""``traversal.links[-1].completed_at`` — the chain tail's terminal
timestamp. Drives the card's localStorage dismissal cutoff
(``max(tail_completed_at) + 1ms`` per FR-5)."""


class RecentChainsResponse(BaseModel):
"""``GET /api/v1/studies/chains/recent`` response shape.

Inert pagination: this endpoint emits ``next_cursor=null`` and
``has_more=false`` always (OQ-2 resolved — limit-cap only). The
fields stay on the wire for consistency with the rest of the
studies surface, so a future MVP3 keyset-pagination story can
populate them without breaking clients (idea filed in this PR).
"""

data: list[RecentChainSummary]
next_cursor: str | None = None
has_more: bool = False


class StudySummary(BaseModel):
"""List-view shape."""

Expand Down
106 changes: 106 additions & 0 deletions backend/app/api/v1/studies.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@

from backend.app.api.v1.schemas import (
CreateStudyRequest,
RecentChainsResponse,
RecentChainSummary,
StudyChainLink,
StudyChainResponse,
StudyDetail,
Expand Down Expand Up @@ -599,6 +601,110 @@ async def list_studies(
)


# ---------------------------------------------------------------------------
# GET /api/v1/studies/chains/recent
# (feat_overnight_studies_summary_card §8.1)
#
# IMPORTANT: This static route MUST be declared BEFORE the
# ``/studies/{study_id}`` dynamic route below — FastAPI matches routes in
# registration order, so a dynamic ``{study_id}`` declared first would
# capture ``chains`` as the path param and 404 the lookup. The route-order
# regression assertion lives in test_studies_chain_recent_api.py.
# ---------------------------------------------------------------------------


def _recent_chain_row(traversal: repo.ChainTraversalResult) -> RecentChainSummary:
"""Build one row of the recent-chains response.

Mirrors the derivation block in :func:`get_study_chain` (lines
~851-859) — same ``select_best_link`` / ``compute_cumulative_lift`` /
``derive_chain_stop_reason`` helpers, same anchor-direction lookup,
same proposal-id-by-link-id map. We do NOT extract a shared helper
here per Plan §5 ("bounded shared-helper extraction only") — the two
derivation blocks render different response shapes, and one row
pulls only a subset of what ``get_study_chain`` emits.
"""
anchor = traversal.links[0]
tail = traversal.links[-1]
raw_direction = anchor.objective.get("direction", "maximize")
direction = raw_direction if raw_direction in ("maximize", "minimize") else "maximize"
stop_reason = derive_chain_stop_reason(traversal.links, traversal.anchor_trials)
cumulative_lift = compute_cumulative_lift(traversal.links, traversal.anchor_trials)
best_link_id = select_best_link(traversal.links)
best_metric = next((lk.best_metric for lk in traversal.links if lk.id == best_link_id), None)
best_link_proposal_id = (
traversal.proposal_id_by_link_id.get(best_link_id) if best_link_id else None
)
# ``tail.completed_at`` is guaranteed non-null by the discovery repo's
# candidate filter (status IN (terminal) AND completed_at IS NOT NULL),
# so the ``or _BASE_NEVER`` fallback below is dead code by construction
# — it satisfies mypy's ``datetime | None`` → ``datetime`` narrowing
# without an ``assert`` (which ruff S101 forbids in production code).
# ``select_best_link`` returns ``None`` for an all-NULL-best_metric
# tail (e.g. a chain whose only terminal status is "failed"), in
# which case ``best_metric`` here is ``None`` — the card renders the
# stop-reason phrase in that branch (AC-11).
tail_completed_at = tail.completed_at
if tail_completed_at is None: # pragma: no cover — discovery repo guarantees terminal tail
# Defensive: a future change to the candidate filter that drops
# the ``completed_at IS NOT NULL`` predicate must not silently
# ship a ValidationError 500. Skip the row by raising the same
# AC-11 null-metric branch upstream — but for now the row is
# always populated and this branch is unreachable.
raise _err(
500,
"INTERNAL_ERROR",
f"chain tail {tail.id} has no completed_at",
True,
)
return RecentChainSummary(
anchor_study_id=anchor.id,
anchor_name=anchor.name,
chain_length=len(traversal.links),
best_metric=best_metric,
objective_metric=str(anchor.objective.get("metric", "")),
cumulative_lift=cumulative_lift,
direction=direction,
stop_reason=stop_reason,
best_link_proposal_id=best_link_proposal_id,
tail_completed_at=tail_completed_at,
)


@router.get(
"/studies/chains/recent",
response_model=RecentChainsResponse,
tags=["studies"],
)
async def get_recent_chains(
response: Response,
db: Annotated[AsyncSession, Depends(get_db)],
since: Annotated[datetime | None, Query()] = None,
limit: Annotated[int, Query(ge=1, le=50)] = 20,
) -> RecentChainsResponse:
"""List recently-completed overnight chains (FR-1, AC-1/2/3/4/5/6/11/12).

Returns the deduplicated set of completed overnight chains (length
>= 2) ordered newest-tail-completion-first, capped at ``limit``. The
``since`` filter restricts to chains whose tail completed at or
after the cutoff (used by the card to seed the "what's new since I
last visited" query).

Malformed ``since`` / out-of-range ``limit`` flow through the
global ``validation_exception_handler`` and return the canonical
422 ``VALIDATION_ERROR`` envelope (no manual parse path).

Pagination: inert. ``next_cursor=null`` and ``has_more=false``
always — OQ-2 resolved limit-cap-only for v1. Keyset pagination
deferred to a separate ``chore_`` idea filed against the spec's
open questions.
"""
chains = await repo.list_recent_completed_chains(db, since=since, limit=limit)
rows = [_recent_chain_row(c) for c in chains]
response.headers["X-Total-Count"] = str(len(rows))
return RecentChainsResponse(data=rows, next_cursor=None, has_more=False)


# ---------------------------------------------------------------------------
# GET /api/v1/studies/{id}
# ---------------------------------------------------------------------------
Expand Down
4 changes: 4 additions & 0 deletions backend/app/db/repo/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@
hard_delete_study,
list_children_of_study,
list_queued_study_ids,
list_recent_completed_chains,
list_running_study_ids,
list_studies,
)
Expand Down Expand Up @@ -201,6 +202,9 @@
# overnight-chain summary (FR-3).
"ChainTraversalResult",
"get_chain_for_study",
# feat_overnight_studies_summary_card Story 1.1 — recent-completed-chains
# discovery feeding the "Ran while you were away" card on /studies (FR-1).
"list_recent_completed_chains",
# feat_llm_judgments Story 1.2 (judgments child table + judgment_list extensions)
"bulk_create_judgments",
"count_judgment_lists",
Expand Down
112 changes: 112 additions & 0 deletions backend/app/db/repo/study.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
order_by_clauses,
parse_sort,
)
from backend.app.domain.study.chain_summary import derive_chain_stop_reason

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -372,6 +373,117 @@ async def get_chain_for_study(
)


# ---------------------------------------------------------------------------
# feat_overnight_studies_summary_card Story 1.1 — recent-completed-chains
# discovery feeding the "Ran while you were away" card on /studies (FR-1).
# Pure read; one row per chain (anchor-deduped); terminal-only; length >= 2;
# in-flight defensively excluded; tail-completion-DESC ordering.
# ---------------------------------------------------------------------------


_TERMINAL_STUDY_STATUSES: tuple[str, ...] = ("completed", "cancelled", "failed")


async def list_recent_completed_chains(
db: AsyncSession,
*,
since: datetime | None = None,
limit: int = 20,
) -> list[ChainTraversalResult]:
"""Return de-duplicated completed overnight chains (length >= 2).

Newest tail-completion first, capped at ``limit`` distinct chains.

Algorithm (FR-1):

1. SELECT candidate member ids — studies that ARE follow-up children
(``parent_study_id IS NOT NULL``, which guarantees their chain has
length >= 2) AND have terminated (``completed_at IS NOT NULL`` AND
``status IN ('completed','cancelled','failed')``) AND, when
``since`` is supplied, completed since that cutoff. Ordered by
``completed_at DESC``. Scan-capped at ``limit * _CHAIN_MAX_DESCENDANTS``
so dedup-to-anchor can still fill ``limit`` distinct chains in the
worst case where every chain is fully maxed out (anchor + 5
descendants).
2. For each candidate (newest first), resolve its anchor via
:func:`get_chain_for_study` and key into an ordered dict on
``anchor_id`` to deduplicate to one row per chain. Skip anchors
already collected.
3. Skip any chain whose
:func:`backend.app.domain.study.chain_summary.derive_chain_stop_reason`
returns ``"in_flight"`` — step 1 already excludes non-terminal
tails, but a chain with a still-running *interior* link must also
be excluded (mirrors the chain panel's terminal-only contract).
4. Skip any chain whose ``len(links) < 2`` — defensive; the
``parent_study_id IS NOT NULL`` candidate filter already implies
length >= 2, but a concurrent hard-delete of the anchor (no
``ondelete='SET NULL'`` on the self-FK, so this is rare) could
leave a single-row traversal.
5. Skip candidates whose :func:`get_chain_for_study` returns ``None``
— the concurrent-delete race where a chain member is hard-deleted
between the candidate query and the traversal (reachable via the
``hard_delete_study`` test teardown path). Mirrors the defensive
skip already in ``get_chain_for_study`` at the hydration step.
6. Stop once ``limit`` distinct chains are collected. Return the
``ChainTraversalResult`` list in tail-completion-DESC order
(preserved by the candidate-order traversal — the *first*
candidate hit for any chain is its newest terminal child, which
is the chain's tail).
"""
scan_cap = max(1, limit * _CHAIN_MAX_DESCENDANTS)
candidate_stmt = (
select(Study.id)
.where(
Study.parent_study_id.is_not(None),
Study.completed_at.is_not(None),
Study.status.in_(_TERMINAL_STUDY_STATUSES),
)
.order_by(Study.completed_at.desc(), Study.id.desc())
.limit(scan_cap)
)
if since is not None:
candidate_stmt = candidate_stmt.where(Study.completed_at >= since)
candidate_ids: list[str] = list((await db.execute(candidate_stmt)).scalars().all())

# Insertion-ordered dict — first hit per anchor wins, preserving
# tail-completion-DESC order. ``seen_study_ids`` tracks every member
# of every chain we've already resolved so subsequent candidates
# that belong to the same chain (a 6-link chain produces up to 5
# qualifying candidates) skip the redundant traversal call. Per
# Gemini Code Assist PR-444 finding #1 — eliminates the N+1 pattern
# without changing the dedup outcome.
by_anchor: dict[str, ChainTraversalResult] = {}
seen_study_ids: set[str] = set()
for candidate_id in candidate_ids:
if len(by_anchor) >= limit:
break
if candidate_id in seen_study_ids:
continue
traversal = await get_chain_for_study(db, candidate_id)
if traversal is None:
# Concurrent hard-delete between candidate query and traversal
# (e.g. test teardown). Skip silently per Story 1.1 task 5.
continue
Comment on lines +455 to +466
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

The current implementation of list_recent_completed_chains suffers from an N+1 query pattern. It iterates over all candidate IDs and calls get_chain_for_study for each one. Since multiple candidates can belong to the same chain, this results in redundant database queries and traversal calculations for the same chain. Keeping track of already traversed study IDs in a set allows us to skip redundant calls entirely, significantly improving performance.

    by_anchor: dict[str, ChainTraversalResult] = {}
    seen_study_ids: set[str] = set()
    for candidate_id in candidate_ids:
        if len(by_anchor) >= limit:
            break
        if candidate_id in seen_study_ids:
            continue
        traversal = await get_chain_for_study(db, candidate_id)
        if traversal is None:
            # Concurrent hard-delete between candidate query and traversal
            # (e.g. test teardown). Skip silently per Story 1.1 task 5.
            continue
        seen_study_ids.update(lk.id for lk in traversal.links)

# Mark every walked link as seen BEFORE the dedup check so a
# candidate from the same chain is skipped early on the next
# iteration even if this chain ends up excluded by the in-flight /
# length guards below.
seen_study_ids.update(link.id for link in traversal.links)
if traversal.anchor_id in by_anchor:
continue
if len(traversal.links) < 2:
# Defensive; the candidate filter implies length >= 2 unless
# the anchor was concurrently deleted out of the chain.
continue
stop_reason = derive_chain_stop_reason(traversal.links, traversal.anchor_trials)
if stop_reason == "in_flight":
# Interior link still running — chain isn't done; exclude.
continue
by_anchor[traversal.anchor_id] = traversal

return list(by_anchor.values())


# ---------------------------------------------------------------------------
# chore_e2e_test_rows_isolation Story 1.1 — hard-delete for test-only cleanup
# ---------------------------------------------------------------------------
Expand Down
3 changes: 3 additions & 0 deletions backend/tests/contract/test_openapi_surface.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,9 @@
("get", "/api/v1/studies/{study_id}/trials", "200"),
# feat_overnight_autopilot (PR #343) — auto-followup chain rollup.
("get", "/api/v1/studies/{study_id}/chain", "200"),
# feat_overnight_studies_summary_card (this PR) — recent-chains discovery
# feeding the "Ran while you were away" card on /studies.
("get", "/api/v1/studies/chains/recent", "200"),
# ----- /api/v1/proposals (feat_digest_proposal + feat_github_pr_worker) -----
("get", "/api/v1/studies/{study_id}/digest", "200"),
("post", "/api/v1/proposals", "201"),
Expand Down
Loading
Loading