diff --git a/backend/app/api/v1/schemas.py b/backend/app/api/v1/schemas.py index 54c70493..383d1bf2 100644 --- a/backend/app/api/v1/schemas.py +++ b/backend/app/api/v1/schemas.py @@ -575,6 +575,12 @@ class StudyConfigSpec(BaseModel): pruner: PrunerKind | None = None seed: int | None = None secondary_metrics: list[str] | None = None + baseline_params: dict[str, str | int | float | bool | None] | None = None + """feat_study_baseline_trial FR-6: explicit baseline-trial params (tier + b of the resolver fallback). Stored as-is inside ``studies.config`` + JSONB. Discriminated-value dict forbids nested objects/arrays — + Pydantic emits ``VALIDATION_ERROR`` (422) on violation. Stays in + ``config`` (not a top-level column) per spec D-7.""" auto_followup_depth: int | None = Field(default=None) """feat_auto_followup_studies FR-1 + D-12: 0..5 valid; 0 is the worker-internal terminal-state value (operators set None to opt out). @@ -696,6 +702,7 @@ class StudyDetail(BaseModel): optuna_study_name: str parent_study_id: str | None baseline_metric: float | None + baseline_trial_id: str | None best_metric: float | None best_trial_id: str | None created_at: datetime @@ -748,6 +755,11 @@ class TrialDetail(BaseModel): error: str | None started_at: datetime | None ended_at: datetime | None + is_baseline: bool = False + """feat_study_baseline_trial FR-8 — TRUE only for the off-band + non-Optuna baseline trial. The frontend uses this to filter the + trials-table by default and to render the "Baseline" badge under the + "Show baseline trial" toggle (FR-9).""" class TrialListResponse(BaseModel): diff --git a/backend/app/api/v1/studies.py b/backend/app/api/v1/studies.py index ef9ec799..bb873e75 100644 --- a/backend/app/api/v1/studies.py +++ b/backend/app/api/v1/studies.py @@ -137,6 +137,7 @@ async def _detail(db: AsyncSession, row: Study) -> StudyDetail: optuna_study_name=row.optuna_study_name, parent_study_id=row.parent_study_id, baseline_metric=row.baseline_metric, + baseline_trial_id=row.baseline_trial_id, best_metric=row.best_metric, best_trial_id=row.best_trial_id, created_at=row.created_at, @@ -726,6 +727,7 @@ async def list_study_trials( error=t.error, started_at=t.started_at, ended_at=t.ended_at, + is_baseline=t.is_baseline, ) for t in rows ], diff --git a/backend/app/db/models/study.py b/backend/app/db/models/study.py index 47846b4c..6516f8b2 100644 --- a/backend/app/db/models/study.py +++ b/backend/app/db/models/study.py @@ -93,7 +93,16 @@ class Study(Base): array. Recorded for audit only — the followup payload itself was inlined into ``search_space`` / ``name`` at study-create time.""" baseline_metric: Mapped[float | None] = mapped_column(Float, nullable=True) - """Single non-Optuna trial run before Optuna starts; populated by the orchestrator.""" + """Single non-Optuna trial run before Optuna starts; populated by the + orchestrator via :func:`backend.app.services.study_state.stamp_baseline_trial` + (feat_study_baseline_trial FR-12). NULL until baseline trial completes; + stays NULL when baseline is skipped or fails.""" + baseline_trial_id: Mapped[str | None] = mapped_column(String(36), nullable=True) + """Denormalized FK to the baseline trial row (the ``is_baseline=TRUE`` + trial in the ``trials`` table for this study). Not a formal FK — same + rationale as ``best_trial_id`` below: the orchestrator stamps it after + the baseline trial completes, no enforce-at-DB constraint + (feat_study_baseline_trial FR-1).""" best_metric: Mapped[float | None] = mapped_column(Float, nullable=True) """Denormalized winner metric value; set on study completion.""" best_trial_id: Mapped[str | None] = mapped_column(String(36), nullable=True) diff --git a/backend/app/db/models/trial.py b/backend/app/db/models/trial.py index 7b9a22e2..67454b57 100644 --- a/backend/app/db/models/trial.py +++ b/backend/app/db/models/trial.py @@ -33,7 +33,17 @@ from datetime import datetime from typing import Any -from sqlalchemy import CheckConstraint, DateTime, Float, ForeignKey, Integer, String, Text +from sqlalchemy import ( + Boolean, + CheckConstraint, + DateTime, + Float, + ForeignKey, + Integer, + String, + Text, + text, +) from sqlalchemy.dialects.postgresql import JSONB from sqlalchemy.orm import Mapped, mapped_column @@ -97,3 +107,13 @@ class Trial(Base): whatever step raised (adapter, render, search, score).""" started_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True), nullable=True) ended_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True), nullable=True) + is_baseline: Mapped[bool] = mapped_column(Boolean, nullable=False, server_default=text("FALSE")) + """Marker for the off-band non-Optuna baseline trial + (feat_study_baseline_trial FR-1). The baseline trial is one row per + study with ``optuna_trial_number=-1`` (NOT-NULL sentinel filler) and + ``is_baseline=TRUE``. Optuna's RDB never sees this row — the discriminator + is the boolean flag, not the trial number. Every aggregate / list / + confidence read path under :mod:`backend.app.db.repo.trial` MUST filter + ``is_baseline=FALSE`` (FR-11). The trials-listing API helper is the + one exception — it returns both Optuna and baseline rows so the UI + can render them under the "Show baseline trial" toggle (FR-9).""" diff --git a/backend/app/db/repo/trial.py b/backend/app/db/repo/trial.py index e7f3e00c..97c32669 100644 --- a/backend/app/db/repo/trial.py +++ b/backend/app/db/repo/trial.py @@ -224,12 +224,14 @@ async def aggregate_trials_summary(db: AsyncSession, study_id: str) -> TrialsSum func.max(Trial.primary_metric).filter(Trial.status == "complete").label("best"), ) .where(Trial.study_id == study_id) + .where(Trial.is_baseline.is_(False)) # FR-11: exclude baseline from aggregates .cte("summary") ) winner = ( select(Trial.id) .where(Trial.study_id == study_id) + .where(Trial.is_baseline.is_(False)) # FR-11 .where(Trial.status == "complete") .where(Trial.primary_metric == summary.c.best) .order_by(Trial.optuna_trial_number) # deterministic tiebreak diff --git a/backend/app/domain/study/auto_followup.py b/backend/app/domain/study/auto_followup.py index 7c69fd75..334f636e 100644 --- a/backend/app/domain/study/auto_followup.py +++ b/backend/app/domain/study/auto_followup.py @@ -3,12 +3,24 @@ Pure domain function deciding whether a completed study should enqueue a follow-up. No DB, no I/O, no async. -Per spec FR-2a (and locked in D-3): the gate is "lift-over-first-decile." -The parent's winner must beat the max metric of the parent's earliest -decile of complete trials by at least ``epsilon`` (default 0.005). When -``feat_study_baseline_trial`` ships and populates ``studies.baseline_metric``, -FR-2b activates and this module switches to "lift-over-baseline" via a -one-line change in :func:`evaluate_chain_gate`. +Per spec FR-2a (and locked in D-3): the gate is "lift-over-first-decile" +when no explicit baseline exists. **FR-2b activated** by +``feat_study_baseline_trial`` (2026-05-25): when +``parent.baseline_metric IS NOT NULL`` (i.e., the orchestrator's baseline +phase ran and successfully stamped the study), lift is computed directly +against the explicit baseline. Otherwise the existing implicit-baseline +(first-decile-max) fallback fires unchanged. + +**Direction-aware** (feat_study_baseline_trial FR-5): the gate now takes +a ``direction: Literal["maximize", "minimize"]`` kwarg (default +``"maximize"`` preserves the existing behavior). For minimize objectives, +lift signs flip so "better than baseline" is always positive — closes +a latent bug in the maximize-only implementation when minimize studies +land. ``ChainGateOutcome.first_decile_max`` is the legacy name kept for +backward compatibility; conceptually it's now the "first-decile extremum" +(max for maximize, min for minimize). Existing callers can rely on the +field name and the maximize-default; direction-aware callers pass +``parent.objective.get('direction')``. Ordering note: the spec/plan referenced ``created_at`` for trial sorting, but :class:`~backend.app.db.models.trial.Trial` exposes ``started_at`` @@ -26,7 +38,7 @@ from collections.abc import Iterable from dataclasses import dataclass from enum import StrEnum -from typing import Any +from typing import Any, Literal class ChainGateDecision(StrEnum): @@ -58,8 +70,11 @@ class ChainGateOutcome: epsilon: float = 0.005 -def compute_first_decile_max(complete_trials: Iterable[Any]) -> float | None: - """Return max(primary_metric) over the first decile of complete trials. +def compute_first_decile_max( + complete_trials: Iterable[Any], + direction: Literal["maximize", "minimize"] = "maximize", +) -> float | None: + """Return the first-decile extremum of complete trials' primary_metric. First decile = ``complete_trials_sorted[:max(1, len // 10)]`` (floor division, per spec FR-2a). Boundary cases: @@ -73,6 +88,13 @@ def compute_first_decile_max(complete_trials: Iterable[Any]) -> float | None: Sort key is ``optuna_trial_number`` ASC — see module docstring for why this is the right ordering field. + For ``direction='maximize'`` (the default, preserving existing + behavior) returns ``max(primary_metric)`` over the decile. For + ``direction='minimize'`` returns ``min(primary_metric)`` — i.e., the + most-easily-beaten value, which is the right baseline-shaped + comparison point under minimize semantics (feat_study_baseline_trial + FR-5). + Returns ``None`` when the first-decile slice has no usable ``primary_metric`` values (all NULL, or zero trials). """ @@ -85,7 +107,7 @@ def compute_first_decile_max(complete_trials: Iterable[Any]) -> float | None: metrics: list[float] = [t.primary_metric for t in decile if t.primary_metric is not None] if not metrics: return None - return max(metrics) + return max(metrics) if direction == "maximize" else min(metrics) def evaluate_chain_gate( @@ -93,35 +115,37 @@ def evaluate_chain_gate( complete_trials: Iterable[Any], *, epsilon: float = 0.005, + direction: Literal["maximize", "minimize"] = "maximize", ) -> ChainGateOutcome: """Decide whether to enqueue a follow-up study for ``parent``. Inputs are loaded by the caller — this function does no I/O. - Duck-typed signature (parent is Any) mirrors the - :func:`backend.app.domain.study.confidence.compute_study_confidence` - pattern at confidence.py:496 — lets tests pass ``SimpleNamespace`` - stand-ins without a Protocol class. Caller passes a real - :class:`~backend.app.db.models.study.Study` in production; tests pass - a SimpleNamespace with the required attributes - (``status``, ``best_metric``, ``config``). + **feat_study_baseline_trial FR-5 activation**: when + ``parent.baseline_metric IS NOT NULL`` (the orchestrator's baseline + phase stamped the study), lift is computed directly against the + explicit baseline. Otherwise the existing first-decile fallback + fires unchanged. ``ChainGateOutcome.first_decile_max`` is populated + ONLY when the fallback branch ran; the explicit-baseline branch + leaves it ``None`` (and the ``lift`` field carries the + explicit-baseline computation). + + **Direction-aware** (FR-5): the ``direction`` kwarg flips lift signs + for minimize objectives so the ``lift > epsilon`` gate predicate + works the same for both directions. Default ``"maximize"`` preserves + backward compatibility — existing callers that don't pass + ``direction`` continue to work. Decision matrix (in evaluation order): 1. ``parent.status in {'failed', 'cancelled'}`` → SKIP_PARENT_FAILED. - Defensive: the digest worker doesn't run on failed studies - (verified at backend/workers/orchestrator.py:452 — digest enqueue - only fires from ``_stop()`` after the ``completed`` transition), - so this branch fires only on manual invocation or race-with-cancel. - 2. ``config.auto_followup_depth`` missing or ``== 0`` → - SKIP_DEPTH_EXHAUSTED. The depth=0 leaf (worker-set terminal value - per FR-1 + D-12) hits this branch on its own enqueue invocation, - which is how the chain ends. - 3. ``parent.best_metric is None`` → SKIP_NO_LIFT (cannot compute - lift without a winner; defensive). - 4. ``first_decile_max`` is ``None`` (no usable trials) → SKIP_NO_LIFT. - 5. ``best_metric > first_decile_max + epsilon`` → ENQUEUE. - Otherwise → SKIP_NO_LIFT. + 2. ``config.auto_followup_depth`` missing or ``== 0`` → SKIP_DEPTH_EXHAUSTED. + 3. ``parent.best_metric is None`` → SKIP_NO_LIFT (defensive). + 4. **Explicit baseline (FR-5)**: ``parent.baseline_metric IS NOT NULL`` → + ``lift = (best - baseline)`` (maximize) or ``(baseline - best)`` + (minimize). Gate on ``lift > epsilon``. + 5. **Fallback**: ``first_decile_max`` is ``None`` → SKIP_NO_LIFT. + 6. Direction-normalized lift over first-decile extremum. """ if parent.status in {"failed", "cancelled"}: return ChainGateOutcome( @@ -144,7 +168,26 @@ def evaluate_chain_gate( epsilon=epsilon, ) - first_decile_max = compute_first_decile_max(complete_trials) + # FR-5: explicit-baseline branch — prefer parent.baseline_metric when set. + baseline_metric = getattr(parent, "baseline_metric", None) + if baseline_metric is not None: + lift = _direction_normalized_lift(parent.best_metric, baseline_metric, direction) + if lift > epsilon: + return ChainGateOutcome( + decision=ChainGateDecision.ENQUEUE, + lift=lift, + first_decile_max=None, # explicit-baseline branch — no decile compute + epsilon=epsilon, + ) + return ChainGateOutcome( + decision=ChainGateDecision.SKIP_NO_LIFT, + lift=lift, + first_decile_max=None, + epsilon=epsilon, + ) + + # Fallback: first-decile-extremum (implicit baseline, direction-aware). + first_decile_max = compute_first_decile_max(complete_trials, direction) if first_decile_max is None: return ChainGateOutcome( decision=ChainGateDecision.SKIP_NO_LIFT, @@ -153,7 +196,7 @@ def evaluate_chain_gate( epsilon=epsilon, ) - lift = parent.best_metric - first_decile_max + lift = _direction_normalized_lift(parent.best_metric, first_decile_max, direction) if lift > epsilon: return ChainGateOutcome( decision=ChainGateDecision.ENQUEUE, @@ -167,3 +210,14 @@ def evaluate_chain_gate( first_decile_max=first_decile_max, epsilon=epsilon, ) + + +def _direction_normalized_lift( + best_metric: float, + baseline_metric: float, + direction: Literal["maximize", "minimize"], +) -> float: + """Normalize lift sign so "better than baseline" is always positive.""" + if direction == "minimize": + return baseline_metric - best_metric + return best_metric - baseline_metric diff --git a/backend/app/domain/study/baseline_resolver.py b/backend/app/domain/study/baseline_resolver.py new file mode 100644 index 00000000..783eb326 --- /dev/null +++ b/backend/app/domain/study/baseline_resolver.py @@ -0,0 +1,203 @@ +"""Baseline-trial parameter resolver (feat_study_baseline_trial FR-3). + +Pure-domain async helper that resolves the parameter dict for the +non-Optuna baseline trial via a 4-tier fallback: + +1. **Tier (d) — Parent proposal config**: if ``study.parent_proposal_id`` + is set, return the params from the trial that the parent proposal + would have shipped (``proposal.study_trial_id``). +2. **Tier (c) — Parent study winner**: if ``study.parent_study_id`` is + set, return the params from the parent study's winning trial + (``parent.best_trial_id``). +3. **Tier (b) — Operator-supplied**: if ``study.config['baseline_params']`` + is set, return it directly (Pydantic already validated the dict shape + at create-time per ``CreateStudyRequest`` / ``StudyConfigSpec``). +4. **Tier (a) — Template defaults**: deterministic middle-of-range for + every declared parameter in ``study.search_space.params``: + + - ``FloatParam`` → ``(low + high) / 2.0`` (geometric mean + ``sqrt(low * high)`` when ``log=True``). + - ``IntParam`` → ``(low + high) // 2``. + - ``CategoricalParam`` → ``choices[(len(choices) - 1) // 2]`` + (lower midpoint for even-cardinality choice lists). + +Returns ``None`` only when tier (a) would produce an empty dict (the +search space has no declared params), in which case the orchestrator +skips the baseline trial entirely (see :func:`backend.workers. +orchestrator._resolve_and_enqueue_baseline`). + +The function is async because tiers (d) and (c) hit the DB to load +parent rows. It performs NO writes — pure read + compute. + +Spec: ``docs/02_product/planned_features/feat_study_baseline_trial/feature_spec.md`` §FR-3. +Decision log entries D-2 (4-tier fallback ordering) and D-7 +(``baseline_params`` lives in ``studies.config`` JSONB). +""" + +from __future__ import annotations + +import math +from typing import Any + +import structlog +from sqlalchemy.ext.asyncio import AsyncSession + +from backend.app.db import repo +from backend.app.db.models import Study +from backend.app.domain.study.search_space import ( + CategoricalParam, + FloatParam, + IntParam, + SearchSpace, +) + +logger = structlog.get_logger(__name__) + + +async def resolve_baseline_params( + db: AsyncSession, + study: Study, +) -> dict[str, Any] | None: + """Resolve baseline-trial params via the FR-3 4-tier fallback. + + Returns ``None`` only when no tier produced non-empty params (the + study's search space has no declared params and no parent / operator + override exists). Returns a non-empty dict otherwise. + + Logged structured events on fall-through: + + - ``baseline_resolve_parent_proposal_missing`` — tier (d) fell through + because the parent proposal's referenced trial is missing. + - ``baseline_resolve_parent_study_missing`` — tier (c) fell through + because the parent study or its best trial is missing. + """ + # Tier (d) — parent proposal config. + if study.parent_proposal_id is not None: + params = await _resolve_from_parent_proposal(db, study.parent_proposal_id) + if params is not None: + return params + + # Tier (c) — parent study winner. + if study.parent_study_id is not None: + params = await _resolve_from_parent_study(db, study.parent_study_id) + if params is not None: + return params + + # Tier (b) — operator-supplied. + params = _resolve_from_operator_supplied(study) + if params is not None: + return params + + # Tier (a) — template defaults. + return _resolve_from_template_defaults(study) + + +async def _resolve_from_parent_proposal( + db: AsyncSession, + parent_proposal_id: str, +) -> dict[str, Any] | None: + """Tier (d): the parent proposal's ``study_trial_id`` is the baseline. + + Returns ``None`` when the proposal or its referenced trial is missing + or has no params (cascade-delete race; treat as fall-through). + """ + proposal = await repo.get_proposal(db, parent_proposal_id) + if proposal is None or proposal.study_trial_id is None: + logger.info( + "baseline_resolve_parent_proposal_missing", + event_type="baseline_resolve_parent_proposal_missing", + parent_proposal_id=parent_proposal_id, + reason="proposal_or_study_trial_id_missing", + ) + return None + trial = await repo.get_trial(db, proposal.study_trial_id) + if trial is None or not trial.params: + logger.info( + "baseline_resolve_parent_proposal_missing", + event_type="baseline_resolve_parent_proposal_missing", + parent_proposal_id=parent_proposal_id, + study_trial_id=proposal.study_trial_id, + reason="trial_missing_or_empty_params", + ) + return None + return dict(trial.params) + + +async def _resolve_from_parent_study( + db: AsyncSession, + parent_study_id: str, +) -> dict[str, Any] | None: + """Tier (c): the parent study's ``best_trial_id`` is the baseline.""" + parent = await repo.get_study(db, parent_study_id) + if parent is None or parent.best_trial_id is None: + logger.info( + "baseline_resolve_parent_study_missing", + event_type="baseline_resolve_parent_study_missing", + parent_study_id=parent_study_id, + reason="parent_or_best_trial_id_missing", + ) + return None + trial = await repo.get_trial(db, parent.best_trial_id) + if trial is None or not trial.params: + logger.info( + "baseline_resolve_parent_study_missing", + event_type="baseline_resolve_parent_study_missing", + parent_study_id=parent_study_id, + best_trial_id=parent.best_trial_id, + reason="trial_missing_or_empty_params", + ) + return None + return dict(trial.params) + + +def _resolve_from_operator_supplied(study: Study) -> dict[str, Any] | None: + """Tier (b): ``study.config['baseline_params']`` if set + non-empty. + + Pydantic validated the dict[str, primitive] shape at create-time + (``StudyConfigSpec.baseline_params``); no re-validation here. + """ + config = study.config or {} + params = config.get("baseline_params") + if not isinstance(params, dict) or not params: + return None + return dict(params) + + +def _resolve_from_template_defaults(study: Study) -> dict[str, Any] | None: + """Tier (a): middle-of-range for every declared search-space param. + + Returns ``None`` when the search space has no params (impossible in + practice — ``SearchSpace.params`` is constrained `min_length=1` by + Pydantic — but defensive in case future iterations relax that). + """ + space = SearchSpace.model_validate(study.search_space) + if not space.params: + return None + + result: dict[str, Any] = {} + for name, param in space.params.items(): + result[name] = _midpoint(param) + return result + + +def _midpoint(param: FloatParam | IntParam | CategoricalParam) -> Any: + """Deterministic mid-of-range per parameter kind. + + - ``FloatParam`` with ``log=False``: arithmetic mean ``(low + high) / 2``. + - ``FloatParam`` with ``log=True``: geometric mean ``sqrt(low * high)``. + - ``IntParam``: integer division ``(low + high) // 2``. + - ``CategoricalParam``: ``choices[(len(choices) - 1) // 2]`` (lower + midpoint for even-cardinality lists). + """ + if isinstance(param, FloatParam): + if param.log: + return math.sqrt(param.low * param.high) + return (param.low + param.high) / 2.0 + if isinstance(param, IntParam): + return (param.low + param.high) // 2 + if isinstance(param, CategoricalParam): + return param.choices[(len(param.choices) - 1) // 2] + raise TypeError(f"unknown ParamSpec subtype: {type(param)!r}") + + +__all__ = ["resolve_baseline_params"] diff --git a/backend/app/domain/study/confidence.py b/backend/app/domain/study/confidence.py index b149811f..704f3677 100644 --- a/backend/app/domain/study/confidence.py +++ b/backend/app/domain/study/confidence.py @@ -501,6 +501,7 @@ def compute_study_confidence( runner_up_trial: Any | None, complete_trials_summary: list[tuple[float, int]], query_text_by_id: dict[str, str] | None = None, + baseline_trial: Any | None = None, ) -> ConfidenceShape | None: """Assemble the ``ConfidenceShape`` from pre-fetched DB data. @@ -602,14 +603,27 @@ def compute_study_confidence( # (cycle-2 GPT-5.5 F2 fix; AC-16 1-complete-trial case). ci_95 = bootstrap_ci_95(winner_values_for_metric) - # Comparison-based per-query signal — requires BOTH winner + runner-up - # to have per_query_metrics (the runner-up's primary_metric alone is - # not enough to compute deltas). + # Comparison-based per-query signal — requires winner + a comparison + # trial (baseline OR runner-up) to have per_query_metrics. + # + # feat_study_baseline_trial FR-4: prefer the baseline trial when set + # AND it has per_query_metrics. Otherwise fall back to runner-up #2. + # The 5 existing tests asserting `comparison_against == "runner_up"` + # stay green because their fixtures don't set baseline_trial. per_query_outcomes: PerQueryOutcomesShape | None = None - if runner_up_trial is not None and winner_per_query and runner_up_trial.per_query_metrics: + comparison_trial: Any | None = None + comparison_against_value: str = "runner_up" + if baseline_trial is not None and baseline_trial.per_query_metrics: + comparison_trial = baseline_trial + comparison_against_value = "baseline" + elif runner_up_trial is not None and runner_up_trial.per_query_metrics: + comparison_trial = runner_up_trial + comparison_against_value = "runner_up" + + if comparison_trial is not None and winner_per_query: outcome = compute_outcome_summary( winner_per_query=winner_per_query, - comparison_per_query=runner_up_trial.per_query_metrics, + comparison_per_query=comparison_trial.per_query_metrics, metric=per_query_key, ) if outcome is not None: @@ -621,7 +635,7 @@ def compute_study_confidence( improved=outcome.improved, unchanged=outcome.unchanged, regressed=outcome.regressed, - comparison_against="runner_up", # FR-3 locked for Phase 1 + comparison_against=comparison_against_value, top_regressors=regressor_rows, ) diff --git a/backend/app/services/study_confidence.py b/backend/app/services/study_confidence.py index 0aefa241..e163373c 100644 --- a/backend/app/services/study_confidence.py +++ b/backend/app/services/study_confidence.py @@ -48,11 +48,21 @@ async def fetch_study_confidence(db: AsyncSession, study: Study) -> ConfidenceSh if winner is None: return None + # Q1a: baseline trial (feat_study_baseline_trial FR-4). Only fetched + # when the study has baseline_trial_id stamped — i.e., the baseline + # phase ran AND succeeded AND was stamped via FR-12. + baseline_trial: Trial | None = None + if study.baseline_trial_id is not None: + baseline_trial = await repo.get_trial(db, study.baseline_trial_id) + # Q2: runner-up trial — 2nd-best complete trial by primary_metric. + # FR-11: exclude baseline rows so the runner-up classification compares + # ONLY against Optuna trials (the baseline lives under its own surface). runner_up_stmt = ( select(Trial) .where( Trial.study_id == study.id, + Trial.is_baseline.is_(False), Trial.status == "complete", Trial.id != winner.id, ) @@ -62,9 +72,14 @@ async def fetch_study_confidence(db: AsyncSession, study: Study) -> ConfidenceSh runner_up = (await db.execute(runner_up_stmt)).scalar_one_or_none() # Q3: complete-trials projection — (primary_metric, optuna_trial_number). + # FR-11: exclude baseline rows from convergence/late-stddev aggregates. summary_stmt = ( select(Trial.primary_metric, Trial.optuna_trial_number) - .where(Trial.study_id == study.id, Trial.status == "complete") + .where( + Trial.study_id == study.id, + Trial.is_baseline.is_(False), + Trial.status == "complete", + ) .order_by(Trial.optuna_trial_number.asc()) ) summary_rows = (await db.execute(summary_stmt)).all() @@ -102,6 +117,7 @@ async def fetch_study_confidence(db: AsyncSession, study: Study) -> ConfidenceSh study_best_metric=study.best_metric, winner_trial=winner, runner_up_trial=runner_up, + baseline_trial=baseline_trial, complete_trials_summary=complete_trials_summary, query_text_by_id=query_text_by_id, ) diff --git a/backend/app/services/study_state.py b/backend/app/services/study_state.py index 4b8b9c51..745ac9ba 100644 --- a/backend/app/services/study_state.py +++ b/backend/app/services/study_state.py @@ -44,10 +44,11 @@ from datetime import UTC, datetime import structlog -from sqlalchemy import event, inspect, select +from sqlalchemy import event, inspect, select, text from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker from sqlalchemy.orm import Session +from backend.app.db import repo from backend.app.db.models import Study logger = structlog.get_logger(__name__) @@ -61,6 +62,26 @@ class StudyNotFound(Exception): """Router → 404 ``STUDY_NOT_FOUND``.""" +class BaselineTrialNotFound(Exception): + """Raised by :func:`stamp_baseline_trial` when ``trial_id`` doesn't exist. + + feat_study_baseline_trial FR-12 — the orchestrator / worker MUST pass a + pre-generated UUIDv7 that corresponds to an inserted row by the time + the helper is called. Missing rows indicate either a cascade race or a + caller bug. + """ + + +class InvalidBaselineTrialState(Exception): + """Raised when the loaded baseline trial row has unexpected attributes. + + feat_study_baseline_trial FR-12 — preconditions: ``trial.study_id == + study_id``, ``trial.is_baseline == True``, ``trial.status == 'complete'``. + Anything else is a caller bug (the helper refuses to stamp + ``baseline_trial_id`` against a non-baseline / non-complete row). + """ + + class InvalidStateTransition(Exception): """Router → 409 ``INVALID_STATE_TRANSITION``.""" @@ -369,6 +390,93 @@ async def fail_study( return study +# --------------------------------------------------------------------------- +# Baseline-trial stamping helper (feat_study_baseline_trial FR-12) +# --------------------------------------------------------------------------- + + +async def stamp_baseline_trial( + db: AsyncSession, + study_id: str, + trial_id: str, + primary_metric: float, +) -> bool: + """Stamp ``studies.baseline_trial_id`` + ``baseline_metric`` idempotently. + + Single chokepoint for all three paths that durably write the baseline + FK (feat_study_baseline_trial D-12): + + 1. The orchestrator's fast-path stamp (FR-2 step 7). + 2. The worker's self-stamp on successful baseline completion + (FR-10 step 7). + 3. The ``resume_study`` re-stamp for unstamped complete baselines + (spec §9 idempotency). + + Returns ``True`` if this caller stamped (1 row affected by the UPDATE); + ``False`` if a sibling already stamped (race-tolerant — the + ``WHERE baseline_trial_id IS NULL`` predicate makes this idempotent). + + Raises :class:`BaselineTrialNotFound` if the trial row is missing + (caller bug — the orchestrator pre-generates the UUIDv7 and the worker + INSERTed before calling). Raises :class:`InvalidBaselineTrialState` + if the row's ``study_id`` / ``is_baseline`` / ``status`` don't match + expectations. + + **Commit is left to the caller.** Both the orchestrator and the worker + MUST call ``await db.commit()`` after this returns to durably land the + stamp. The async-session pattern in :func:`complete_study` / + :func:`fail_study` above sets the precedent. + """ + trial = await repo.get_trial(db, trial_id) + if trial is None: + raise BaselineTrialNotFound(f"baseline trial {trial_id!r} not found in trials table") + if trial.study_id != study_id: + raise InvalidBaselineTrialState( + f"baseline trial {trial_id!r} has study_id={trial.study_id!r}, expected {study_id!r}" + ) + if not trial.is_baseline: + raise InvalidBaselineTrialState( + f"trial {trial_id!r} is not a baseline row (is_baseline=False)" + ) + if trial.status != "complete": + raise InvalidBaselineTrialState( + f"baseline trial {trial_id!r} status={trial.status!r}, expected 'complete'" + ) + + # Idempotent UPDATE — only stamps if not already stamped. + result = await db.execute( + text( + "UPDATE studies " + "SET baseline_trial_id = :trial_id, baseline_metric = :primary_metric " + "WHERE id = :study_id AND baseline_trial_id IS NULL " + "RETURNING id" + ), + { + "trial_id": trial_id, + "primary_metric": primary_metric, + "study_id": study_id, + }, + ) + stamped = result.fetchone() is not None + if stamped: + logger.info( + "baseline_stamped", + event_type="baseline_stamped", + study_id=study_id, + trial_id=trial_id, + primary_metric=primary_metric, + ) + else: + logger.info( + "baseline_stamp_no_op", + event_type="baseline_stamp_no_op", + study_id=study_id, + trial_id=trial_id, + reason="baseline_trial_id_already_set", + ) + return stamped + + # --------------------------------------------------------------------------- # Event listener — module-level callable + idempotent installer. # --------------------------------------------------------------------------- diff --git a/backend/tests/integration/test_baseline_migration_round_trip.py b/backend/tests/integration/test_baseline_migration_round_trip.py new file mode 100644 index 00000000..a8575553 --- /dev/null +++ b/backend/tests/integration/test_baseline_migration_round_trip.py @@ -0,0 +1,236 @@ +"""``0020_studies_baseline_trial`` migration test (feat_study_baseline_trial Story 1.1). + +Asserts the schema shape of the two columns + partial unique index added by +``migrations/versions/0020_studies_baseline_trial.py``: + +* upgrade head adds ``studies.baseline_trial_id VARCHAR(36) NULL`` +* upgrade head adds ``trials.is_baseline BOOLEAN NOT NULL DEFAULT FALSE`` +* upgrade head creates ``uq_trials_study_baseline_complete`` partial unique + index with the correct WHERE predicate +* downgrade to 0019 drops all three artifacts +* upgrade → downgrade → upgrade round-trip preserves the other studies + + trials columns +* Idempotent re-run: running upgrade head twice does not raise + (the DO $$ ... IF NOT EXISTS $$ guards + CREATE INDEX IF NOT EXISTS make + the migration safe to re-apply, per AC-13). + +Mirrors ``test_clusters_target_filter_migration.py`` for skip semantics + +alembic invocation. +""" + +from __future__ import annotations + +import os +import socket +import subprocess +from collections.abc import Iterator +from pathlib import Path +from urllib.parse import urlparse + +import pytest +from sqlalchemy import create_engine, text + +from backend.app.core.settings import get_settings + +REPO = Path(__file__).resolve().parents[3] + + +def _postgres_reachable() -> bool: + if not os.environ.get("DATABASE_URL_FILE") or not os.environ.get("POSTGRES_PASSWORD_FILE"): + return False + try: + url = get_settings().database_url + except Exception: # noqa: BLE001 + return False + parsed = urlparse(url) + host = parsed.hostname or "localhost" + port = parsed.port or 5432 + try: + with socket.create_connection((host, port), timeout=1.0): + return True + except (TimeoutError, OSError): + return False + + +pytestmark = pytest.mark.skipif( + not _postgres_reachable(), + reason=( + "Postgres not reachable from this process — see " + "docs/03_runbooks/local-dev.md §'Local-vs-CI test layers'." + ), +) + + +def _alembic(*args: str) -> subprocess.CompletedProcess[str]: + return subprocess.run( + ["uv", "run", "alembic", *args], + cwd=REPO, + capture_output=True, + text=True, + check=True, + ) + + +def _sync_database_url() -> str: + return get_settings().database_url.replace("postgresql+asyncpg://", "postgresql://") + + +@pytest.fixture +def restore_head() -> Iterator[None]: + yield + try: + _alembic("upgrade", "head") + except subprocess.CalledProcessError: + pass + + +def _studies_columns(conn) -> dict[str, dict[str, object]]: + rows = conn.execute( + text( + "SELECT column_name, data_type, character_maximum_length, is_nullable " + "FROM information_schema.columns " + "WHERE table_schema = 'public' AND table_name = 'studies'" + ) + ).fetchall() + return {r[0]: {"data_type": r[1], "max_length": r[2], "nullable": r[3]} for r in rows} + + +def _trials_columns(conn) -> dict[str, dict[str, object]]: + rows = conn.execute( + text( + "SELECT column_name, data_type, is_nullable, column_default " + "FROM information_schema.columns " + "WHERE table_schema = 'public' AND table_name = 'trials'" + ) + ).fetchall() + return {r[0]: {"data_type": r[1], "nullable": r[2], "default": r[3]} for r in rows} + + +def _partial_index_predicate(conn, index_name: str) -> str | None: + """Return the WHERE predicate of a partial index, or None if missing.""" + row = conn.execute( + text("SELECT pg_get_indexdef(c.oid) FROM pg_class c WHERE c.relname = :name"), + {"name": index_name}, + ).fetchone() + return row[0] if row else None + + +@pytest.mark.integration +class TestBaselineTrialMigration: + def test_upgrade_adds_baseline_trial_id_column(self, restore_head: None) -> None: + _alembic("upgrade", "head") + engine = create_engine(_sync_database_url(), future=True) + try: + with engine.connect() as conn: + cols = _studies_columns(conn) + assert "baseline_trial_id" in cols, ( + "0020 upgrade should add studies.baseline_trial_id" + ) + col = cols["baseline_trial_id"] + assert col["data_type"] == "character varying" + assert col["max_length"] == 36 + assert col["nullable"] == "YES" + finally: + engine.dispose() + + def test_upgrade_adds_is_baseline_column(self, restore_head: None) -> None: + _alembic("upgrade", "head") + engine = create_engine(_sync_database_url(), future=True) + try: + with engine.connect() as conn: + cols = _trials_columns(conn) + assert "is_baseline" in cols, "0020 upgrade should add trials.is_baseline" + col = cols["is_baseline"] + assert col["data_type"] == "boolean" + assert col["nullable"] == "NO" + assert col["default"] is not None and "false" in str(col["default"]).lower() + finally: + engine.dispose() + + def test_upgrade_creates_partial_unique_index(self, restore_head: None) -> None: + _alembic("upgrade", "head") + engine = create_engine(_sync_database_url(), future=True) + try: + with engine.connect() as conn: + indexdef = _partial_index_predicate(conn, "uq_trials_study_baseline_complete") + assert indexdef is not None, ( + "0020 upgrade should create uq_trials_study_baseline_complete" + ) + # Predicate must include both is_baseline and status='complete'. + lower = indexdef.lower() + assert "is_baseline" in lower and "complete" in lower, ( + f"index predicate missing expected clauses: {indexdef!r}" + ) + assert "unique" in lower, f"index should be UNIQUE: {indexdef!r}" + finally: + engine.dispose() + + def test_downgrade_drops_columns_and_index(self, restore_head: None) -> None: + _alembic("upgrade", "head") + _alembic("downgrade", "0019") + engine = create_engine(_sync_database_url(), future=True) + try: + with engine.connect() as conn: + assert "baseline_trial_id" not in _studies_columns(conn) + assert "is_baseline" not in _trials_columns(conn) + assert _partial_index_predicate(conn, "uq_trials_study_baseline_complete") is None + finally: + engine.dispose() + + def test_round_trip_preserves_other_columns(self, restore_head: None) -> None: + _alembic("upgrade", "head") + engine = create_engine(_sync_database_url(), future=True) + try: + with engine.connect() as conn: + before_studies = set(_studies_columns(conn).keys()) + before_trials = set(_trials_columns(conn).keys()) + finally: + engine.dispose() + + _alembic("downgrade", "0019") + _alembic("upgrade", "head") + + engine = create_engine(_sync_database_url(), future=True) + try: + with engine.connect() as conn: + after_studies = set(_studies_columns(conn).keys()) + after_trials = set(_trials_columns(conn).keys()) + assert after_studies == before_studies + assert after_trials == before_trials + finally: + engine.dispose() + + def test_upgrade_is_idempotent(self, restore_head: None) -> None: + """Re-running ``alembic upgrade head`` with the columns + index + already present must not raise (AC-13 + plan F7). + + Implementation: after the first upgrade, the alembic_version table + already records 0020 as the head, so a second ``upgrade head`` is + a trivial no-op at the alembic level. To prove the migration's + SQL is itself idempotent, we set the alembic version back to 0019 + WITHOUT running the downgrade SQL (which would drop the columns), + then re-run upgrade — exercising the IF NOT EXISTS guards. + """ + _alembic("upgrade", "head") + + engine = create_engine(_sync_database_url(), future=True) + try: + with engine.begin() as conn: + conn.execute(text("UPDATE alembic_version SET version_num = '0019'")) + finally: + engine.dispose() + + # Re-run upgrade — should be a no-op because all idempotency guards + # see the columns + index already present. + _alembic("upgrade", "head") + + engine = create_engine(_sync_database_url(), future=True) + try: + with engine.connect() as conn: + assert "baseline_trial_id" in _studies_columns(conn) + assert "is_baseline" in _trials_columns(conn) + assert ( + _partial_index_predicate(conn, "uq_trials_study_baseline_complete") is not None + ) + finally: + engine.dispose() diff --git a/backend/tests/integration/test_study_lifecycle.py b/backend/tests/integration/test_study_lifecycle.py index 5acff2de..cddeb707 100644 --- a/backend/tests/integration/test_study_lifecycle.py +++ b/backend/tests/integration/test_study_lifecycle.py @@ -69,13 +69,24 @@ def __init__(self, storage: Any) -> None: self.run_trial_tasks: list[asyncio.Task[None]] = [] self.enqueued: list[tuple[str, tuple[Any, ...]]] = [] - async def enqueue_job(self, func_name: str, *args: Any, **_kwargs: Any) -> None: + async def enqueue_job(self, func_name: str, *args: Any, **_kwargs: Any) -> object: self.enqueued.append((func_name, args)) if func_name == "run_trial": from backend.workers.trials import run_trial task = asyncio.create_task(run_trial({"optuna_storage": self._storage}, *args)) self.run_trial_tasks.append(task) + elif func_name == "run_baseline_trial": + # feat_study_baseline_trial Story 1.7: orchestrator enqueues + # run_baseline_trial before the Optuna loop. Dispatch inline + # so the wait helpers can observe a terminal trial row. + from backend.workers.baseline import run_baseline_trial + + task = asyncio.create_task(run_baseline_trial({}, *args)) + self.run_trial_tasks.append(task) + # Return a non-None sentinel so the orchestrator's BaselineEnqueueResult + # treats this as kind='enqueued' rather than 'deduped'. + return object() async def close(self) -> None: for task in self.run_trial_tasks: @@ -95,7 +106,14 @@ async def _running_orchestrator( # Speed up the polling loop for tests. original_tick = orchestrator._REPLENISH_TICK_S + original_baseline_floor = orchestrator._BASELINE_WAIT_FLOOR_S + original_baseline_margin = orchestrator._BASELINE_WAIT_MARGIN_S orchestrator._REPLENISH_TICK_S = tick_s + # feat_study_baseline_trial: cap the baseline-phase wait at ~2s in + # tests (production default 60s minimum is too slow for the + # 30s-test-timeout test_study_cancel test). + orchestrator._BASELINE_WAIT_FLOOR_S = 2.0 + orchestrator._BASELINE_WAIT_MARGIN_S = 1.0 storage = pool._storage ctx: dict[str, Any] = {"optuna_storage": storage, "arq_pool": pool} task = asyncio.create_task(orchestrator.start_study(ctx, fixture.study_id)) @@ -103,6 +121,8 @@ async def _running_orchestrator( yield task finally: orchestrator._REPLENISH_TICK_S = original_tick + orchestrator._BASELINE_WAIT_FLOOR_S = original_baseline_floor + orchestrator._BASELINE_WAIT_MARGIN_S = original_baseline_margin if not task.done(): task.cancel() with pytest.raises((asyncio.CancelledError, BaseException)): diff --git a/backend/tests/integration/test_study_lifecycle_migration.py b/backend/tests/integration/test_study_lifecycle_migration.py index 9b79ed1d..e4f92bd0 100644 --- a/backend/tests/integration/test_study_lifecycle_migration.py +++ b/backend/tests/integration/test_study_lifecycle_migration.py @@ -422,6 +422,7 @@ class TestNotNullCoverage: ("studies", "optuna_study_name", "NO"), ("studies", "parent_study_id", "YES"), ("studies", "baseline_metric", "YES"), + ("studies", "baseline_trial_id", "YES"), # feat_study_baseline_trial 0020 ("studies", "best_metric", "YES"), ("studies", "best_trial_id", "YES"), ("studies", "created_at", "NO"), @@ -439,6 +440,7 @@ class TestNotNullCoverage: ("trials", "error", "YES"), ("trials", "started_at", "YES"), ("trials", "ended_at", "YES"), + ("trials", "is_baseline", "NO"), # feat_study_baseline_trial 0020 # proposals ("proposals", "id", "NO"), ("proposals", "study_id", "YES"), diff --git a/backend/tests/unit/domain/study/test_auto_followup.py b/backend/tests/unit/domain/study/test_auto_followup.py index e07be182..8295dcf9 100644 --- a/backend/tests/unit/domain/study/test_auto_followup.py +++ b/backend/tests/unit/domain/study/test_auto_followup.py @@ -31,12 +31,15 @@ def _study( status: str = "completed", best_metric: float | None = 0.5, auto_followup_depth: int | None = 3, + baseline_metric: float | None = None, ) -> SimpleNamespace: """Build a Study stand-in. The domain function reads ``status``, - ``best_metric``, and ``config[auto_followup_depth]`` only.""" + ``best_metric``, ``config[auto_followup_depth]``, and (post- + feat_study_baseline_trial) ``baseline_metric``.""" return SimpleNamespace( status=status, best_metric=best_metric, + baseline_metric=baseline_metric, config={"auto_followup_depth": auto_followup_depth}, ) @@ -104,12 +107,85 @@ def test_partial_none_metrics_in_decile_skips_them(self) -> None: trials += [_trial(num=i, metric=0.9) for i in range(2, 20)] assert compute_first_decile_max(trials) == 0.5 + def test_minimize_returns_min_not_max(self) -> None: + """feat_study_baseline_trial FR-5: direction-aware extremum.""" + trials = [_trial(num=0, metric=0.5), _trial(num=1, metric=0.2)] + trials += [_trial(num=i, metric=0.9) for i in range(2, 20)] + # len=20 → first 2 trials. Maximize: max(0.5, 0.2) = 0.5. + assert compute_first_decile_max(trials, "maximize") == 0.5 + # Minimize: min(0.5, 0.2) = 0.2. + assert compute_first_decile_max(trials, "minimize") == 0.2 + # --------------------------------------------------------------------------- # evaluate_chain_gate # --------------------------------------------------------------------------- +class TestEvaluateChainGateBaselineBranch: + """feat_study_baseline_trial FR-5: explicit-baseline branch.""" + + def test_baseline_branch_enqueues_when_lift_exceeds_epsilon(self) -> None: + """AC-7: parent.baseline_metric set → lift = best - baseline.""" + parent = _study(best_metric=0.65, baseline_metric=0.55, auto_followup_depth=3) + # complete_trials is irrelevant when baseline_metric is set. + outcome = evaluate_chain_gate(parent, [], direction="maximize") + assert outcome.decision is ChainGateDecision.ENQUEUE + assert outcome.lift == pytest.approx(0.10) + # first_decile_max is None when the explicit-baseline branch fires. + assert outcome.first_decile_max is None + + def test_baseline_branch_skips_when_lift_within_epsilon(self) -> None: + parent = _study(best_metric=0.553, baseline_metric=0.55, auto_followup_depth=3) + outcome = evaluate_chain_gate(parent, [], direction="maximize") + assert outcome.decision is ChainGateDecision.SKIP_NO_LIFT + assert outcome.lift == pytest.approx(0.003) + assert outcome.first_decile_max is None + + def test_fallback_to_first_decile_when_baseline_metric_is_none(self) -> None: + """AC-8: baseline_metric=None → falls back to first-decile.""" + trials = [_trial(num=i, metric=0.30) for i in range(20)] + parent = _study(best_metric=0.65, baseline_metric=None, auto_followup_depth=3) + outcome = evaluate_chain_gate(parent, trials, direction="maximize") + assert outcome.decision is ChainGateDecision.ENQUEUE + assert outcome.lift == pytest.approx(0.35) + assert outcome.first_decile_max == 0.30 + + +class TestEvaluateChainGateDirectionAware: + """feat_study_baseline_trial FR-5 / AC-18.""" + + def test_minimize_direction_with_baseline(self) -> None: + """AC-18: minimize objective inverts lift sign so 'better than + baseline' is always positive.""" + # Lower is better. winner 0.30 beats baseline 0.50 by 0.20. + parent = _study(best_metric=0.30, baseline_metric=0.50, auto_followup_depth=3) + outcome = evaluate_chain_gate(parent, [], direction="minimize") + assert outcome.decision is ChainGateDecision.ENQUEUE + assert outcome.lift == pytest.approx(0.20) + + def test_minimize_direction_with_first_decile_fallback(self) -> None: + """Minimize + first-decile fallback: extremum is min, lift is + first_decile_min - best.""" + # First decile of 20 trials → first 2 trials. + trials = [_trial(num=0, metric=0.5), _trial(num=1, metric=0.4)] + trials += [_trial(num=i, metric=0.1) for i in range(2, 20)] + # Minimize: first_decile_min = 0.4. winner = 0.10 beats by 0.30. + parent = _study(best_metric=0.10, baseline_metric=None, auto_followup_depth=3) + outcome = evaluate_chain_gate(parent, trials, direction="minimize") + assert outcome.decision is ChainGateDecision.ENQUEUE + assert outcome.lift == pytest.approx(0.30) + assert outcome.first_decile_max == 0.4 # the min in minimize mode + + def test_maximize_default_preserves_backward_compat(self) -> None: + """No direction kwarg → defaults to maximize (existing behavior).""" + trials = [_trial(num=i, metric=0.30) for i in range(20)] + parent = _study(best_metric=0.42, auto_followup_depth=3) + outcome = evaluate_chain_gate(parent, trials) + assert outcome.decision is ChainGateDecision.ENQUEUE + assert outcome.lift == pytest.approx(0.12) + + class TestEvaluateChainGate: def test_lift_above_epsilon_returns_enqueue(self) -> None: # parent.best_metric=0.42, first_decile_max=0.30, lift=0.12 > epsilon=0.005 diff --git a/backend/tests/unit/domain/study/test_baseline_resolver.py b/backend/tests/unit/domain/study/test_baseline_resolver.py new file mode 100644 index 00000000..cb06b367 --- /dev/null +++ b/backend/tests/unit/domain/study/test_baseline_resolver.py @@ -0,0 +1,310 @@ +"""Unit tests for :mod:`backend.app.domain.study.baseline_resolver`. + +Covers FR-3's 4-tier fallback resolver: + +- Tier (d) parent proposal config +- Tier (c) parent study winner +- Tier (b) operator-supplied +- Tier (a) template defaults (middle-of-range) + +Plus the fall-through cascades and the log emission for missing-parent +edges (cascade-delete races). +""" + +from __future__ import annotations + +import math +from types import SimpleNamespace +from typing import Any +from unittest.mock import AsyncMock + +import pytest + +from backend.app.db import repo as _db_repo +from backend.app.domain.study import baseline_resolver # noqa: F401 # exported for symmetry +from backend.app.domain.study.baseline_resolver import ( + _midpoint, + _resolve_from_operator_supplied, + _resolve_from_template_defaults, + resolve_baseline_params, +) +from backend.app.domain.study.search_space import ( + CategoricalParam, + FloatParam, + IntParam, +) + + +def _study(**overrides: Any) -> Any: + """Build a SimpleNamespace stand-in for a Study row. + + Tests use SimpleNamespace because the resolver is duck-typed (per + the same pattern as `compute_study_confidence`); no need to construct + real ORM rows for unit tests. + """ + base = { + "id": "study-1", + "parent_proposal_id": None, + "parent_study_id": None, + "config": {}, + "search_space": { + "params": { + "boost_title": {"type": "float", "low": 0.5, "high": 10.0}, + } + }, + } + base.update(overrides) + return SimpleNamespace(**base) + + +# --------------------------------------------------------------------------- +# Tier (a) — template defaults (pure; no DB) +# --------------------------------------------------------------------------- + + +class TestTemplateDefaultsMidpoint: + def test_float_param_linear_midpoint(self) -> None: + result = _midpoint(FloatParam(type="float", low=0.5, high=10.0, log=False)) + assert result == 5.25 + + def test_float_param_log_midpoint_geometric_mean(self) -> None: + # sqrt(0.1 * 10.0) = 1.0 + result = _midpoint(FloatParam(type="float", low=0.1, high=10.0, log=True)) + assert math.isclose(result, 1.0) + + def test_int_param_lower_midpoint_for_even_range(self) -> None: + # (1 + 4) // 2 = 2 — lower midpoint between 2 and 3. + result = _midpoint(IntParam(type="int", low=1, high=4)) + assert result == 2 + + def test_int_param_midpoint_for_odd_range(self) -> None: + # (1 + 5) // 2 = 3 — true median. + result = _midpoint(IntParam(type="int", low=1, high=5)) + assert result == 3 + + def test_categorical_param_lower_midpoint_for_even_choices(self) -> None: + # ['a', 'b'] → (2-1)//2 = 0 → 'a' (lower midpoint). + result = _midpoint(CategoricalParam(type="categorical", choices=["a", "b"])) + assert result == "a" + + def test_categorical_param_lower_midpoint_for_four_choices(self) -> None: + # ['a','b','c','d'] → (4-1)//2 = 1 → 'b' (lower midpoint between b/c). + result = _midpoint(CategoricalParam(type="categorical", choices=["a", "b", "c", "d"])) + assert result == "b" + + def test_categorical_param_median_for_odd_choices(self) -> None: + # ['a','b','c'] → (3-1)//2 = 1 → 'b' (true median). + result = _midpoint(CategoricalParam(type="categorical", choices=["a", "b", "c"])) + assert result == "b" + + def test_template_defaults_returns_dict_for_multi_param_space(self) -> None: + study = _study( + search_space={ + "params": { + "boost_title": {"type": "float", "low": 0.5, "high": 10.0}, + "min_should_match": {"type": "int", "low": 1, "high": 5}, + "operator": { + "type": "categorical", + "choices": ["and", "or"], + }, + } + } + ) + result = _resolve_from_template_defaults(study) + assert result == { + "boost_title": 5.25, + "min_should_match": 3, + "operator": "and", # lower midpoint of ['and','or'] + } + + +# --------------------------------------------------------------------------- +# Tier (b) — operator-supplied +# --------------------------------------------------------------------------- + + +class TestOperatorSupplied: + def test_returns_dict_when_baseline_params_present(self) -> None: + study = _study(config={"baseline_params": {"boost_title": 1.5}}) + assert _resolve_from_operator_supplied(study) == {"boost_title": 1.5} + + def test_returns_none_when_config_absent(self) -> None: + study = _study(config={}) + assert _resolve_from_operator_supplied(study) is None + + def test_returns_none_when_baseline_params_null(self) -> None: + study = _study(config={"baseline_params": None}) + assert _resolve_from_operator_supplied(study) is None + + def test_returns_none_when_baseline_params_empty_dict(self) -> None: + # Empty dict means operator-supplied is treated as not-supplied; + # resolver falls through to template defaults. + study = _study(config={"baseline_params": {}}) + assert _resolve_from_operator_supplied(study) is None + + def test_returns_copy_not_alias(self) -> None: + original = {"boost_title": 1.5} + study = _study(config={"baseline_params": original}) + result = _resolve_from_operator_supplied(study) + assert result == original + assert result is not original # defensive copy + + +# --------------------------------------------------------------------------- +# Top-level resolver — async dispatch through the 4 tiers +# --------------------------------------------------------------------------- + + +@pytest.fixture +def mock_db() -> Any: + """Mock AsyncSession — never actually executes SQL because we + monkeypatch repo functions.""" + return AsyncMock() + + +class TestTopLevelResolver: + async def test_tier_d_parent_proposal_hits_when_set( + self, mock_db: Any, monkeypatch: pytest.MonkeyPatch + ) -> None: + study = _study(parent_proposal_id="proposal-1") + proposal = SimpleNamespace(id="proposal-1", study_trial_id="trial-1") + trial = SimpleNamespace(id="trial-1", params={"boost_title": 7.7}) + + monkeypatch.setattr( + _db_repo, + "get_proposal", + AsyncMock(return_value=proposal), + ) + monkeypatch.setattr(_db_repo, "get_trial", AsyncMock(return_value=trial)) + + result = await resolve_baseline_params(mock_db, study) + assert result == {"boost_title": 7.7} + + async def test_tier_d_falls_through_when_proposal_missing( + self, mock_db: Any, monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture + ) -> None: + study = _study(parent_proposal_id="proposal-missing") + monkeypatch.setattr( + _db_repo, + "get_proposal", + AsyncMock(return_value=None), + ) + # Falls through to tier (a) template defaults. + result = await resolve_baseline_params(mock_db, study) + assert result == {"boost_title": 5.25} + + async def test_tier_d_falls_through_when_trial_missing( + self, mock_db: Any, monkeypatch: pytest.MonkeyPatch + ) -> None: + study = _study(parent_proposal_id="proposal-1") + proposal = SimpleNamespace(id="proposal-1", study_trial_id="trial-missing") + monkeypatch.setattr( + _db_repo, + "get_proposal", + AsyncMock(return_value=proposal), + ) + monkeypatch.setattr(_db_repo, "get_trial", AsyncMock(return_value=None)) + result = await resolve_baseline_params(mock_db, study) + # Falls through to tier (a). + assert result == {"boost_title": 5.25} + + async def test_tier_d_falls_through_when_proposal_has_no_study_trial_id( + self, mock_db: Any, monkeypatch: pytest.MonkeyPatch + ) -> None: + study = _study(parent_proposal_id="proposal-1") + proposal = SimpleNamespace(id="proposal-1", study_trial_id=None) + monkeypatch.setattr( + _db_repo, + "get_proposal", + AsyncMock(return_value=proposal), + ) + result = await resolve_baseline_params(mock_db, study) + assert result == {"boost_title": 5.25} + + async def test_tier_c_parent_study_winner_hits_when_set( + self, mock_db: Any, monkeypatch: pytest.MonkeyPatch + ) -> None: + study = _study(parent_study_id="parent-1") + parent = SimpleNamespace(id="parent-1", best_trial_id="best-1") + trial = SimpleNamespace(id="best-1", params={"boost_title": 3.3}) + + monkeypatch.setattr(_db_repo, "get_study", AsyncMock(return_value=parent)) + monkeypatch.setattr(_db_repo, "get_trial", AsyncMock(return_value=trial)) + + result = await resolve_baseline_params(mock_db, study) + assert result == {"boost_title": 3.3} + + async def test_tier_c_falls_through_when_parent_missing( + self, mock_db: Any, monkeypatch: pytest.MonkeyPatch + ) -> None: + study = _study(parent_study_id="parent-missing") + monkeypatch.setattr(_db_repo, "get_study", AsyncMock(return_value=None)) + result = await resolve_baseline_params(mock_db, study) + # Falls through to tier (a). + assert result == {"boost_title": 5.25} + + async def test_tier_b_operator_supplied_hits_when_present(self, mock_db: Any) -> None: + study = _study(config={"baseline_params": {"boost_title": 1.5}}) + result = await resolve_baseline_params(mock_db, study) + assert result == {"boost_title": 1.5} + + async def test_tier_a_falls_through_to_template_defaults(self, mock_db: Any) -> None: + study = _study() # no parent, no operator override + result = await resolve_baseline_params(mock_db, study) + # Tier (a) midpoint of [0.5, 10.0] = 5.25. + assert result == {"boost_title": 5.25} + + async def test_full_fall_through_d_to_c_to_b_to_a( + self, mock_db: Any, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Tier (d) misses → tier (c) misses → tier (b) present → returns tier (b).""" + study = _study( + parent_proposal_id="proposal-missing", + parent_study_id="parent-missing", + config={"baseline_params": {"boost_title": 2.2}}, + ) + monkeypatch.setattr( + _db_repo, + "get_proposal", + AsyncMock(return_value=None), + ) + monkeypatch.setattr(_db_repo, "get_study", AsyncMock(return_value=None)) + result = await resolve_baseline_params(mock_db, study) + assert result == {"boost_title": 2.2} + + async def test_tier_priority_d_beats_b( + self, mock_db: Any, monkeypatch: pytest.MonkeyPatch + ) -> None: + """When both tier (d) and tier (b) are present, (d) wins.""" + study = _study( + parent_proposal_id="proposal-1", + config={"baseline_params": {"boost_title": 99.9}}, + ) + proposal = SimpleNamespace(id="proposal-1", study_trial_id="trial-1") + trial = SimpleNamespace(id="trial-1", params={"boost_title": 7.7}) + monkeypatch.setattr( + _db_repo, + "get_proposal", + AsyncMock(return_value=proposal), + ) + monkeypatch.setattr(_db_repo, "get_trial", AsyncMock(return_value=trial)) + result = await resolve_baseline_params(mock_db, study) + # Tier (d) wins, not 99.9. + assert result == {"boost_title": 7.7} + + +# --------------------------------------------------------------------------- +# Defense-in-depth — empty search space +# --------------------------------------------------------------------------- + + +class TestEdgeCases: + def test_unknown_param_type_raises_typeerror(self) -> None: + """The resolver dispatches on concrete subtypes; an unknown type + should fail loud rather than silently returning None.""" + + class UnknownParam: + pass + + with pytest.raises(TypeError, match="unknown ParamSpec subtype"): + _midpoint(UnknownParam()) # type: ignore[arg-type] diff --git a/backend/tests/unit/domain/study/test_confidence.py b/backend/tests/unit/domain/study/test_confidence.py index 738adaa2..581e3096 100644 --- a/backend/tests/unit/domain/study/test_confidence.py +++ b/backend/tests/unit/domain/study/test_confidence.py @@ -461,6 +461,120 @@ def test_ci_95_independent_of_runner_up_per_query(self) -> None: assert result.convergence is None # only 1 trial < CONVERGENCE_MIN_COMPLETE +class TestBaselineBranch: + """FR-4 baseline-branch coverage for compute_study_confidence. + + When study.baseline_trial_id is stamped AND the baseline trial has + per_query_metrics, the comparison source flips from runner-up to + baseline (AC-4). Otherwise falls back to runner-up (AC-5 / AC-6). + """ + + def test_baseline_branch_when_baseline_has_per_query(self) -> None: + """AC-4: baseline + winner both have per_query → comparison_against = baseline.""" + winner = _trial( + optuna_trial_number=5, + primary_metric=0.7, + per_query_metrics={ + "q1": {"ndcg@10": 0.7}, + "q2": {"ndcg@10": 0.5}, + }, + ) + baseline = _trial( + optuna_trial_number=-1, # baseline sentinel + primary_metric=0.5, + per_query_metrics={ + "q1": {"ndcg@10": 0.4}, + "q2": {"ndcg@10": 0.6}, + }, + ) + runner_up = _trial( + optuna_trial_number=2, + primary_metric=0.65, + per_query_metrics={ + "q1": {"ndcg@10": 0.66}, + "q2": {"ndcg@10": 0.51}, + }, + ) + + result = compute_study_confidence( + study_objective={"metric": "ndcg", "k": 10, "direction": "maximize"}, + study_best_metric=0.7, + winner_trial=winner, + runner_up_trial=runner_up, + baseline_trial=baseline, + complete_trials_summary=[(0.7, 5), (0.65, 2)], + query_text_by_id={"q1": "q1 text", "q2": "q2 text"}, + ) + assert result is not None + assert result.per_query_outcomes is not None + # Comparison is against baseline now, NOT runner_up. + assert result.per_query_outcomes.comparison_against == "baseline" + # q1: winner 0.7 - baseline 0.4 = +0.3 > 0.01 → improved + # q2: winner 0.5 - baseline 0.6 = -0.1 < -0.01 → regressed + assert result.per_query_outcomes.improved == 1 + assert result.per_query_outcomes.regressed == 1 + + def test_falls_back_to_runner_up_when_baseline_is_none(self) -> None: + """AC-5: baseline_trial=None falls back to runner-up comparison.""" + winner = _trial( + optuna_trial_number=5, + primary_metric=0.7, + per_query_metrics={"q1": {"ndcg@10": 0.7}}, + ) + runner_up = _trial( + optuna_trial_number=2, + primary_metric=0.65, + per_query_metrics={"q1": {"ndcg@10": 0.66}}, + ) + result = compute_study_confidence( + study_objective={"metric": "ndcg", "k": 10, "direction": "maximize"}, + study_best_metric=0.7, + winner_trial=winner, + runner_up_trial=runner_up, + baseline_trial=None, + complete_trials_summary=[(0.7, 5), (0.65, 2)], + query_text_by_id={}, + ) + assert result is not None + assert result.per_query_outcomes is not None + assert result.per_query_outcomes.comparison_against == "runner_up" + + def test_falls_back_to_runner_up_when_baseline_has_no_per_query(self) -> None: + """AC-6: baseline_trial has per_query_metrics=None → fall back to runner-up. + + This is the "baseline failed mid-score" edge case where the + baseline row exists but never got per-query data populated. + """ + winner = _trial( + optuna_trial_number=5, + primary_metric=0.7, + per_query_metrics={"q1": {"ndcg@10": 0.7}}, + ) + baseline = _trial( + optuna_trial_number=-1, + primary_metric=0.5, + per_query_metrics=None, # the AC-6 edge + ) + runner_up = _trial( + optuna_trial_number=2, + primary_metric=0.65, + per_query_metrics={"q1": {"ndcg@10": 0.66}}, + ) + result = compute_study_confidence( + study_objective={"metric": "ndcg", "k": 10, "direction": "maximize"}, + study_best_metric=0.7, + winner_trial=winner, + runner_up_trial=runner_up, + baseline_trial=baseline, + complete_trials_summary=[(0.7, 5), (0.65, 2)], + query_text_by_id={}, + ) + assert result is not None + assert result.per_query_outcomes is not None + # Falls back to runner-up. + assert result.per_query_outcomes.comparison_against == "runner_up" + + # Sanity check that all constants are defined and referenced (drift guard). def test_constants_exported() -> None: assert BOOTSTRAP_MIN_N_QUERIES == 5 diff --git a/backend/tests/unit/services/test_stamp_baseline_trial.py b/backend/tests/unit/services/test_stamp_baseline_trial.py new file mode 100644 index 00000000..c071e3e3 --- /dev/null +++ b/backend/tests/unit/services/test_stamp_baseline_trial.py @@ -0,0 +1,193 @@ +"""Unit tests for :func:`backend.app.services.study_state.stamp_baseline_trial`. + +The helper does one DB read (``repo.get_trial``) and one DB write (idempotent +UPDATE via raw SQL). Both are mocked here — integration coverage lives in +``backend/tests/integration/test_stamp_baseline_trial_integration.py``. + +Spec: feat_study_baseline_trial FR-12. AC-1 / AC-16 depend on this contract. +""" + +from __future__ import annotations + +from types import SimpleNamespace +from typing import Any +from unittest.mock import AsyncMock, MagicMock + +import pytest +import structlog + +from backend.app.db import repo as _db_repo +from backend.app.services.study_state import ( + BaselineTrialNotFound, + InvalidBaselineTrialState, + stamp_baseline_trial, +) + + +def _trial(**overrides: Any) -> Any: + """SimpleNamespace stand-in for a Trial row.""" + base = { + "id": "trial-1", + "study_id": "study-1", + "is_baseline": True, + "status": "complete", + "primary_metric": 0.612, + } + base.update(overrides) + return SimpleNamespace(**base) + + +@pytest.fixture +def mock_db() -> Any: + """AsyncMock AsyncSession — execute() returns a configurable mock.""" + db = AsyncMock() + return db + + +def _stub_execute_returns_row(db: Any, row: object | None) -> None: + """Configure db.execute() to simulate the UPDATE ... RETURNING result. + + db.execute(...) is awaited and returns a Result whose .fetchone() is + synchronous (per SQLAlchemy 2.0 async). The stamping helper calls + `result.fetchone()` — None means 0 rows affected, a row means 1. + """ + result = MagicMock() + result.fetchone = MagicMock(return_value=row) + db.execute = AsyncMock(return_value=result) + + +class TestStampBaselineTrial: + async def test_happy_path_stamps_and_returns_true( + self, mock_db: Any, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setattr( + _db_repo, + "get_trial", + AsyncMock(return_value=_trial()), + ) + _stub_execute_returns_row(mock_db, ("study-1",)) + + stamped = await stamp_baseline_trial(mock_db, "study-1", "trial-1", 0.612) + + assert stamped is True + assert mock_db.execute.await_count == 1 + # The UPDATE must use named bind params (plan F8). + _stmt, params = mock_db.execute.await_args.args + assert params == { + "trial_id": "trial-1", + "primary_metric": 0.612, + "study_id": "study-1", + } + + async def test_already_stamped_returns_false( + self, mock_db: Any, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Idempotent: a sibling already stamped → UPDATE affects 0 rows + (WHERE baseline_trial_id IS NULL no longer matches).""" + monkeypatch.setattr( + _db_repo, + "get_trial", + AsyncMock(return_value=_trial()), + ) + _stub_execute_returns_row(mock_db, None) # 0 rows affected + + stamped = await stamp_baseline_trial(mock_db, "study-1", "trial-1", 0.612) + + assert stamped is False + + async def test_missing_trial_raises_baseline_trial_not_found( + self, mock_db: Any, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setattr( + _db_repo, + "get_trial", + AsyncMock(return_value=None), + ) + + with pytest.raises(BaselineTrialNotFound, match="not found"): + await stamp_baseline_trial(mock_db, "study-1", "trial-missing", 0.5) + + async def test_wrong_study_id_raises_invalid_state( + self, mock_db: Any, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setattr( + _db_repo, + "get_trial", + AsyncMock(return_value=_trial(study_id="other-study")), + ) + + with pytest.raises(InvalidBaselineTrialState, match="study_id"): + await stamp_baseline_trial(mock_db, "study-1", "trial-1", 0.5) + + async def test_non_baseline_trial_raises_invalid_state( + self, mock_db: Any, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setattr( + _db_repo, + "get_trial", + AsyncMock(return_value=_trial(is_baseline=False)), + ) + + with pytest.raises(InvalidBaselineTrialState, match="is_baseline=False"): + await stamp_baseline_trial(mock_db, "study-1", "trial-1", 0.5) + + async def test_non_complete_status_raises_invalid_state( + self, mock_db: Any, monkeypatch: pytest.MonkeyPatch + ) -> None: + for non_complete in ("failed", "pruned"): + monkeypatch.setattr( + _db_repo, + "get_trial", + AsyncMock(return_value=_trial(status=non_complete)), + ) + with pytest.raises(InvalidBaselineTrialState, match="status="): + await stamp_baseline_trial(mock_db, "study-1", "trial-1", 0.5) + + async def test_does_not_commit(self, mock_db: Any, monkeypatch: pytest.MonkeyPatch) -> None: + """Commit is left to the caller (spec FR-12).""" + monkeypatch.setattr( + _db_repo, + "get_trial", + AsyncMock(return_value=_trial()), + ) + _stub_execute_returns_row(mock_db, ("study-1",)) + + await stamp_baseline_trial(mock_db, "study-1", "trial-1", 0.5) + + assert mock_db.commit.await_count == 0 + + async def test_emits_stamped_log_on_success( + self, + mock_db: Any, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + monkeypatch.setattr( + _db_repo, + "get_trial", + AsyncMock(return_value=_trial()), + ) + _stub_execute_returns_row(mock_db, ("study-1",)) + + with structlog.testing.capture_logs() as logs: + await stamp_baseline_trial(mock_db, "study-1", "trial-1", 0.612) + + events = [log.get("event_type") for log in logs] + assert "baseline_stamped" in events, logs + + async def test_emits_no_op_log_when_already_stamped( + self, + mock_db: Any, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + monkeypatch.setattr( + _db_repo, + "get_trial", + AsyncMock(return_value=_trial()), + ) + _stub_execute_returns_row(mock_db, None) + + with structlog.testing.capture_logs() as logs: + await stamp_baseline_trial(mock_db, "study-1", "trial-1", 0.612) + + events = [log.get("event_type") for log in logs] + assert "baseline_stamp_no_op" in events, logs diff --git a/backend/tests/unit/test_workers.py b/backend/tests/unit/test_workers.py index eafb1f18..d74f14c6 100644 --- a/backend/tests/unit/test_workers.py +++ b/backend/tests/unit/test_workers.py @@ -60,6 +60,8 @@ def test_worker_settings_importable(_settings_env: None) -> None: "register_webhook", # feat_auto_followup_studies Story 2.1 "enqueue_followup_study", + # feat_study_baseline_trial Story 1.4 + "run_baseline_trial", } diff --git a/backend/tests/unit/workers/test_baseline_trial.py b/backend/tests/unit/workers/test_baseline_trial.py new file mode 100644 index 00000000..1a11b388 --- /dev/null +++ b/backend/tests/unit/workers/test_baseline_trial.py @@ -0,0 +1,329 @@ +"""Unit tests for :func:`backend.workers.baseline.run_baseline_trial`. + +The worker is heavy I/O — covers adapter + scorer + DB + studystate. +Tests mock every external dependency via ``monkeypatch`` per the existing +``test_run_trial.py`` convention. Integration coverage (real Postgres + +real Arq + real qrels load) lives in +``backend/tests/integration/test_orchestrator_baseline_trial.py``. + +Spec: feat_study_baseline_trial FR-10. AC-1 / AC-3 / AC-16 depend. +""" + +from __future__ import annotations + +import asyncio as _asyncio_module +from types import SimpleNamespace +from typing import Any +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from backend.app.db import repo as _db_repo +from backend.app.services import study_state as _study_state +from backend.workers import baseline as baseline_worker + + +def _study(**overrides: Any) -> Any: + base = { + "id": "study-1", + "cluster_id": "cluster-1", + "template_id": "template-1", + "query_set_id": "qs-1", + "judgment_list_id": "jl-1", + "target": "products", + "objective": {"metric": "ndcg", "k": 10, "direction": "maximize"}, + "config": {"trial_timeout_s": 30}, + "search_space": {"params": {"boost_title": {"type": "float", "low": 0.5, "high": 10.0}}}, + } + base.update(overrides) + return SimpleNamespace(**base) + + +def _cluster() -> Any: + return SimpleNamespace(id="cluster-1", engine_type="elasticsearch", base_url="http://es:9200") + + +def _template_row() -> Any: + return SimpleNamespace( + id="template-1", + name="my-template", + engine_type="elasticsearch", + body='{"query": {"match_all": {}}}', + declared_params={"boost_title": "float"}, + ) + + +def _query() -> Any: + return SimpleNamespace(id="q-1", query_text="red shoes") + + +@pytest.fixture +def patched_externals(monkeypatch: pytest.MonkeyPatch) -> dict[str, Any]: + """Stub all I/O dependencies of run_baseline_trial.""" + mock_db = AsyncMock() + # session_factory().__aenter__() returns mock_db. + factory = MagicMock() + factory.return_value.__aenter__ = AsyncMock(return_value=mock_db) + factory.return_value.__aexit__ = AsyncMock(return_value=None) + monkeypatch.setattr(baseline_worker, "get_session_factory", lambda: factory) + + monkeypatch.setattr(baseline_worker, "_existing_terminal_row", AsyncMock(return_value=None)) + + monkeypatch.setattr(_db_repo, "get_study", AsyncMock(return_value=_study())) + monkeypatch.setattr(_db_repo, "get_cluster", AsyncMock(return_value=_cluster())) + monkeypatch.setattr( + _db_repo, + "get_query_template", + AsyncMock(return_value=_template_row()), + ) + monkeypatch.setattr( + _db_repo, + "list_queries_for_set", + AsyncMock(return_value=[_query()]), + ) + monkeypatch.setattr(baseline_worker, "load_qrels", AsyncMock(return_value={"q-1": {}})) + + mock_adapter = AsyncMock() + mock_adapter.render = MagicMock( + return_value=SimpleNamespace(query_id="q-1", body={"query": {}}) + ) + mock_adapter.search_batch = AsyncMock( + return_value={"q-1": [SimpleNamespace(doc_id="d1", score=0.8)]} + ) + mock_adapter.aclose = AsyncMock() + monkeypatch.setattr(baseline_worker, "build_adapter", lambda _c: mock_adapter) + + monkeypatch.setattr( + baseline_worker, + "score", + lambda qrels, run, metrics: { + "aggregate": {"ndcg@10": 0.612, "map@10": 0.5, "mrr": 0.7}, + "per_query": {"q-1": {"ndcg@10": 0.612}}, + }, + ) + + mock_trial = SimpleNamespace( + id="trial-1", + study_id="study-1", + is_baseline=True, + status="complete", + primary_metric=0.612, + ) + monkeypatch.setattr(_db_repo, "create_trial", AsyncMock(return_value=mock_trial)) + + monkeypatch.setattr( + _study_state, + "stamp_baseline_trial", + AsyncMock(return_value=True), + ) + + return { + "db": mock_db, + "adapter": mock_adapter, + "create_trial": _db_repo.create_trial, + "stamp": _study_state.stamp_baseline_trial, + } + + +class TestRunBaselineTrialHappyPath: + async def test_happy_path_inserts_and_stamps(self, patched_externals: dict[str, Any]) -> None: + await baseline_worker.run_baseline_trial( + ctx={}, study_id="study-1", trial_id="trial-1", params={"boost_title": 5.0} + ) + + # Trial INSERTed. + patched_externals["create_trial"].assert_awaited_once() + kwargs = patched_externals["create_trial"].await_args.kwargs + assert kwargs["id"] == "trial-1" + assert kwargs["study_id"] == "study-1" + assert kwargs["is_baseline"] is True + assert kwargs["optuna_trial_number"] == -1 + assert kwargs["status"] == "complete" + assert kwargs["primary_metric"] == 0.612 + + # Stamping helper called. + patched_externals["stamp"].assert_awaited_once_with( + patched_externals["db"], "study-1", "trial-1", 0.612 + ) + + async def test_uses_default_secondary_metrics_when_config_absent( + self, + patched_externals: dict[str, Any], + monkeypatch: pytest.MonkeyPatch, + ) -> None: + # Override study to remove secondary_metrics config. + study = _study(config={"trial_timeout_s": 30}) # no secondary_metrics + monkeypatch.setattr(_db_repo, "get_study", AsyncMock(return_value=study)) + + captured: dict[str, Any] = {} + + def _capture_score(qrels: Any, run: Any, metrics: set[str]) -> Any: + captured["metrics"] = metrics + return { + "aggregate": {"ndcg@10": 0.5, "map@10": 0.4, "mrr": 0.6}, + "per_query": {"q-1": {"ndcg@10": 0.5}}, + } + + monkeypatch.setattr(baseline_worker, "score", _capture_score) + + await baseline_worker.run_baseline_trial( + ctx={}, study_id="study-1", trial_id="trial-1", params={} + ) + + # ndcg@10 (primary) + the default secondary set. + assert captured["metrics"] == {"ndcg@10", "map@10", "mrr"} + + +class TestRunBaselineTrialIdempotency: + async def test_idempotent_when_terminal_row_already_exists( + self, + patched_externals: dict[str, Any], + monkeypatch: pytest.MonkeyPatch, + ) -> None: + existing = SimpleNamespace( + id="trial-1", + status="complete", + is_baseline=True, + primary_metric=0.612, + ) + monkeypatch.setattr( + baseline_worker, "_existing_terminal_row", AsyncMock(return_value=existing) + ) + + await baseline_worker.run_baseline_trial( + ctx={}, study_id="study-1", trial_id="trial-1", params={} + ) + + # No new INSERT — but the stamp helper IS called (idempotent re-stamp). + patched_externals["create_trial"].assert_not_awaited() + patched_externals["stamp"].assert_awaited_once() + + async def test_idempotent_complete_existing_with_failed_status_no_stamp( + self, + patched_externals: dict[str, Any], + monkeypatch: pytest.MonkeyPatch, + ) -> None: + existing = SimpleNamespace( + id="trial-1", + status="failed", + is_baseline=True, + primary_metric=None, + ) + monkeypatch.setattr( + baseline_worker, "_existing_terminal_row", AsyncMock(return_value=existing) + ) + + await baseline_worker.run_baseline_trial( + ctx={}, study_id="study-1", trial_id="trial-1", params={} + ) + + patched_externals["create_trial"].assert_not_awaited() + # Failed baseline doesn't stamp. + patched_externals["stamp"].assert_not_awaited() + + +class TestRunBaselineTrialFailures: + async def test_adapter_search_raises_persists_failed_row( + self, + patched_externals: dict[str, Any], + monkeypatch: pytest.MonkeyPatch, + ) -> None: + # Adapter raises mid-search. + patched_externals["adapter"].search_batch = AsyncMock( + side_effect=RuntimeError("cluster unreachable") + ) + + # create_trial is called twice: first attempt (complete) fails because + # adapter raised, then again from the failure-handler path. Reset to + # a single AsyncMock that always returns a Trial. + await baseline_worker.run_baseline_trial( + ctx={}, study_id="study-1", trial_id="trial-1", params={} + ) + + # Last create_trial call should be the failed-row INSERT. + assert patched_externals["create_trial"].await_count == 1 + kwargs = patched_externals["create_trial"].await_args.kwargs + assert kwargs["status"] == "failed" + assert kwargs["primary_metric"] is None + assert "cluster unreachable" in kwargs["error"] + # Failed baseline does NOT stamp. + patched_externals["stamp"].assert_not_awaited() + + async def test_scorer_raises_persists_failed_row( + self, + patched_externals: dict[str, Any], + monkeypatch: pytest.MonkeyPatch, + ) -> None: + def _raise(*a: Any, **kw: Any) -> Any: + raise ValueError("metric not supported") + + monkeypatch.setattr(baseline_worker, "score", _raise) + + await baseline_worker.run_baseline_trial( + ctx={}, study_id="study-1", trial_id="trial-1", params={} + ) + + assert patched_externals["create_trial"].await_count == 1 + kwargs = patched_externals["create_trial"].await_args.kwargs + assert kwargs["status"] == "failed" + assert "metric not supported" in kwargs["error"] + patched_externals["stamp"].assert_not_awaited() + + async def test_operational_error_reraises_for_arq_retry( + self, + patched_externals: dict[str, Any], + monkeypatch: pytest.MonkeyPatch, + ) -> None: + from sqlalchemy.exc import OperationalError + + # Force the search to raise an OperationalError; the worker MUST + # re-raise rather than swallowing it (Arq retries). + patched_externals["adapter"].search_batch = AsyncMock( + side_effect=OperationalError("DB unreachable", None, Exception()) + ) + + with pytest.raises(OperationalError): + await baseline_worker.run_baseline_trial( + ctx={}, study_id="study-1", trial_id="trial-1", params={} + ) + + +class TestFaultSeam: + async def test_fault_delay_seam_triggers_asyncio_sleep( + self, + patched_externals: dict[str, Any], + monkeypatch: pytest.MonkeyPatch, + ) -> None: + sleep_args: list[float] = [] + + async def _fake_sleep(s: float) -> None: + sleep_args.append(s) + + monkeypatch.setattr(_asyncio_module, "sleep", _fake_sleep) + monkeypatch.setenv("FEAT_STUDY_BASELINE_TRIAL_FAULT", "delay_before_score") + monkeypatch.setenv("FEAT_STUDY_BASELINE_TRIAL_FAULT_DELAY_S", "0.3") + + await baseline_worker.run_baseline_trial( + ctx={}, study_id="study-1", trial_id="trial-1", params={} + ) + + assert sleep_args == [0.3] + + async def test_no_delay_when_env_var_unset( + self, + patched_externals: dict[str, Any], + monkeypatch: pytest.MonkeyPatch, + ) -> None: + sleep_called: list[float] = [] + + async def _fake_sleep(s: float) -> None: + sleep_called.append(s) + + monkeypatch.setattr(_asyncio_module, "sleep", _fake_sleep) + monkeypatch.delenv("FEAT_STUDY_BASELINE_TRIAL_FAULT", raising=False) + + await baseline_worker.run_baseline_trial( + ctx={}, study_id="study-1", trial_id="trial-1", params={} + ) + + assert sleep_called == [] diff --git a/backend/tests/unit/workers/test_orchestrator_baseline_phase.py b/backend/tests/unit/workers/test_orchestrator_baseline_phase.py new file mode 100644 index 00000000..6c5af931 --- /dev/null +++ b/backend/tests/unit/workers/test_orchestrator_baseline_phase.py @@ -0,0 +1,201 @@ +"""Unit tests for the FR-2 baseline phase helpers in ``backend.workers.orchestrator``. + +Covers (real-backend integration coverage lives in +``backend/tests/integration/test_orchestrator_baseline_trial.py``): + +- :class:`BaselineEnqueueResult` discriminated union (plan-cycle-2 F2 + regression guard). +- :func:`_compute_baseline_wait_s` formula (FR-2 step 5). +- :func:`_resolve_and_enqueue_baseline` — skipped / enqueued / deduped + branches. +- :func:`_run_baseline_phase` resume-path stamping of an existing complete + baseline. +""" + +from __future__ import annotations + +from types import SimpleNamespace +from typing import Any +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from backend.app.db import repo as _db_repo +from backend.app.services import study_state as _study_state +from backend.workers import orchestrator as orch +from backend.workers.orchestrator import ( + BaselineEnqueueResult, + _compute_baseline_wait_s, +) + + +def _study(**overrides: Any) -> Any: + base = { + "id": "study-1", + "config": {"trial_timeout_s": 30}, + "baseline_trial_id": None, + "parent_proposal_id": None, + "parent_study_id": None, + "search_space": {"params": {"x": {"type": "float", "low": 0.0, "high": 1.0}}}, + } + base.update(overrides) + return SimpleNamespace(**base) + + +class TestBaselineEnqueueResult: + def test_skipped_kind_has_no_trial_id(self) -> None: + result = BaselineEnqueueResult(kind="skipped") + assert result.kind == "skipped" + assert result.trial_id is None + + def test_enqueued_kind_carries_trial_id(self) -> None: + result = BaselineEnqueueResult(kind="enqueued", trial_id="t-1") + assert result.kind == "enqueued" + assert result.trial_id == "t-1" + + def test_deduped_kind_has_no_trial_id(self) -> None: + result = BaselineEnqueueResult(kind="deduped") + assert result.kind == "deduped" + assert result.trial_id is None + + +class TestComputeBaselineWaitS: + def test_short_timeout_floors_at_60(self) -> None: + study = _study(config={"trial_timeout_s": 5}) + # 5 + 30 = 35, floor at 60. + assert _compute_baseline_wait_s(study) == 60.0 + + def test_typical_timeout_returns_plus_30(self) -> None: + study = _study(config={"trial_timeout_s": 60}) + # max(60, 60+30) = 90, min(600, 90) = 90. + assert _compute_baseline_wait_s(study) == 90.0 + + def test_long_timeout_caps_at_600(self) -> None: + study = _study(config={"trial_timeout_s": 1200}) + # max(60, 1200+30) = 1230, min(600, 1230) = 600. + assert _compute_baseline_wait_s(study) == 600.0 + + def test_missing_trial_timeout_uses_settings_default( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + fake_settings = SimpleNamespace(studies_default_timeout_s=45) + monkeypatch.setattr(orch, "get_settings", lambda: fake_settings) + study = _study(config={}) + # max(60, 45+30) = 75. + assert _compute_baseline_wait_s(study) == 75.0 + + +class TestResolveAndEnqueueBaseline: + async def test_skipped_when_resolver_returns_none( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setattr(orch, "resolve_baseline_params", AsyncMock(return_value=None)) + arq_pool = AsyncMock() + result = await orch._resolve_and_enqueue_baseline(AsyncMock(), arq_pool, _study()) + assert result.kind == "skipped" + arq_pool.enqueue_job.assert_not_awaited() + + async def test_enqueued_when_resolver_returns_params( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setattr(orch, "resolve_baseline_params", AsyncMock(return_value={"x": 0.5})) + arq_pool = AsyncMock() + arq_pool.enqueue_job = AsyncMock(return_value=MagicMock()) # non-None = accepted + + result = await orch._resolve_and_enqueue_baseline(AsyncMock(), arq_pool, _study()) + + assert result.kind == "enqueued" + assert result.trial_id is not None and len(result.trial_id) == 36 + kwargs = arq_pool.enqueue_job.await_args.kwargs + assert kwargs == {"_job_id": "baseline:study-1"} + # Positional args: function_name, study_id, trial_id, params. + args = arq_pool.enqueue_job.await_args.args + assert args[0] == "run_baseline_trial" + assert args[1] == "study-1" + assert args[3] == {"x": 0.5} + + async def test_deduped_when_arq_returns_none(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(orch, "resolve_baseline_params", AsyncMock(return_value={"x": 0.5})) + arq_pool = AsyncMock() + arq_pool.enqueue_job = AsyncMock(return_value=None) # duplicate rejected + + result = await orch._resolve_and_enqueue_baseline(AsyncMock(), arq_pool, _study()) + + assert result.kind == "deduped" + assert result.trial_id is None + + +class TestRunBaselinePhaseResumePath: + async def test_skips_when_already_stamped(self, monkeypatch: pytest.MonkeyPatch) -> None: + """Resume path: if baseline_trial_id is already set, do nothing.""" + study_with_stamp = _study(baseline_trial_id="trial-1") + db_session = AsyncMock() + factory = MagicMock() + factory.return_value.__aenter__ = AsyncMock(return_value=db_session) + factory.return_value.__aexit__ = AsyncMock(return_value=None) + + monkeypatch.setattr(_db_repo, "get_study", AsyncMock(return_value=study_with_stamp)) + # Spy on the resolver — it should NOT be called. + spy_resolver = AsyncMock() + monkeypatch.setattr(orch, "resolve_baseline_params", spy_resolver) + arq_pool = AsyncMock() + + await orch._run_baseline_phase(factory, arq_pool, "study-1") + + spy_resolver.assert_not_awaited() + arq_pool.enqueue_job.assert_not_awaited() + + async def test_stamps_when_complete_baseline_exists_unstamped( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Resume path: complete baseline row exists but study unstamped — stamp it.""" + study = _study(baseline_trial_id=None) + existing_baseline = SimpleNamespace( + id="trial-1", + status="complete", + primary_metric=0.612, + ) + db_session = AsyncMock() + factory = MagicMock() + factory.return_value.__aenter__ = AsyncMock(return_value=db_session) + factory.return_value.__aexit__ = AsyncMock(return_value=None) + + monkeypatch.setattr(_db_repo, "get_study", AsyncMock(return_value=study)) + monkeypatch.setattr( + orch, "_find_terminal_baseline_row", AsyncMock(return_value=existing_baseline) + ) + stamp_mock = AsyncMock(return_value=True) + monkeypatch.setattr(_study_state, "stamp_baseline_trial", stamp_mock) + + # Spy: the resolver should NOT be called (resume short-circuits). + spy_resolver = AsyncMock() + monkeypatch.setattr(orch, "resolve_baseline_params", spy_resolver) + arq_pool = AsyncMock() + + await orch._run_baseline_phase(factory, arq_pool, "study-1") + + stamp_mock.assert_awaited_once_with(db_session, "study-1", "trial-1", 0.612) + spy_resolver.assert_not_awaited() + arq_pool.enqueue_job.assert_not_awaited() + + async def test_skips_when_failed_baseline_exists(self, monkeypatch: pytest.MonkeyPatch) -> None: + """Resume path: only failed baseline row exists — DO NOT retry.""" + study = _study(baseline_trial_id=None) + failed_baseline = SimpleNamespace(id="trial-1", status="failed", primary_metric=None) + db_session = AsyncMock() + factory = MagicMock() + factory.return_value.__aenter__ = AsyncMock(return_value=db_session) + factory.return_value.__aexit__ = AsyncMock(return_value=None) + + monkeypatch.setattr(_db_repo, "get_study", AsyncMock(return_value=study)) + monkeypatch.setattr( + orch, "_find_terminal_baseline_row", AsyncMock(return_value=failed_baseline) + ) + spy_resolver = AsyncMock() + monkeypatch.setattr(orch, "resolve_baseline_params", spy_resolver) + arq_pool = AsyncMock() + + await orch._run_baseline_phase(factory, arq_pool, "study-1") + + spy_resolver.assert_not_awaited() + arq_pool.enqueue_job.assert_not_awaited() diff --git a/backend/workers/all.py b/backend/workers/all.py index 9a62d5a7..33ae29a2 100644 --- a/backend/workers/all.py +++ b/backend/workers/all.py @@ -60,6 +60,7 @@ from backend.app.db.session import get_session_factory from backend.app.eval.optuna_runtime import build_storage from backend.workers.auto_followup import enqueue_followup_study +from backend.workers.baseline import run_baseline_trial from backend.workers.digest import generate_digest from backend.workers.git_pr import open_pr from backend.workers.judgments import generate_judgments_llm @@ -209,6 +210,7 @@ class WorkerSettings: functions: list[Any] = [ run_trial, + run_baseline_trial, # feat_study_baseline_trial Story 1.4 func(start_study, timeout=_ORCHESTRATOR_JOB_TIMEOUT_S), func(resume_study, timeout=_ORCHESTRATOR_JOB_TIMEOUT_S), generate_digest, diff --git a/backend/workers/baseline.py b/backend/workers/baseline.py new file mode 100644 index 00000000..640eecc1 --- /dev/null +++ b/backend/workers/baseline.py @@ -0,0 +1,345 @@ +"""``run_baseline_trial`` Arq job (feat_study_baseline_trial Story 1.4 / FR-10). + +One-shot non-Optuna trial executed before the Optuna polling loop starts. +Mirrors :mod:`backend.workers.trials.run_trial` for the render → search → +score → persist pipeline, but does NOT touch Optuna's RDB: + +* No ``study.ask()`` / ``study.tell()`` — the baseline isn't an Optuna + trial. The orchestrator pre-generates a UUIDv7 ``trial_id`` and + passes it as a job argument; the worker writes a ``trials`` row with + ``is_baseline=TRUE`` and ``optuna_trial_number=-1`` (NOT-NULL sentinel + filler — the canonical discriminator is ``is_baseline``). + +* Idempotency by ``trial_id`` (NOT ``(study_id, optuna_trial_number)``): + if a terminal row with ``id = trial_id`` already exists, no-op and + return. This handles Arq retries cleanly because the orchestrator + passes the same UUID on every retry. + +* On ``status='complete'``: self-stamps ``studies.baseline_trial_id`` + + ``baseline_metric`` via :func:`backend.app.services.study_state. + stamp_baseline_trial` (FR-12 chokepoint), then commits. This is the + durable stamping path; the orchestrator's fast-path stamp in + ``start_study`` is just an accelerator (per D-13). + +* On failure (adapter raises, scorer raises, render raises): persist + the failed ``Trial`` row with ``is_baseline=TRUE, status='failed', + error=``; commit; return normally (Arq treats as success). + Failed baselines do NOT fail the study — the orchestrator falls back + to runner-up comparison and first-decile-extremum auto-followup gate. + +* Test-only fault seam (plan F9): when + ``FEAT_STUDY_BASELINE_TRIAL_FAULT=delay_before_score`` is set, the + worker sleeps for ``FEAT_STUDY_BASELINE_TRIAL_FAULT_DELAY_S`` seconds + before scoring. Used by ``test_baseline_late_completion_stamp.py`` to + force the orchestrator's wait phase to time out so the worker's + self-stamp is the only path that lands the FK (covers AC-16). + +Spec: ``feat_study_baseline_trial/feature_spec.md`` FR-10. AC-1 / AC-3 / +AC-16 depend on this contract. +""" + +from __future__ import annotations + +import asyncio +import os +from datetime import UTC, datetime +from typing import Any, cast + +import structlog +from sqlalchemy import select +from sqlalchemy.exc import IntegrityError +from sqlalchemy.exc import OperationalError as SAOperationalError +from sqlalchemy.ext.asyncio import AsyncSession + +from backend.app.adapters.protocol import NativeQuery, QueryTemplate +from backend.app.core.settings import get_settings +from backend.app.db import repo +from backend.app.db.models import Trial +from backend.app.db.session import get_session_factory +from backend.app.eval.qrels_loader import load_qrels +from backend.app.eval.scoring import Qrels, Run, objective_metric_key, score +from backend.app.services import study_state +from backend.app.services.cluster import build_adapter + +logger = structlog.get_logger(__name__) + +# Mirrors ``backend.workers.trials._DEFAULT_SECONDARY_METRICS`` — the same +# inventory ensures baseline + Optuna trials have comparable metric surfaces. +_DEFAULT_SECONDARY_METRICS: frozenset[str] = frozenset({"ndcg@10", "map@10", "mrr"}) + +_BASELINE_OPTUNA_TRIAL_NUMBER: int = -1 +"""Sentinel filler for the NOT-NULL ``trials.optuna_trial_number`` column. +The canonical baseline discriminator is ``is_baseline=TRUE`` — Optuna +never queries this row (it uses its own RDB).""" + + +async def _existing_terminal_row(db: AsyncSession, trial_id: str) -> Trial | None: + """Look up an existing terminal ``trials`` row by ``trial_id``. + + FR-10 idempotency: if a row with the orchestrator-generated ``trial_id`` + already exists at terminal status, return it so the worker can no-op + on retry without duplicating the INSERT. + """ + stmt = ( + select(Trial) + .where(Trial.id == trial_id) + .where(Trial.status.in_(("complete", "failed", "pruned"))) + .limit(1) + ) + return (await db.execute(stmt)).scalar_one_or_none() + + +async def run_baseline_trial( + ctx: dict[str, Any], + study_id: str, + trial_id: str, + params: dict[str, Any], +) -> None: + """Execute one non-Optuna baseline trial end-to-end. See module docstring.""" + structlog.contextvars.bind_contextvars( + study_id=study_id, + trial_id=trial_id, + is_baseline=True, + ) + started_at: datetime | None = None + adapter = None + + session_factory = get_session_factory() + async with session_factory() as db: + try: + # A. Idempotency check (FR-10): if this trial_id already terminal, + # no-op. Arq retries land here on repeat. + existing = await _existing_terminal_row(db, trial_id) + if existing is not None: + logger.info( + "baseline trial already terminal — no-op", + event_type="baseline_already_terminal", + existing_status=existing.status, + ) + # If complete but unstamped, attempt the FR-12 stamp (idempotent). + if existing.status == "complete" and existing.primary_metric is not None: + try: + await study_state.stamp_baseline_trial( + db, study_id, trial_id, float(existing.primary_metric) + ) + await db.commit() + except ( + study_state.BaselineTrialNotFound, + study_state.InvalidBaselineTrialState, + ): + await db.rollback() + return + + # B. Load study, cluster, template, queries, qrels. + study_row = await repo.get_study(db, study_id) + if study_row is None: + logger.warning( + "study deleted before baseline trial executed", + event_type="baseline_study_missing", + ) + return + + cluster = await repo.get_cluster(db, study_row.cluster_id) + if cluster is None: + raise RuntimeError( + f"cluster {study_row.cluster_id!r} not found " + f"for baseline trial in study {study_id}" + ) + adapter = build_adapter(cluster) + + template_row = await repo.get_query_template(db, study_row.template_id) + if template_row is None: + raise RuntimeError( + f"template {study_row.template_id!r} not found " + f"for baseline trial in study {study_id}" + ) + template = QueryTemplate( + name=template_row.name, + engine_type=cast(Any, template_row.engine_type), + body=template_row.body, + declared_params=cast(dict[str, str], template_row.declared_params), + ) + queries = await repo.list_queries_for_set(db, study_row.query_set_id) + qrels: Qrels = await load_qrels(db, study_row.judgment_list_id) + + # C. Resolve retrieval depth + metric set (mirror run_trial). + objective = study_row.objective + objective_key = objective_metric_key(objective) + top_k_raw = objective.get("k") + top_k = top_k_raw if isinstance(top_k_raw, int) else 100 + + metrics_set: set[str] = {objective_key} + if "secondary_metrics" in study_row.config: + secondaries = study_row.config["secondary_metrics"] + if isinstance(secondaries, list): + metrics_set.update(str(m) for m in secondaries) + else: + metrics_set.update(_DEFAULT_SECONDARY_METRICS) + + # D. Resolve per-trial timeout (same precedence as run_trial). + configured_timeout = study_row.config.get("trial_timeout_s") + trial_timeout_s = float( + configured_timeout + if configured_timeout is not None + else get_settings().studies_default_timeout_s + ) + + # E. Render queries. + started_at = datetime.now(UTC) + native_queries: list[NativeQuery] = [ + adapter.render(template, params, q.query_text) for q in queries + ] + native_queries = [ + NativeQuery(query_id=str(q.id), body=nq.body) + for q, nq in zip(queries, native_queries, strict=True) + ] + + # F. Test-only fault seam (plan F9): force a delay before score + # so test_baseline_late_completion_stamp.py can exercise the + # orchestrator's wait-timeout + worker self-stamp path. + fault = os.environ.get("FEAT_STUDY_BASELINE_TRIAL_FAULT") + if fault == "delay_before_score": + delay_s = float(os.environ.get("FEAT_STUDY_BASELINE_TRIAL_FAULT_DELAY_S", "5")) + logger.info( + "baseline fault seam active — delaying before score", + event_type="baseline_fault_delay", + delay_s=delay_s, + ) + await asyncio.sleep(delay_s) + + # G. Execute search via the adapter. + hits = await adapter.search_batch( + target=study_row.target, + queries=native_queries, + top_k=top_k, + strict_errors=False, + timeout=trial_timeout_s, + ) + + # H. Score. + run_dict: Run = { + qid: {hit.doc_id: float(hit.score) for hit in hit_list} + for qid, hit_list in hits.items() + } + scored = score(qrels, run_dict, metrics_set) + primary_metric = float(scored["aggregate"][objective_key]) + duration_ms = int(round((datetime.now(UTC) - started_at).total_seconds() * 1000)) + + # I. INSERT the Trial row. Catch IntegrityError from the + # partial unique index in case a sibling worker (e.g., Arq + # dedupe bypass under exotic race) inserted first. + try: + await repo.create_trial( + db, + id=trial_id, + study_id=study_id, + optuna_trial_number=_BASELINE_OPTUNA_TRIAL_NUMBER, + params=params, + primary_metric=primary_metric, + metrics=scored["aggregate"], + per_query_metrics=scored["per_query"], + duration_ms=duration_ms, + status="complete", + error=None, + started_at=started_at, + ended_at=datetime.now(UTC), + is_baseline=True, + ) + await db.commit() + except IntegrityError as exc: + # Partial unique index uq_trials_study_baseline_complete + # rejected this INSERT — a sibling worker already wrote a + # complete baseline for this study. The FR-12 stamp will + # have already landed via that sibling; this worker exits + # cleanly. (Defense-in-depth — Arq _job_id dedupe in FR-2 + # should normally prevent this entirely.) + await db.rollback() + logger.warning( + "baseline INSERT lost partial-unique race", + event_type="baseline_insert_race", + error=str(exc)[:200], + ) + return + + # J. Self-stamp the studies row (FR-10 step 7). + try: + await study_state.stamp_baseline_trial(db, study_id, trial_id, primary_metric) + await db.commit() + except ( + study_state.BaselineTrialNotFound, + study_state.InvalidBaselineTrialState, + ) as exc: + # The trial row exists (we just inserted it), so missing / + # invalid-state would be a real bug. Log + rollback. + await db.rollback() + logger.exception( + "baseline self-stamp failed — caller bug", + event_type="baseline_self_stamp_error", + error=str(exc)[:200], + ) + + logger.info( + "baseline trial completed", + event_type="baseline_trial_completed", + status="complete", + primary_metric=primary_metric, + duration_ms=duration_ms, + ) + + except SAOperationalError: + # Infra-level failure (DB unreachable) — re-raise for Arq retry. + await db.rollback() + raise + + except Exception as exc: + await db.rollback() + ended_at = datetime.now(UTC) + failed_duration_ms: int | None + if started_at is not None: + failed_duration_ms = int(round((ended_at - started_at).total_seconds() * 1000)) + else: + failed_duration_ms = None + error_text = str(exc)[:500] + + try: + await repo.create_trial( + db, + id=trial_id, + study_id=study_id, + optuna_trial_number=_BASELINE_OPTUNA_TRIAL_NUMBER, + params=params, + primary_metric=None, + metrics={}, + duration_ms=failed_duration_ms, + status="failed", + error=error_text, + started_at=started_at, + ended_at=ended_at, + is_baseline=True, + ) + await db.commit() + except IntegrityError: + await db.rollback() + # The trial_id is unique by orchestrator-generation; this + # branch shouldn't fire. Log + return. + logger.exception( + "baseline failed-row INSERT also raised IntegrityError", + event_type="baseline_failed_insert_race", + ) + return + + logger.warning( + "baseline trial failed", + event_type="baseline_trial_failed", + status="failed", + error=error_text, + duration_ms=failed_duration_ms, + ) + + finally: + if adapter is not None: + await adapter.aclose() + structlog.contextvars.unbind_contextvars("study_id", "trial_id", "is_baseline") + + +__all__ = ["run_baseline_trial"] diff --git a/backend/workers/digest.py b/backend/workers/digest.py index d9a23755..3c97b6e0 100644 --- a/backend/workers/digest.py +++ b/backend/workers/digest.py @@ -842,9 +842,13 @@ async def generate_digest(ctx: dict[str, Any], study_id: str) -> None: ) await _persist_zero_trials_digest(db, study) return + # FR-11: exclude baseline from the top-10 list — the digest + # surfaces Optuna's exploration; baseline is reported + # separately via study.baseline_metric. top_stmt = ( select(Trial) .where(Trial.study_id == study_id) + .where(Trial.is_baseline.is_(False)) .where(Trial.status == "complete") .order_by(Trial.primary_metric.desc()) .limit(TOP_K_TRIALS) diff --git a/backend/workers/orchestrator.py b/backend/workers/orchestrator.py index 96e2af69..aba30c52 100644 --- a/backend/workers/orchestrator.py +++ b/backend/workers/orchestrator.py @@ -35,21 +35,23 @@ import hashlib from collections.abc import AsyncIterator from contextlib import asynccontextmanager +from dataclasses import dataclass from datetime import UTC, datetime, timedelta -from typing import Any +from typing import Any, Literal import optuna import structlog import uuid_utils from arq.connections import ArqRedis, RedisSettings, create_pool from sqlalchemy import select, text -from sqlalchemy.ext.asyncio import AsyncSession +from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker from backend.app.core.settings import get_settings from backend.app.db import repo -from backend.app.db.models import Trial +from backend.app.db.models import Study, Trial from backend.app.db.repo.trial import TrialsSummary, aggregate_trials_summary from backend.app.db.session import get_session_factory +from backend.app.domain.study.baseline_resolver import resolve_baseline_params from backend.app.domain.study.search_space import SearchSpace, apply_search_space from backend.app.eval.optuna_runtime import build_pruner, build_sampler, get_or_create_study from backend.app.services import study_state @@ -63,6 +65,18 @@ _REPLENISH_TICK_S = 1.0 """Spec §19 decision log: orchestrator polls every 1s.""" +_BASELINE_WAIT_CEILING_S = 600.0 +"""feat_study_baseline_trial FR-2 step 5: maximum orchestrator wait for the +baseline trial (10 minutes — long enough for slow engines, short enough +that operators notice). For trial_timeout_s > 570s, wait deliberately +gives up before the worker; FR-10 self-stamp covers late completions.""" + +_BASELINE_WAIT_FLOOR_S = 60.0 +"""Minimum baseline wait — floor for studies with very short trial timeouts.""" + +_BASELINE_WAIT_MARGIN_S = 30.0 +"""Slack above the worker's per-trial timeout for queue + DB latency.""" + _DRAIN_TIMEOUT_S = 30.0 """Spec FR-4 cancel path: wait up to 30s for in-flight trials to terminate.""" @@ -169,6 +183,12 @@ async def start_study(ctx: dict[str, Any], study_id: str) -> None: # the JSON won't change on retry). space = SearchSpace.model_validate(study.search_space) + # C'. Baseline phase (feat_study_baseline_trial FR-2). Runs ONCE between + # search-space parse and the Optuna polling loop. Skipped silently when + # the study already has baseline_trial_id stamped (resume path) or when + # the resolver returns None (no params to run). + await _run_baseline_phase(session_factory, arq_pool, study_id) + # D. Polling loop — fresh session per tick (C3-F2 cycle-3 fix). settings = get_settings() parallelism: int = study.config.get("parallelism", settings.studies_default_parallelism) @@ -312,6 +332,274 @@ async def resume_study(ctx: dict[str, Any], study_id: str) -> None: await start_study(ctx, study_id) +# --------------------------------------------------------------------------- +# Baseline phase (feat_study_baseline_trial FR-2 + FR-3 + FR-12) +# --------------------------------------------------------------------------- + + +@dataclass(frozen=True) +class BaselineEnqueueResult: + """Discriminated result from :func:`_resolve_and_enqueue_baseline`. + + feat_study_baseline_trial plan-cycle-2 F2: the orchestrator must + distinguish three terminal cases that drive different wait strategies. + + * ``"skipped"`` — resolver returned None (no params) OR study already + has a complete baseline stamped (resume path). Proceed straight to + Optuna phase. + * ``"enqueued"`` — fresh job accepted by Arq. Wait by ``trial_id``. + * ``"deduped"`` — Arq rejected as duplicate (an earlier orchestrator + invocation already enqueued for this study). Wait by ``study_id`` + since the original ``trial_id`` is unknown to this invocation. + """ + + kind: Literal["skipped", "enqueued", "deduped"] + trial_id: str | None = None # set when kind == "enqueued" + + +async def _run_baseline_phase( + session_factory: async_sessionmaker[AsyncSession], + arq_pool: ArqRedis, + study_id: str, +) -> None: + """Run the FR-2 baseline phase: resolve → enqueue → wait → stamp. + + Single entry point called from ``start_study``. Handles all three + BaselineEnqueueResult kinds + the resume path (where a complete + baseline row already exists). NEVER raises — failures are logged + and the orchestrator proceeds to the Optuna phase. + """ + # 1. Resume re-stamp: if a complete baseline already exists but + # baseline_trial_id is NULL (worker crashed mid-stamp), stamp now. + async with session_factory() as db: + study = await repo.get_study(db, study_id) + if study is None: + return + if study.baseline_trial_id is not None: + # Already stamped — nothing to do. + return + existing = await _find_terminal_baseline_row(db, study_id) + if existing is not None and existing.status == "complete": + try: + await study_state.stamp_baseline_trial( + db, study_id, existing.id, float(existing.primary_metric or 0.0) + ) + await db.commit() + except ( + study_state.BaselineTrialNotFound, + study_state.InvalidBaselineTrialState, + ): + await db.rollback() + return + if existing is not None and existing.status == "failed": + # Failed baseline (from a prior run) — do NOT retry; proceed. + logger.info( + "baseline phase skipped — prior baseline failed", + event_type="baseline_skipped", + study_id=study_id, + reason="prior_baseline_failed", + ) + return + + # 2. Resolve params + enqueue. + async with session_factory() as db: + study = await repo.get_study(db, study_id) + if study is None: + return + result = await _resolve_and_enqueue_baseline(db, arq_pool, study) + + # 3. Dispatch. + if result.kind == "skipped": + return + + wait_s = _compute_baseline_wait_s(study) + + trial: Trial | None + if result.kind == "enqueued": + if result.trial_id is None: + raise RuntimeError("BaselineEnqueueResult(kind='enqueued') must carry a trial_id") + trial = await _wait_for_baseline_trial_by_id( + session_factory, study_id, result.trial_id, wait_s + ) + else: # deduped + trial = await _wait_for_baseline_trial_by_study(session_factory, study_id, wait_s) + + # 4. Stamp (or log + proceed). + if trial is None: + logger.warning( + "baseline wait timeout — proceeding to Optuna; worker will self-stamp", + event_type="baseline_wait_timeout", + study_id=study_id, + wait_s=wait_s, + ) + return + if trial.status == "complete": + async with session_factory() as db: + try: + await study_state.stamp_baseline_trial( + db, study_id, trial.id, float(trial.primary_metric or 0.0) + ) + await db.commit() + except ( + study_state.BaselineTrialNotFound, + study_state.InvalidBaselineTrialState, + ) as exc: + await db.rollback() + logger.warning( + "baseline stamp from orchestrator failed — worker self-stamp should cover", + event_type="baseline_orchestrator_stamp_error", + study_id=study_id, + error=str(exc)[:200], + ) + else: # failed + logger.warning( + "baseline trial failed — proceeding to Optuna without baseline", + event_type="baseline_failed", + study_id=study_id, + error=(trial.error or "")[:200], + ) + + +def _compute_baseline_wait_s(study: Study) -> float: + """FR-2 step 5: ``min(600, max(60, trial_timeout_s + 30))``.""" + settings = get_settings() + trial_timeout_s = study.config.get("trial_timeout_s") or settings.studies_default_timeout_s + return min( + _BASELINE_WAIT_CEILING_S, + max(_BASELINE_WAIT_FLOOR_S, float(trial_timeout_s) + _BASELINE_WAIT_MARGIN_S), + ) + + +async def _find_terminal_baseline_row(db: AsyncSession, study_id: str) -> Trial | None: + """Return any terminal is_baseline=TRUE row for this study, or None.""" + stmt = ( + select(Trial) + .where(Trial.study_id == study_id) + .where(Trial.is_baseline.is_(True)) + .where(Trial.status.in_(("complete", "failed", "pruned"))) + .limit(1) + ) + return (await db.execute(stmt)).scalar_one_or_none() + + +async def _resolve_and_enqueue_baseline( + db: AsyncSession, + arq_pool: ArqRedis, + study: Study, +) -> BaselineEnqueueResult: + """Resolve baseline params (FR-3) and enqueue the Arq job (FR-2 step 3). + + Uses ``_job_id=f"baseline:{study.id}"`` for Arq deduplication so a + resume invocation cannot enqueue a duplicate baseline job (defense + layer 1 of the 3-layer resume-race guard per D-16). + """ + params = await resolve_baseline_params(db, study) + if params is None: + logger.info( + "baseline phase skipped — resolver returned no params", + event_type="baseline_skipped", + study_id=study.id, + reason="resolver_returned_none", + ) + return BaselineEnqueueResult(kind="skipped") + + trial_id = str(uuid_utils.uuid7()) + job_id = f"baseline:{study.id}" + job = await arq_pool.enqueue_job( + "run_baseline_trial", + study.id, + trial_id, + params, + _job_id=job_id, + ) + if job is None: + # Arq rejected as duplicate — original job still queued/running. + logger.info( + "baseline enqueue deduped — original job still in flight", + event_type="baseline_enqueue_deduped", + study_id=study.id, + attempted_trial_id=trial_id, + ) + return BaselineEnqueueResult(kind="deduped") + + logger.info( + "baseline trial enqueued", + event_type="baseline_enqueued", + study_id=study.id, + trial_id=trial_id, + ) + return BaselineEnqueueResult(kind="enqueued", trial_id=trial_id) + + +async def _wait_for_baseline_trial_by_id( + session_factory: async_sessionmaker[AsyncSession], + study_id: str, + trial_id: str, + wait_s: float, +) -> Trial | None: + """Poll trials by ``id = trial_id`` until terminal, cancel, or timeout. + + Used when ``BaselineEnqueueResult.kind == 'enqueued'`` — the orchestrator + knows the exact trial_id it generated. + + Returns ``None`` on timeout OR on cancel (status leaves 'running'); the + caller's polling loop sees the status change on its own tick and exits. + """ + deadline = asyncio.get_event_loop().time() + wait_s + while True: + async with session_factory() as db: + stmt = ( + select(Trial) + .where(Trial.id == trial_id) + .where(Trial.status.in_(("complete", "failed", "pruned"))) + .limit(1) + ) + trial = (await db.execute(stmt)).scalar_one_or_none() + if trial is not None: + return trial + if await _study_cancelled(db, study_id): + return None + if asyncio.get_event_loop().time() >= deadline: + return None + await asyncio.sleep(_REPLENISH_TICK_S) + + +async def _wait_for_baseline_trial_by_study( + session_factory: async_sessionmaker[AsyncSession], + study_id: str, + wait_s: float, +) -> Trial | None: + """Poll trials by ``study_id + is_baseline=TRUE`` until terminal, cancel, or timeout. + + Used when ``BaselineEnqueueResult.kind == 'deduped'`` — the original + enqueue's trial_id is unknown to this orchestrator invocation, so we + observe any terminal baseline row for the study (per plan-cycle-2 F2). + """ + deadline = asyncio.get_event_loop().time() + wait_s + while True: + async with session_factory() as db: + trial = await _find_terminal_baseline_row(db, study_id) + if trial is not None: + return trial + if await _study_cancelled(db, study_id): + return None + if asyncio.get_event_loop().time() >= deadline: + return None + await asyncio.sleep(_REPLENISH_TICK_S) + + +async def _study_cancelled(db: AsyncSession, study_id: str) -> bool: + """Return True if the study is no longer ``running``. + + Cancel-aware bail-out for the baseline wait helpers — without this, an + operator cancel mid-baseline would have to wait out the full + ``_BASELINE_WAIT_FLOOR_S`` (60s) before the orchestrator's polling loop + saw the new status. + """ + current = await repo.get_study(db, study_id) + return current is None or current.status != "running" + + # --------------------------------------------------------------------------- # Internals # --------------------------------------------------------------------------- @@ -324,9 +612,16 @@ async def _last_n_all_failed(db: AsyncSession, study_id: str, *, n: int) -> bool trials exist, returns False (insufficient signal). A single non-failed trial in the window resets the streak. """ + # FR-11: exclude the baseline row (is_baseline=TRUE, optuna_trial_number=-1) + # from the "last N trials" streak check. Without this filter, after the + # baseline phase completes but before the first Optuna trial reaches + # terminal, ORDER BY optuna_trial_number DESC LIMIT n could return only + # the baseline row — a failed baseline would spuriously trigger the + # "5 consecutive failures" abort even though no Optuna trial ran. stmt = ( select(Trial.status) .where(Trial.study_id == study_id) + .where(Trial.is_baseline.is_(False)) .order_by(Trial.optuna_trial_number.desc()) .limit(n) ) @@ -356,9 +651,12 @@ async def _last_n_all_zero(db: AsyncSession, study_id: str, *, n: int) -> bool: producing false-positive aborts whenever a non-zero, failed, or pruned row sits inside the recent window. """ + # FR-11: same rationale as _last_n_all_failed — exclude the baseline + # row so a failed baseline can't trigger the no-signal abort. stmt = ( select(Trial.status, Trial.primary_metric) .where(Trial.study_id == study_id) + .where(Trial.is_baseline.is_(False)) .order_by(Trial.optuna_trial_number.desc()) .limit(n) ) diff --git a/docs/00_overview/DASHBOARD.md b/docs/00_overview/DASHBOARD.md index 90c08c6c..7fd54878 100644 --- a/docs/00_overview/DASHBOARD.md +++ b/docs/00_overview/DASHBOARD.md @@ -6,7 +6,7 @@ _Top-level index across MVP1 → GA v1+ as of **2026-05-25**. Click a release na | Release | Theme | Progress | Status | |---|---|---|---| -| [MVP1 / v0.1](MVP1_DASHBOARD.md) | The Loop | 76 / 76 scoped done · 13 remaining | **In progress** | +| [MVP1 / v0.1](MVP1_DASHBOARD.md) | The Loop | 76 / 77 scoped done · 14 remaining | **In progress** | | [MVP1.5 / v0.1.5](MVP1_5_DASHBOARD.md) | Real Signals | 1 item(s) queued | **Held / queued** | | [MVP2 / v0.2](MVP2_DASHBOARD.md) | Observable | 1 / 1 scoped done · 1 remaining | **In progress** | | MVP3 / v0.3 | Production Stacks | — | **Not yet scoped** | diff --git a/docs/00_overview/MVP1_DASHBOARD.md b/docs/00_overview/MVP1_DASHBOARD.md index 226e106a..87676a1c 100644 --- a/docs/00_overview/MVP1_DASHBOARD.md +++ b/docs/00_overview/MVP1_DASHBOARD.md @@ -6,23 +6,29 @@ _Reflects feature-folder state as of **2026-05-25** (latest mtime of any planned ## Next up -All scoped MVP1 features shipped 🎉 +**[feat_study_baseline_trial](../02_product/planned_features/feat_study_baseline_trial/feature_spec.md)** — Feature, currently in **Plan** -Pull from the Idea backlog or capture a new feature spec. +> The orchestrator runs a single non-Optuna baseline trial before Optuna starts, persists it as a real `Trial` row, stamps `studies.baseline_metric` + `studies.baseline_trial_id`, and the confidence analytics + auto-followup gate + digest nar + +Plan approved; run /impl-execute to ship + +```bash +/impl-execute docs/02_product/planned_features/feat_study_baseline_trial/implementation_plan.md --all +``` ## MVP1 Progress | Metric | Value | |---|---| -| Scoped items done | **76 / 76** (100%) — feat_/infra_/chore_/epic_ past idea stage | +| Scoped items done | **76 / 77** (99%) — feat_/infra_/chore_/epic_ past idea stage | | Pending work | **17** items (every not-done feat/infra/chore/bug across all priorities) | | → P0 — do next | **0** unblocking / paying daily cost | | → P1 | **0** high-value, ready when P0 clears | | → P2 (default) | 16 important to file, not blocking | | → Backlog | 1 captured for record, not planned | | Open bugs | 6 | -| Legacy "Path to MVP1" | 13 items — scoped-not-done + bugs + chore-ideas only (excludes feat/infra ideas) | -| Backlog ideas | 4 idea-only feat/infra (not yet scoped into MVP1) | +| Legacy "Path to MVP1" | 14 items — scoped-not-done + bugs + chore-ideas only (excludes feat/infra ideas) | +| Backlog ideas | 3 idea-only feat/infra (not yet scoped into MVP1) | | In flight | 0 feature(s) actively shipping | ## Pipeline @@ -131,35 +137,36 @@ Pull from the Idea backlog or capture a new feature spec. _None._ -### Plan (0) +### Plan (1) -_None._ +| # | Priority | Feature | Type | One-liner | Depends on | Status | +|---|---|---|---|---|---|---| +| 1 | P2 | [feat_study_baseline_trial](../02_product/planned_features/feat_study_baseline_trial/feature_spec.md) | Feature | The orchestrator runs a single non-Optuna baseline trial before Optuna starts, persists it as a real `Trial` row, stamps `studies.baseline_metric` + `studies.baseline_trial_id`, and the confidence ana | — | [PR #180](https://github.com/SoundMindsAI/relyloop/pull/180) merged 2026-05-21 | ### Spec (0) _None._ -### Idea (17) +### Idea (16) | # | Priority | Feature | Type | One-liner | Depends on | Status | |---|---|---|---|---|---|---| -| 1 | P2 | [feat_study_baseline_trial](../02_product/planned_features/feat_study_baseline_trial/idea.md) | Feature | `studies.baseline_metric` exists as a column on the `studies` table (declared in `feat_study_lifecycle` Phase 1, [`backend/app/db/models/study.py:76`](../../backend/app/db/models/study.py#L76)) with t | — | Idea — deferred Phase 2 work from `feat_pr_metric_confidence` (Phase 1 merged 2026-05-21 as PR #180 squash `d0a8358`). | -| 2 | P2 | [feat_study_clone_narrow_bounds](../02_product/planned_features/feat_study_clone_narrow_bounds/idea.md) | Feature | `feat_study_clone_from_previous` v1 ships verbatim-copy + editable-fields. The next iteration friction is: after cloning, the engineer must manually narrow the `search_space` bounds around the best-tr | — | Idea — deferred follow-up of [`feat_study_clone_from_previous`](../feat_study_clone_from_previous/) (per that spec's locked D-3). | -| 3 | P2 | [infra_agent_sibling_worktree_isolation](../02_product/planned_features/infra_agent_sibling_worktree_isolation/idea.md) | Infra | Running an autonomous agent in a sibling git worktree while the operator's main checkout has the Docker Compose stack up exposes two surfaces that aren't designed for parallel work: | — | Idea — tangential observations from the autonomous `chore_reconciler_terminal_closed_no_poll` agent run (PR #216, merged 2026-05-23) | -| 4 | P2 | [infra_study_preflight_real_engine_integration](../02_product/planned_features/infra_study_preflight_real_engine_integration/idea.md) | Infra | `feat_study_preflight_overlap_probe`'s integration tests (AC-1 through AC-4b in [`backend/tests/integration/test_studies_api.py`](../../backend/tests/integration/test_studies_api.py)) use… | — | Idea — surfaced during `feat_study_preflight_overlap_probe` (PR ___) phase-gate review | -| 5 | P2 | [chore_auto_followup_completed_parent_stop_chain_race](../02_product/planned_features/chore_auto_followup_completed_parent_stop_chain_race/idea.md) | Chore | The cycle-3 C3-1 cascade-cancel design tolerates terminal parents (cascade traverses through `completed` intermediates to reach in-flight descendants). But the FR-1 digest trigger fires `enqueue_follo | — | Idea — surfaced during the Epic 1+2 phase-gate GPT-5.5 review of `feat_auto_followup_studies` (cumulative-diff review finding F2, accepted in part as a future-work capture) | -| 6 | P2 | [chore_auto_followup_e2e_chain_seed_helper](../02_product/planned_features/chore_auto_followup_e2e_chain_seed_helper/idea.md) | Chore | `feat_auto_followup_studies` Story 3.3 specified a Playwright E2E spec that seeds a 3-node chain (root R → middle M → leaf L) and asserts: | — | Idea | -| 7 | P2 | [chore_dashboard_regen_quoted_pr_false_positive](../02_product/planned_features/chore_dashboard_regen_quoted_pr_false_positive/idea.md) | Chore | [`_extract_pr_number`](../../scripts/build_mvp1_dashboard.py#L572)'s priority-3 fuzzy match has two regexes: | — | Idea — surfaced during `chore_dashboard_pr_extraction_from_idea` empirical verification (2026-05-23) | -| 8 | P2 | [chore_e2e_seed_acme_idea_obsolete](../02_product/planned_features/chore_e2e_seed_acme_idea_obsolete/idea.md) | Chore | [`chore_e2e_seed_acme_helper_dead/idea.md`](../02_product/planned_features/chore_e2e_seed_acme_helper_dead/idea.md) (dated 2026-05-21) proposed two paths: | — | Idea — surfaced during `chore_migration_test_head_brittleness` `/idea-preflight` pick (2026-05-23) | -| 9 | P2 | [chore_studies_post_arq_spy_fixture](../02_product/planned_features/chore_studies_post_arq_spy_fixture/idea.md) | Chore | The studies POST handler at [`backend/app/api/v1/studies.py:307`](../../backend/app/api/v1/studies.py#L307) calls `await _enqueue_start_study(request, study_id)` after a successful create. The helper | — | Idea — surfaced during `feat_study_preflight_overlap_probe` (PR ___) phase-gate review | -| 10 | P2 | [chore_template_library_expansion](../02_product/planned_features/chore_template_library_expansion/idea.md) | Chore | Three connected gaps: | — | Idea — surfaced during a UX review of parameter-tuning ergonomics on 2026-05-19. | -| 11 | P2 | [bug_datatable_col_vis_density_localstorage_undefined_jsdom](../02_product/planned_features/bug_datatable_col_vis_density_localstorage_undefined_jsdom/idea.md) | Bug | The first integration test in the file (`toggling a column off via the menu removes its cells and persists to localStorage`, line 148) accesses `window.localStorage` successfully. By the time the 3rd– | — | Idea — captured during feat_study_clone_from_previous Story 2.1 vitest sweep | -| 12 | P2 | [bug_dockerfile_missing_scripts_dir](../02_product/planned_features/bug_dockerfile_missing_scripts_dir/idea.md) | Bug | [`backend/app/services/demo_seeding.py:39`](../../backend/app/services/demo_seeding.py#L39) imports four constants from `scripts/seed_meaningful_demos.py`: | — | **Fixed** in PR #232 commit (this branch). Idea file captures the bug + the fix + the systemic lesson for future contributors. | -| 13 | P2 | [bug_markdown_doc_localstorage_undefined_jsdom](../02_product/planned_features/bug_markdown_doc_localstorage_undefined_jsdom/idea.md) | Bug | The afterEach hook unconditionally calls `window.localStorage.removeItem(...)` after each test, but `window.localStorage` is `undefined` in the test environment by the time the hook runs — either the | — | Idea — captured during feat_digest_executable_followups implementation (Story 5.1 vitest sweep) | -| 14 | P2 | [bug_smoke_dashboard_demo_state_locator_missing](../02_product/planned_features/bug_smoke_dashboard_demo_state_locator_missing/idea.md) | Bug | Both `getByTestId('reset-demo-state-disclosure')` and `getByTestId('demo-data-banner')` are NOT being rendered on the smoke-stack's `/` route. Either: | — | Idea — captured during feat_study_clone_from_previous PR #243 CI watch | -| 15 | P2 | [bug_vitest_jsdom_localstorage_failures](../02_product/planned_features/bug_vitest_jsdom_localstorage_failures/idea.md) | Bug | `pnpm vitest run` on `feature/home-demo-reseed-endpoint` (and `main`) reports the following 4 files failing with the same root error: | — | open. | -| 16 | P2 | [bug_webhook_concurrent_merge_race_timing_sensitive](../02_product/planned_features/bug_webhook_concurrent_merge_race_timing_sensitive/idea.md) | Bug | Idea — surfaced during `bug_demo_clusters_unreachable_in_healthz` PR #236 CI. | — | Idea — surfaced during `bug_demo_clusters_unreachable_in_healthz` PR #236 CI. | -| 17 | Backlog | [chore_e2e_seed_acme_helper_dead](../02_product/planned_features/chore_e2e_seed_acme_helper_dead/idea.md) | Chore | `seedAcmeProductsChain` is a 140-line helper that constructs a cluster + query_set + template + judgment_list + study + optional proposal/digest chain "Acme Products" demo scenario. The function is co | — | Idea — surfaced during `chore_e2e_test_rows_isolation` Story 1.2 coverage audit | +| 1 | P2 | [feat_study_clone_narrow_bounds](../02_product/planned_features/feat_study_clone_narrow_bounds/idea.md) | Feature | `feat_study_clone_from_previous` v1 ships verbatim-copy + editable-fields. The next iteration friction is: after cloning, the engineer must manually narrow the `search_space` bounds around the best-tr | — | Idea — deferred follow-up of [`feat_study_clone_from_previous`](../feat_study_clone_from_previous/) (per that spec's locked D-3). | +| 2 | P2 | [infra_agent_sibling_worktree_isolation](../02_product/planned_features/infra_agent_sibling_worktree_isolation/idea.md) | Infra | Running an autonomous agent in a sibling git worktree while the operator's main checkout has the Docker Compose stack up exposes two surfaces that aren't designed for parallel work: | — | Idea — tangential observations from the autonomous `chore_reconciler_terminal_closed_no_poll` agent run (PR #216, merged 2026-05-23) | +| 3 | P2 | [infra_study_preflight_real_engine_integration](../02_product/planned_features/infra_study_preflight_real_engine_integration/idea.md) | Infra | `feat_study_preflight_overlap_probe`'s integration tests (AC-1 through AC-4b in [`backend/tests/integration/test_studies_api.py`](../../backend/tests/integration/test_studies_api.py)) use… | — | Idea — surfaced during `feat_study_preflight_overlap_probe` (PR ___) phase-gate review | +| 4 | P2 | [chore_auto_followup_completed_parent_stop_chain_race](../02_product/planned_features/chore_auto_followup_completed_parent_stop_chain_race/idea.md) | Chore | The cycle-3 C3-1 cascade-cancel design tolerates terminal parents (cascade traverses through `completed` intermediates to reach in-flight descendants). But the FR-1 digest trigger fires `enqueue_follo | — | Idea — surfaced during the Epic 1+2 phase-gate GPT-5.5 review of `feat_auto_followup_studies` (cumulative-diff review finding F2, accepted in part as a future-work capture) | +| 5 | P2 | [chore_auto_followup_e2e_chain_seed_helper](../02_product/planned_features/chore_auto_followup_e2e_chain_seed_helper/idea.md) | Chore | `feat_auto_followup_studies` Story 3.3 specified a Playwright E2E spec that seeds a 3-node chain (root R → middle M → leaf L) and asserts: | — | Idea | +| 6 | P2 | [chore_dashboard_regen_quoted_pr_false_positive](../02_product/planned_features/chore_dashboard_regen_quoted_pr_false_positive/idea.md) | Chore | [`_extract_pr_number`](../../scripts/build_mvp1_dashboard.py#L572)'s priority-3 fuzzy match has two regexes: | — | Idea — surfaced during `chore_dashboard_pr_extraction_from_idea` empirical verification (2026-05-23) | +| 7 | P2 | [chore_e2e_seed_acme_idea_obsolete](../02_product/planned_features/chore_e2e_seed_acme_idea_obsolete/idea.md) | Chore | [`chore_e2e_seed_acme_helper_dead/idea.md`](../02_product/planned_features/chore_e2e_seed_acme_helper_dead/idea.md) (dated 2026-05-21) proposed two paths: | — | Idea — surfaced during `chore_migration_test_head_brittleness` `/idea-preflight` pick (2026-05-23) | +| 8 | P2 | [chore_studies_post_arq_spy_fixture](../02_product/planned_features/chore_studies_post_arq_spy_fixture/idea.md) | Chore | The studies POST handler at [`backend/app/api/v1/studies.py:307`](../../backend/app/api/v1/studies.py#L307) calls `await _enqueue_start_study(request, study_id)` after a successful create. The helper | — | Idea — surfaced during `feat_study_preflight_overlap_probe` (PR ___) phase-gate review | +| 9 | P2 | [chore_template_library_expansion](../02_product/planned_features/chore_template_library_expansion/idea.md) | Chore | Three connected gaps: | — | Idea — surfaced during a UX review of parameter-tuning ergonomics on 2026-05-19. | +| 10 | P2 | [bug_datatable_col_vis_density_localstorage_undefined_jsdom](../02_product/planned_features/bug_datatable_col_vis_density_localstorage_undefined_jsdom/idea.md) | Bug | The first integration test in the file (`toggling a column off via the menu removes its cells and persists to localStorage`, line 148) accesses `window.localStorage` successfully. By the time the 3rd– | — | Idea — captured during feat_study_clone_from_previous Story 2.1 vitest sweep | +| 11 | P2 | [bug_dockerfile_missing_scripts_dir](../02_product/planned_features/bug_dockerfile_missing_scripts_dir/idea.md) | Bug | [`backend/app/services/demo_seeding.py:39`](../../backend/app/services/demo_seeding.py#L39) imports four constants from `scripts/seed_meaningful_demos.py`: | — | **Fixed** in PR #232 commit (this branch). Idea file captures the bug + the fix + the systemic lesson for future contributors. | +| 12 | P2 | [bug_markdown_doc_localstorage_undefined_jsdom](../02_product/planned_features/bug_markdown_doc_localstorage_undefined_jsdom/idea.md) | Bug | The afterEach hook unconditionally calls `window.localStorage.removeItem(...)` after each test, but `window.localStorage` is `undefined` in the test environment by the time the hook runs — either the | — | Idea — captured during feat_digest_executable_followups implementation (Story 5.1 vitest sweep) | +| 13 | P2 | [bug_smoke_dashboard_demo_state_locator_missing](../02_product/planned_features/bug_smoke_dashboard_demo_state_locator_missing/idea.md) | Bug | Both `getByTestId('reset-demo-state-disclosure')` and `getByTestId('demo-data-banner')` are NOT being rendered on the smoke-stack's `/` route. Either: | — | Idea — captured during feat_study_clone_from_previous PR #243 CI watch | +| 14 | P2 | [bug_vitest_jsdom_localstorage_failures](../02_product/planned_features/bug_vitest_jsdom_localstorage_failures/idea.md) | Bug | `pnpm vitest run` on `feature/home-demo-reseed-endpoint` (and `main`) reports the following 4 files failing with the same root error: | — | open. | +| 15 | P2 | [bug_webhook_concurrent_merge_race_timing_sensitive](../02_product/planned_features/bug_webhook_concurrent_merge_race_timing_sensitive/idea.md) | Bug | Idea — surfaced during `bug_demo_clusters_unreachable_in_healthz` PR #236 CI. | — | Idea — surfaced during `bug_demo_clusters_unreachable_in_healthz` PR #236 CI. | +| 16 | Backlog | [chore_e2e_seed_acme_helper_dead](../02_product/planned_features/chore_e2e_seed_acme_helper_dead/idea.md) | Chore | `seedAcmeProductsChain` is a 140-line helper that constructs a cluster + query_set + template + judgment_list + study + optional proposal/digest chain "Acme Products" demo scenario. The function is co | — | Idea — surfaced during `chore_e2e_test_rows_isolation` Story 1.2 coverage audit | ## Dependency graph @@ -172,6 +179,8 @@ graph LR classDef plan fill:#fef9c3,stroke:#854d0e,color:#854d0e; classDef spec fill:#dbeafe,stroke:#1e40af,color:#1e40af; classDef idea fill:#f1f5f9,stroke:#334155,color:#334155; + feat_study_baseline_trial["study baseline trial"] + class feat_study_baseline_trial plan; infra_foundation["foundation"] class infra_foundation done; feat_study_lifecycle["study lifecycle"] diff --git a/docs/00_overview/dashboard.html b/docs/00_overview/dashboard.html index 6ae9c95a..3b26fd12 100644 --- a/docs/00_overview/dashboard.html +++ b/docs/00_overview/dashboard.html @@ -384,7 +384,7 @@

Releases

The Loop
-
76 / 76 scoped done · 13 remaining
+
76 / 77 scoped done · 14 remaining
In progress
diff --git a/docs/00_overview/mvp1_dashboard.html b/docs/00_overview/mvp1_dashboard.html index 2d26cb28..cad0973b 100644 --- a/docs/00_overview/mvp1_dashboard.html +++ b/docs/00_overview/mvp1_dashboard.html @@ -382,12 +382,12 @@

RelyLoop MVP1 Dashboard

-
-
Next up
-
All scoped MVP1 features shipped 🎉
-
- Pull from the Idea backlog or capture a new feature spec. -
+
+
Next up — Feature, currently in Plan
+ +
The orchestrator runs a single non-Optuna baseline trial before Optuna starts, persists it as a real `Trial` row, stamps `studies.baseline_metric` + `studies.baseline_trial_id`, and the confidence analytics + auto-followup gate + digest nar
+
Plan approved; run /impl-execute to ship
+ /impl-execute docs/02_product/planned_features/feat_study_baseline_trial/implementation_plan.md --all
@@ -395,11 +395,11 @@

RelyLoop MVP1 Dashboard

MVP1 Progress

-
+
Scoped items done
-
76 / 76
-
100% of feat_/infra_/chore_/epic_ items past idea stage
-
+
76 / 77
+
99% of feat_/infra_/chore_/epic_ items past idea stage
+
Pending work
@@ -435,14 +435,14 @@

MVP1 Progress

Legacy "Path to MVP1"
-
13
+
14
scoped not-done + bugs + chore-ideas only (excludes feat/infra ideas)
Backlog ideas: - 4 idea-only feat/infra folders (not yet scoped into MVP1) + 3 idea-only feat/infra folders (not yet scoped into MVP1) In flight: @@ -463,20 +463,7 @@

Pipeline

-

Idea 17

- -
- -
- Feature - P2 - -
-
`studies.baseline_metric` exists as a column on the `studies` table (declared in `feat_study_lifecycle` Phase 1, [`backend/app/db/models/study.py:76`](../../backend/app/db/models/study.py#L76)) with t
- - -
- +

Idea 16

@@ -693,7 +680,19 @@

Spec 0

-

Plan 0

+

Plan 1

+ +
+ +
+ Feature + P2 + PR #180 merged 2026-05-21 +
+
The orchestrator runs a single non-Optuna baseline trial before Optuna starts, persists it as a real `Trial` row, stamps `studies.baseline_metric` + `studies.baseline_trial_id`, and the confidence ana
+ + +
@@ -1952,6 +1951,8 @@

Dependency graph (feat_ + infra_)

classDef plan fill:#fef9c3,stroke:#854d0e,color:#854d0e; classDef spec fill:#dbeafe,stroke:#1e40af,color:#1e40af; classDef idea fill:#f1f5f9,stroke:#334155,color:#334155; + feat_study_baseline_trial["study baseline trial"] + class feat_study_baseline_trial plan; infra_foundation["foundation"] class infra_foundation done; feat_study_lifecycle["study lifecycle"] @@ -2157,6 +2158,8 @@

Dependency graph (feat_ + infra_)

classDef plan fill:#fef9c3,stroke:#854d0e,color:#854d0e; classDef spec fill:#dbeafe,stroke:#1e40af,color:#1e40af; classDef idea fill:#f1f5f9,stroke:#334155,color:#334155; + feat_study_baseline_trial["study baseline trial"] + class feat_study_baseline_trial plan; infra_foundation["foundation"] class infra_foundation done; feat_study_lifecycle["study lifecycle"] diff --git a/docs/01_architecture/data-model.md b/docs/01_architecture/data-model.md index 47bb722d..0630d8ac 100644 --- a/docs/01_architecture/data-model.md +++ b/docs/01_architecture/data-model.md @@ -211,7 +211,8 @@ CREATE TABLE studies ( parent_study_id UUID REFERENCES studies(id), -- for forks (MVP2) parent_proposal_id VARCHAR(36) REFERENCES proposals(id), -- feat_digest_executable_followups (0018) — set when this study was spawned from a digest "Run this followup" parent_proposal_followup_index INT, -- 0-based index into the parent digest's suggested_followups; paired with parent_proposal_id (CHECK enforces both-NULL or both-set-with-index>=0); BEFORE DELETE trigger on proposals atomically NULLs both columns on parent hard-delete - baseline_metric REAL, -- single non-Optuna trial run before Optuna starts; populated by orchestrator + baseline_metric REAL, -- single non-Optuna trial run before Optuna starts; populated by orchestrator + worker self-stamp via services.study_state.stamp_baseline_trial (feat_study_baseline_trial 0020) + baseline_trial_id VARCHAR(36), -- denormalized FK to the is_baseline=TRUE trial row (feat_study_baseline_trial 0020); not a formal FK — orchestrator stamps it post-completion best_metric REAL, best_trial_id UUID, created_at TIMESTAMPTZ NOT NULL DEFAULT now(), @@ -232,11 +233,18 @@ CREATE TABLE trials ( error TEXT, started_at TIMESTAMPTZ, ended_at TIMESTAMPTZ, + is_baseline BOOLEAN NOT NULL DEFAULT FALSE, -- feat_study_baseline_trial (0020) — TRUE only for the off-band non-Optuna baseline trial (optuna_trial_number=-1 sentinel) CONSTRAINT trials_per_query_metrics_object_check CHECK (per_query_metrics IS NULL OR jsonb_typeof(per_query_metrics) = 'object') ); CREATE INDEX trials_study_metric ON trials (study_id, primary_metric DESC NULLS LAST); +-- feat_study_baseline_trial (0020) — at most one COMPLETE baseline per study. +-- Combined with Arq _job_id dedupe + FR-12 stamping helper's WHERE baseline_trial_id IS NULL +-- predicate, this is the 3-layer defense against orchestrator double-enqueue on resume (D-16). +CREATE UNIQUE INDEX uq_trials_study_baseline_complete + ON trials (study_id) + WHERE is_baseline = TRUE AND status = 'complete'; ``` `trials` is hard-delete only (no `deleted_at`) — when a study is removed, trials cascade-delete with it; trial history is regenerable from Optuna's RDB if needed. diff --git a/docs/02_product/planned_features/feat_study_baseline_trial/feature_spec.md b/docs/02_product/planned_features/feat_study_baseline_trial/feature_spec.md new file mode 100644 index 00000000..9a183f8e --- /dev/null +++ b/docs/02_product/planned_features/feat_study_baseline_trial/feature_spec.md @@ -0,0 +1,700 @@ +# Feature Specification — Baseline-trial computation (`feat_study_baseline_trial`) + +**Date:** 2026-05-25 +**Status:** Draft — pending GPT-5.5 cross-model review +**Owners:** Eric Starr (engineering), Eric Starr (product) +**Related docs:** + +- Idea: [`idea.md`](idea.md) +- Phase 1 spec (shipped): [`feat_pr_metric_confidence/feature_spec.md`](../../../00_overview/implemented_features/2026_05_21_feat_pr_metric_confidence/feature_spec.md) +- Sibling that gates on this feature: [`feat_auto_followup_studies/feature_spec.md`](../../../00_overview/implemented_features/2026_05_24_feat_auto_followup_studies/feature_spec.md) (FR-2b) +- Sibling that supplies lineage: [`feat_digest_executable_followups/feature_spec.md`](../../../00_overview/implemented_features/2026_05_24_feat_digest_executable_followups/feature_spec.md) +- API conventions: [`docs/01_architecture/api-conventions.md`](../../../01_architecture/api-conventions.md) +- Data model: [`docs/01_architecture/data-model.md`](../../../01_architecture/data-model.md) + +**Depends on:** Phase 1 of `feat_pr_metric_confidence` (PR #180, merged 2026-05-21). Additive — no API contract break, no migration to undo. + +--- + +## 1) Purpose + +- **Problem:** `studies.baseline_metric` is declared (`backend/app/db/models/study.py:95`) but never written. The PR body's `## Metric delta` section shows `baseline=None → achieved=X` with no `delta_pct`. Phase 1 of `feat_pr_metric_confidence` ships per-query analytics that compare the winner against the runner-up #2 trial — useful for "is this winner robust?" but not "does this regress queries production gets right?" +- **Outcome:** The orchestrator runs a single non-Optuna baseline trial before Optuna starts, persists it as a real `Trial` row, stamps `studies.baseline_metric` + `studies.baseline_trial_id`, and the confidence analytics + auto-followup gate + digest narrative + PR body all switch from "vs runner-up" to "vs baseline" automatically. The approver's central question — "does my candidate beat my current production behavior?" — becomes answerable on every PR. +- **Non-goal:** This feature does NOT redesign the orchestrator's polling loop, does NOT modify the search-space schema, does NOT add a new evaluation metric, and does NOT change Optuna's sampler/pruner behavior. The baseline trial is just one more `Trial` row that the runtime largely treats like any other — with a `is_baseline=true` flag and an off-band trial-number sentinel so the Optuna RDB never sees it. + +## 2) Current state audit + +### Existing implementations + +| Surface | File:Line | What it does today | Why this feature touches it | +|---|---|---|---| +| `studies.baseline_metric` column | `backend/app/db/models/study.py:95` | Declared `Float NULL` with docstring "populated by the orchestrator (Phase 2)". Always `NULL` in production. | Phase 2 finally writes it. | +| `_compute_metric_delta` | `backend/workers/digest.py:510-522` | Reads `study.baseline_metric` into `baseline`, computes `delta_pct = (achieved - baseline) / baseline * 100` when non-zero baseline. Today both inputs are None. | After this feature, both inputs are populated → the PR body's metric-delta section gains real numbers automatically (zero code change). | +| Digest user prompt | `prompts/digest_narrative.user.jinja:10-13` | `` block already emits `baseline_metric: {{ baseline_metric if baseline_metric is not none else 'N/A (no baseline trial)' }}`. | The template is already future-proof — no change needed. | +| Digest system prompt | `prompts/digest_narrative.system.md:34-39` | Already documents `comparison_against` taking `runner_up` (MVP1) or `baseline` (Phase 2). | The framing guidance for "regressed vs production baseline" wording lives here — needs a 1–2 sentence addition (FR-7). | +| `compute_study_confidence` | `backend/app/domain/study/confidence.py:496-635` | Pure-domain orchestrator. Always emits `comparison_against="runner_up"` at line 624 with comment `# FR-3 locked for Phase 1`. | One-line conditional change to switch to `"baseline"` when `study.baseline_trial_id` is set AND the row has `per_query_metrics`. | +| `ComparisonAgainst` Literal | `backend/app/domain/study/confidence.py:114` | `Literal["runner_up", "baseline"]` — both values already wire-modeled. Docstring at line 115 explicitly states Phase 1 unconditionally emits `runner_up` and `baseline` is reserved for Phase 2. | No change. | +| `evaluate_chain_gate` | `backend/app/domain/study/auto_followup.py:91-169` | Computes `lift = parent.best_metric - first_decile_max` (implicit-baseline proxy from earliest decile of complete trials). Module docstring at lines 7-11 explicitly says: "When `feat_study_baseline_trial` ships and populates `studies.baseline_metric`, FR-2b activates and this module switches to 'lift-over-baseline' via a one-line change." | Touch points enumerated in FR-5 below. | +| `ConfidencePanel` UI | `ui/src/components/studies/confidence-panel.tsx:98,113` | Reads `per_query_outcomes.comparison_against` and calls `formatComparison()` — already handles both wire values. Test at `ui/src/__tests__/components/studies/confidence-panel.test.tsx:136` ("switches the comparison label to 'vs baseline' when comparison_against === 'baseline' (Phase 2 future)") already passes. | No code change. The label flip is data-driven and the future test is already green. | +| Frontend enum allowlist | `ui/src/lib/enums.ts:83-87` | `COMPARISON_AGAINST_VALUES = ['runner_up', 'baseline'] as const` with `// Values must match backend/app/domain/study/confidence.py ComparisonAgainst.` source-of-truth comment. | No change. | +| Glossary entry | `ui/src/lib/glossary.ts:676` | `confidence.comparison_against` entry already authored. | No change. | +| Frontend types | `ui/src/lib/types.ts:2135,2690` | `comparison_against: 'runner_up' \| 'baseline'` and `baseline_metric: number \| null` already typed. | Add `baseline_trial_id: string \| null` to `StudyDetail` (the openapi types regenerate from the FastAPI schema). | +| PR body confidence section | `backend/workers/git_pr.py:513` | `f"{outcomes.regressed} regressed (vs {outcomes.comparison_against})"` — template-driven. | No code change; the wire value flips. | +| `create_proposal_from_study` agent tool | `backend/app/agent/tools/proposals/create_proposal_from_study.py:62-68` | Already builds `metric_delta = {"baseline_metric": study.baseline_metric, "best_metric": study.best_metric}` when either is non-None. | No code change; today the dict is built only when `best_metric` is non-None and stamps `baseline_metric=None`. Post-feature, it contains both numbers. | +| `studies.py:_detail` | `backend/app/api/v1/studies.py:121-153` | Serializes `baseline_metric` onto `StudyDetail`. | Add `baseline_trial_id` to the serializer + `StudyDetail` schema. | +| Existing tests asserting `comparison_against == "runner_up"` | `backend/tests/unit/workers/test_digest_prompt_render.py:185,243`; `backend/tests/unit/domain/study/test_confidence.py:432`; `backend/tests/contract/test_pr_body_confidence_section.py:54,205`; `backend/tests/integration/test_studies_api_confidence.py:477` | 5 tests assert the literal value `"runner_up"` against fixtures that have no baseline. | NO change — these fixtures don't set `baseline_trial_id`, so the FR-4 conditional keeps them on the `"runner_up"` branch (regression coverage for the fallback path). | + +### Navigation and link impact + +N/A — no new pages, no URL changes. The feature is data-flow + worker-orchestration only. + +### Existing test impact + +| Test file | Pattern | Count | Required change | +|---|---|---|---| +| `backend/tests/unit/domain/study/test_confidence.py` | tests for `compute_study_confidence` with `comparison_against == "runner_up"` | 1 assertion (line 432) | Keep (regression coverage for FR-4 fallback). | +| `backend/tests/contract/test_pr_body_confidence_section.py` | tests for PR body confidence wording with `comparison_against="runner_up"` | 2 fixtures (lines 54, 205) | Keep (regression coverage). Add new fixture with `baseline_trial_id` set to cover FR-4 baseline branch. | +| `backend/tests/integration/test_studies_api_confidence.py` | integration test for `/api/v1/studies/{id}` confidence shape | 1 assertion (line 477) + 1 fixture (line 112 with `baseline_metric=None`) | Keep + add baseline-branch test. | +| `backend/tests/integration/test_existing_row_read_compat.py` | regression: rows pre-dating `baseline_metric` populate read correctly | line 117 sets `baseline_metric=None` | Keep — verifies the FR-7 fallback path. | +| `backend/tests/integration/test_open_pr_worker_confidence_plumbing.py` | open_pr worker confidence read-side plumbing | line 115 sets `baseline_metric=None` | Keep. Add a new test with baseline trial + non-None baseline_metric. | +| `backend/tests/integration/_digest_helpers.py` | shared helper `seed_completed_study(baseline_metric=0.612)` | already parameterised | No change. | +| `backend/tests/unit/workers/test_digest_prompt_render.py` | digest user-prompt render with `baseline_metric=0.612` | 2 assertions on `runner_up` (185, 243) | Keep — runner_up branch is hit when `baseline_trial_id IS NULL`. Add new test for the `baseline` branch. | + +### Existing behaviors affected by scope change + +- **`_compute_metric_delta` output**: Current: `{primary_metric_key: {baseline: None, achieved: 0.65, delta_pct: None}}`. New: `{primary_metric_key: {baseline: 0.51, achieved: 0.65, delta_pct: 27.5}}`. Decision: **yes**, this is the headline UX win — locked in §19 D-1. +- **`compute_study_confidence` comparison source**: Current: always runner-up #2. New: baseline trial when `baseline_trial_id IS NOT NULL` AND that row has `per_query_metrics`; runner-up #2 otherwise. Decision: **yes**, locked in §19 D-2. +- **Auto-followup gate's "lift" definition**: Current: `parent.best_metric - first_decile_max` (implicit-baseline from earliest decile). New: `parent.best_metric - parent.baseline_metric` when `parent.baseline_metric IS NOT NULL`; `first_decile_max` fallback otherwise. Decision: **yes**, locked in §19 D-3. This is the "one-line change" the `auto_followup.py:9-11` module docstring promised. +- **Trial-listing UI shows the baseline trial**: Current: nothing to show. New: depends on the UX decision in §19 Open Question 1 — filter out by default, or show with a "Baseline" badge. Decision: **deferred to spec**, recommended default in §19. + +--- + +## 3) Scope + +### In scope + +- New column `studies.baseline_trial_id` (denormalized FK to the baseline trial row). +- New column `trials.is_baseline BOOLEAN NOT NULL DEFAULT FALSE` (sentinel marker for the off-band non-Optuna trial). +- Partial unique index `uq_trials_study_baseline_complete ON trials (study_id) WHERE is_baseline = TRUE AND status = 'complete'` (single-complete-baseline-per-study guarantee — see D-16). +- Alembic migration `0020_studies_baseline_trial` adding both columns + the partial index with reversible downgrade + idempotency guards. +- New worker function `run_baseline_trial(ctx, study_id, params)` in `backend/workers/baseline.py` mirroring `run_trial`'s render → search → score → persist shape but without `study.ask()` / `study.tell()`. +- Orchestrator change in `backend/workers/orchestrator.py:start_study` — resolve baseline params via the 4-tier fallback, enqueue `run_baseline_trial`, wait synchronously, stamp `study.baseline_metric` + `study.baseline_trial_id` BEFORE entering the Optuna polling loop. +- Pure-domain resolver `resolve_baseline_params` in `backend/app/domain/study/baseline_resolver.py` implementing the 4-tier fallback (D-2 below). +- One-line change in `backend/app/domain/study/confidence.py:624` to emit `comparison_against = "baseline"` when `baseline_trial_id` resolved AND the row has `per_query_metrics`. +- One-line change in `backend/app/domain/study/auto_followup.py:156` to switch lift computation when `parent.baseline_metric IS NOT NULL`. +- Optional `config.baseline_params: dict[str, str | int | float | bool | None] | None` field on `CreateStudyRequest.config` (operator-supplied override; 3rd tier of the fallback; lives inside `studies.config` JSONB per D-7). +- `StudyDetail.baseline_trial_id` exposure on the `/api/v1/studies/{id}` response. +- Digest system-prompt extension (1–2 sentences) for "regressed vs production baseline" narrative framing. +- Trial-listing UI: `is_baseline` filtering behavior (see §19 OQ-1). +- Test coverage at every layer (unit / integration / contract / E2E). + +### Out of scope + +- Re-running the baseline trial on parameter changes (it's a one-shot at study-create-time). +- Backfilling `baseline_trial_id` / `baseline_metric` for studies created before this feature lands. They stay `NULL` → confidence analytics fall back to runner-up; auto-followup gate falls back to first-decile-max. +- Multi-baseline comparison (e.g., compare against two production configs). Out of MVP1 scope. +- Updating the chat agent's `get_study` / `create_proposal_from_study` tools to expose `baseline_trial_id` beyond what the existing `baseline_metric` propagation already does (already wired — no change). +- Auto-fork detection that picks the parent-study's params automatically without operator input. Today's `feat_auto_followup_studies` already auto-spawns followups; this feature inherits its lineage logic for free via the 4-tier fallback resolver. +- Re-running the baseline when the operator edits the parent template post-study-creation. Studies are immutable post-create per `feat_study_lifecycle`. + +### API convention check + +Verified against [`docs/01_architecture/api-conventions.md`](../../../01_architecture/api-conventions.md) and `backend/app/api/v1/studies.py`: + +- **Endpoint prefix:** `/api/v1/studies` — confirmed at `backend/app/api/v1/studies.py:34` (router prefix). +- **Router namespace:** `backend/app/api/v1/studies.py` (existing — no new router file). +- **HTTP methods:** No new endpoint added by this feature. The change is on the response shape of existing `POST /api/v1/studies` + `GET /api/v1/studies/{id}` + agent tool boundaries. +- **Error envelope (non-auth):** `{"detail": {"error_code": "", "message": "", "retryable": }}` per `api-conventions.md`. Confirmed by reading `backend/app/api/errors.py` and the existing `_err()` helper at `backend/app/api/v1/studies.py:113`. +- **Auth error shape:** N/A — MVP1 has no auth surface per CLAUDE.md release matrix. + +### Phase boundaries + +This feature is a single phase. Phase 1 of `feat_pr_metric_confidence` (the parent feature) shipped 2026-05-21; this feature is its deferred Phase 2 split into its own `/pipeline` lifecycle for tracking discoverability. No further sub-phases. + +## 4) Product principles and constraints + +- **Backward-compatible by construction**: studies created before this migration stay `baseline_trial_id IS NULL` and continue to render `comparison_against = "runner_up"` (FR-4 fallback) and use `first_decile_max` for the auto-followup gate (FR-5 fallback). No backfill. +- **Failed baseline must not fail the study**: the baseline is informational, not load-bearing. A baseline trial that raises (cluster unreachable, query DSL invalid, scorer crash) results in a `Trial` row with `status='failed'`, `baseline_trial_id` stays NULL, the orchestrator logs and proceeds. Confidence + auto-followup gate fall back per FR-4 / FR-5. +- **Operator's "current production behavior" is what they declare it to be**: the 4-tier fallback (D-2) is opinionated. The operator's mental model — "what does this PR CHANGE vs. what's currently live?" — is preserved by routing through `parent_proposal_id` first (the digest-executable followup case) and falling back to template defaults only when nothing else is available. +- **One-line change rule**: the existing `feat_pr_metric_confidence` infrastructure was designed for this extension. Every cross-cutting consumer is data-driven; this feature should NOT need a multi-call cascade through services. +- **Single source of truth for `is_baseline`**: trials carry a boolean flag, not an `optuna_trial_number = -1` sentinel. Reason: Optuna's RDB does not tolerate negative trial numbers (verified via `optuna.study.Study.ask().number` always returning a non-negative int — and the existing `infra_optuna_eval` worker contract uses `study.trials[optuna_trial_number]` which expects non-negative indexes). +- **Baseline trial uses the same engine adapter contract as Optuna trials**: render → `search_batch` → score. No special-case path; just no `study.ask()` / `study.tell()`. This keeps the engine-adapter Protocol clean. +- **Always read settings from `Settings`**: `OPENAI_BASE_URL`, `OPENAI_MODEL`, etc. are never hardcoded (CLAUDE.md Absolute Rule #8). Same for `studies_default_timeout_s` — the baseline trial respects the same per-trial timeout the user's `studies.config.trial_timeout_s` carries. + +### Anti-patterns + +- **Do not** treat `optuna_trial_number = -1` as the *primary* baseline discriminator. The canonical flag is `trials.is_baseline = TRUE`. The `-1` sentinel exists only because the column is `NOT NULL` (column was declared NOT-NULL in `feat_study_lifecycle` Phase 1 migration; making it nullable now would be a backward-incompatible schema change for a single edge). Optuna's RDB never queries the app `trials` table, so the sentinel is purely an app-side filler. Every code path that filters or counts Optuna trials MUST do so by `is_baseline = FALSE`, never by the absence of `optuna_trial_number = -1`. +- **Do not** call `study.ask()` / `study.tell()` from `run_baseline_trial`. The baseline isn't an Optuna trial — it's a recording of a known parameter combination's performance. Tell-ing Optuna about it would either (a) prejudice the TPE sampler with a fixed-seed observation Optuna treats as a prior, or (b) raise on duplicate trial-number registration. **Persist directly to the `trials` table with `is_baseline=true` and skip Optuna entirely.** +- **Do not** make baseline-trial timeout configurable separately from `studies.config.trial_timeout_s`. Operators already tune that knob; a second timeout knob just for baseline is debt with no upside. +- **Do not** block on baseline failure with a hard error. The baseline is informational. If the adapter fails, score raises, or the timeout fires, persist the failed Trial row, leave `baseline_trial_id IS NULL`, log + proceed. +- **Do not** add a separate `baseline_trials` table. The denormalization-vs-normalization debate was settled in `feat_study_lifecycle`: trials are append-only and a `Trial` row is the canonical record. A second table would duplicate the schema and complicate cascade-delete semantics. +- **Do not** modify the existing 5 tests that assert `comparison_against == "runner_up"` to instead assert `"baseline"`. Those tests cover the FR-4 fallback path — they MUST keep failing on regressions to that path. + +## 5) Assumptions and dependencies + +- **Phase 1 (`feat_pr_metric_confidence`)**: ✅ shipped 2026-05-21 (PR #180). Required for `ComparisonAgainst` Literal, `ConfidenceShape`, `compute_study_confidence`, and the per-query-metrics column. +- **`feat_study_lifecycle` Phase 1+2**: ✅ shipped 2026-05-10/11 (PR #18 + #25). Required for the `studies` + `trials` tables, orchestrator, `run_trial` worker, `study_state` service. +- **`feat_digest_executable_followups`**: ✅ shipped 2026-05-24 (PR #225). Provides `studies.parent_proposal_id` + `parent_proposal_followup_index` for the 1st-tier fallback. +- **`feat_auto_followup_studies`**: ✅ shipped 2026-05-24 (PR #223). Provides `studies.parent_study_id` as an MVP1-active field for the 2nd-tier fallback. +- **`infra_adapter_elastic`**: ✅ shipped 2026-05-10 (PR #16). Required for the `SearchAdapter` Protocol the baseline trial uses. +- **`infra_optuna_eval`**: ✅ shipped 2026-05-10 (PR #23). The `score()` + `qrels_loader` infrastructure the baseline trial reuses. +- **`feat_llm_judgments`**: ✅ shipped 2026-05-11 (PR #35). Provides the judgments the baseline trial's qrels come from. + +No external dependencies. No new SaaS accounts. + +## 6) Actors and roles + +- **Primary actor**: relevance engineer (the only user role in MVP1). +- **Role model**: N/A — single-tenant install, no auth surface (per CLAUDE.md release matrix; MVP1-3). +- **Permission boundaries**: every operator can read every study, including its baseline trial. + +### Authorization + +N/A — single-tenant install, no auth surface (MVP1-3 per CLAUDE.md). + +### Audit events + +N/A — `audit_log` lands at MVP2 per [`docs/01_architecture/data-model.md`](../../../01_architecture/data-model.md) §"Reserved for later releases". A future spec note (deferred): when MVP2 lands, the baseline-trial completion + the `baseline_trial_id` stamp are state-mutations on the studies row and should both emit audit events. Capturing here so the MVP2 audit-emission sweep doesn't miss them. + +## 7) Functional requirements + +### FR-1: Migration adds `studies.baseline_trial_id`, `trials.is_baseline`, and a partial unique index +- Requirement: + - The system **MUST** add an Alembic migration `0020_studies_baseline_trial` that: + - Adds `studies.baseline_trial_id String(36) NULL` (not a formal FK — same rationale as `best_trial_id` at `study.py:99-103`). + - Adds `trials.is_baseline BOOLEAN NOT NULL DEFAULT FALSE`. + - Adds a **partial unique index** `uq_trials_study_baseline_complete ON trials (study_id) WHERE is_baseline = TRUE AND status = 'complete'`. This guarantees at most ONE complete baseline trial per study at the DB level — defense against the resume-race scenario where two orchestrator invocations both enqueue baseline jobs for the same study (per D-16). A second concurrent INSERT raises `IntegrityError`, which the worker catches, treats as "another worker already inserted", and exits cleanly without re-stamping. + - Round-trips cleanly via `downgrade()` — drop index, drop column on trials, drop column on studies. + - Uses idempotency guards (`DO $$ BEGIN ... IF NOT EXISTS ... END $$`) on every `ALTER TABLE` / `CREATE INDEX` so re-runs are no-ops. +- Notes: No backfill. Existing studies stay `baseline_trial_id IS NULL`. Existing trials stay `is_baseline=FALSE`. The partial unique index applies only to NEW baseline trials (existing trial rows all have `is_baseline=FALSE` and don't match the index predicate). + +### FR-2: Orchestrator runs a single non-Optuna baseline trial before Optuna starts +- Requirement: + - The system **MUST** insert a new phase in `backend/workers/orchestrator.py:start_study` between section "C. Parse search_space" (line 170) and section "D. Polling loop" (line 173) that: + 1. Resolves baseline params via `resolve_baseline_params(db, study)` (FR-3). + 2. If `params` is `None`, skip baseline entirely (preserve current behavior); log `event_type="baseline_skipped"` with reason. + 3. Else, generate a fresh `trial_id` (UUIDv7) and enqueue with a deterministic Arq job-id keyed on study: `arq_pool.enqueue_job("run_baseline_trial", study_id, trial_id, params, _job_id=f"baseline:{study_id}")`. The `_job_id` prevents Arq from accepting a duplicate enqueue if `resume_study` fires a second baseline-create before the first has landed an INSERT. When `enqueue_job` returns `None` (Arq rejected as duplicate), the orchestrator logs `event_type="baseline_enqueue_deduped"` and proceeds to the wait phase — the original enqueue's worker will land the row, and on observed `is_baseline=TRUE, status='complete'` row, FR-12 stamps it from whichever path observes it first. + 4. Wait for the baseline trial to terminal-out by polling the `trials` table for the row at `(study_id, trial_id, is_baseline=TRUE)` with `status IN ('complete', 'failed')`. Use a fresh session per poll tick (mirror the polling-loop pattern at orchestrator.py:182-188). Tick interval: 1 second (same `_REPLENISH_TICK_S`). + 5. Wait timeout: `wait_s = min(600, max(60, (study.config.trial_timeout_s or settings.studies_default_timeout_s) + 30))`. Floor 60s, ceiling 600s (10 min — long enough for slow engines, short enough that operators notice). For `trial_timeout_s ≤ 570s` (the common case), `wait_s` exceeds the worker's own per-trial timeout by ≥30s so the trial completes naturally before the wait gives up. For `trial_timeout_s > 570s` (rare; the `StudyConfigSpec` bound is 5..3600), the wait deliberately gives up before the worker — this is intentional: operators with extreme per-trial timeouts get the Optuna phase started promptly, and the worker self-stamps via FR-10 if the baseline eventually completes (per D-13). + 6. On timeout: log `event_type="baseline_wait_timeout"`, leave `baseline_trial_id IS NULL` (the worker will self-stamp later via FR-10 if it eventually completes), proceed to Optuna phase. The orchestrator does NOT attempt to cancel the in-flight baseline job — the worker is short-lived and the late stamp is correct behavior (operators get baseline data eventually even when the orchestrator gave up waiting; the Optuna loop is already running). + 7. On terminal row: if `status='complete'`, stamp `study.baseline_metric = trial.primary_metric` and `study.baseline_trial_id = trial.id`, commit. If `status='failed'`, log `event_type="baseline_failed"` with the trial's `error` text, leave `baseline_trial_id IS NULL`, proceed. + - The orchestrator **MUST** continue to the Optuna polling loop regardless of baseline outcome. +- Notes: Synchronous wait, NOT parallel-with-Optuna. Rationale (D-4 below): the baseline is a one-shot fast trial (~1-5s); Optuna can wait. Running them in parallel risks both ending up `running` at the same time, which complicates UI ordering + leaks the baseline into the Optuna trial counter at the polling phase's first read. + +### FR-3: Baseline-params resolution via 4-tier fallback +- Requirement: + - The system **MUST** provide a pure-domain function `resolve_baseline_params(db, study) -> dict[str, Any] | None` in `backend/app/domain/study/baseline_resolver.py` (NOTE: takes `db` for the parent-row lookup; the function is async but pure of business logic — no service-layer side effects). Resolution order: + 1. **(d) Parent-proposal config** — if `study.parent_proposal_id IS NOT NULL`: load the parent proposal's `study_trial_id` (the best trial of the parent study). Return that trial's `params` dict. If the parent trial is missing/deleted, log `event_type="baseline_resolve_parent_proposal_missing"` and fall through to tier (c). + 2. **(c) Parent-study winner** — if `study.parent_study_id IS NOT NULL`: load the parent study and look up the trial at `parent.best_trial_id`. Return that trial's `params`. If missing, log + fall through to (b). + 3. **(b) Operator-supplied** — if `study.config["baseline_params"]` is set (operator passed `baseline_params` in the create-study request body), return it directly. Schema-level validation at `CreateStudyRequest` time guarantees the dict shape; the resolver does NOT re-validate against the search-space — that's by design (an operator may want to baseline against a config that isn't in the current study's search space, e.g. their actual production config). + 4. **(a) Template defaults** — return the deterministic middle-of-range for each declared param in `study.search_space.params`: + - `FloatParam` → `(low + high) / 2.0` (with log-scale geometric mean when `log=true`: `sqrt(low * high)`). + - `IntParam` → `(low + high) // 2` (Python integer division — picks the lower midpoint when the range is even-cardinality). + - `CategoricalParam` → `choices[(len(choices) - 1) // 2]` (median index — picks the **lower** midpoint when even-cardinality; e.g., for `['a','b','c','d']` returns `'b'`). + 5. If after all four tiers the resolver returns `{}` (the template has no declared params), return `None` — no baseline trial runs. + - The resolver **MUST** be invoked from `start_study` BEFORE the Optuna loop, AFTER `SearchSpace.model_validate` (orchestrator.py:170). +- Notes: Pure-domain so it's independently unit-testable. The async signature is needed because tiers (c) and (d) hit `repo.get_trial` / `repo.get_study` / `repo.get_proposal`. + +### FR-4: `compute_study_confidence` switches comparison source when baseline trial available +- Requirement: + - The system **MUST** modify `backend/app/domain/study/confidence.py:compute_study_confidence` so that when: + - `study.baseline_trial_id IS NOT NULL`, AND + - the corresponding trial row exists, AND + - that trial has non-empty `per_query_metrics`, + the per-query-outcomes comparison source becomes the baseline trial; `comparison_against = "baseline"`. + - Otherwise (baseline missing / failed / row deleted / missing per_query_metrics): fall back to runner-up #2 (Phase 1 behavior); emit `comparison_against = "runner_up"`. + - The function signature **MAY** add a new keyword-only argument `baseline_trial: Any | None = None` (paired with `runner_up_trial`). Callers (`backend.app.services.study_confidence.fetch_study_confidence`) pre-fetch the baseline trial in the same 4-query read pattern's Q-2 sibling. +- Notes: The single-line `comparison_against="runner_up"` literal at confidence.py:624 becomes a conditional. The `per_query_outcomes` block is otherwise unchanged. No API contract break — `ConfidenceShape.per_query_outcomes.comparison_against` is already typed `Literal["runner_up", "baseline"]`. + +### FR-5: Auto-followup gate switches to lift-over-baseline when parent has baseline +- Requirement: + - The system **MUST** modify `backend/app/domain/study/auto_followup.py:evaluate_chain_gate` to take a new `direction: Literal["maximize", "minimize"]` argument (defaulting to `"maximize"` for backward compat; caller passes `parent.objective["direction"]`). The lift computation becomes direction-aware: + - **Maximize** direction (existing default — every MVP1 study today is maximize per inspection of `feat_study_lifecycle` examples): + - If `parent.baseline_metric IS NOT NULL`: `lift = parent.best_metric - parent.baseline_metric`. + - Otherwise: `lift = parent.best_metric - first_decile_max` (existing implicit-baseline behavior). + - **Minimize** direction: signs flip — `lift = baseline_metric - parent.best_metric` (or `first_decile_min - parent.best_metric` for the implicit-baseline fallback; the `compute_first_decile_max` helper **MUST** also get a direction-aware sibling `compute_first_decile_extremum(complete_trials, direction)`). + - The gate decision (`lift > epsilon` → ENQUEUE) stays unchanged — the lift value itself is always normalized so "better than baseline" is positive. + - The `ChainGateOutcome.first_decile_max` field **MUST** be renamed to `first_decile_extremum` (forward-only rename; no callers depend on the field name yet — verified: only `evaluate_chain_gate` constructs `ChainGateOutcome`, only the test suite reads it). + - The module docstring **MUST** be updated to reflect that FR-2b is now ACTIVE: delete the "When feat_study_baseline_trial ships..." sentence; replace with "FR-2b activated: when `parent.baseline_metric IS NOT NULL`, lift is computed directly against the baseline. Direction-aware via the `direction` argument (added 2026-05-25)." +- Notes: This is the "one-line change" the existing module docstring at `auto_followup.py:9-11` promised, plus the GPT-5.5-finding direction-awareness fix. Direction-awareness closes a latent bug in the Phase 1 `feat_auto_followup_studies` shipment that this feature is uniquely positioned to fix (because we're touching the same lines anyway). + +### FR-6: `CreateStudyRequest` accepts optional `baseline_params` +- Requirement: + - The system **MUST** add `config.baseline_params: dict[str, Any] | None = None` to the `StudyConfigSpec` schema (the nested `config` field of `CreateStudyRequest`). Stored as-is in `studies.config` JSONB. + - The schema **MUST NOT** validate `baseline_params` against the current study's search space at create time — operators may legitimately want to baseline against a production config that's outside the current search space (e.g., to demonstrate the search space's win over a known-good config). + - The schema **MUST** type `baseline_params` as `dict[str, str | int | float | bool | None] | None`. The discriminated value type forbids nested dicts / arrays — Pydantic enforces this at parse time and emits a `VALIDATION_ERROR` (422) on violation. The OpenAPI schema generated from this Pydantic type carries the constraint, and the frontend's `dict` type-narrows correctly. +- Notes: `baseline_params` lands in `studies.config` (not a top-level column) because every study-tunable in MVP1 lives in `studies.config` and we don't want to grow the top-level schema for a per-study optional field. + +### FR-7: Digest narrative system prompt extension +- Requirement: + - The system **MUST** update `prompts/digest_narrative.system.md` to add a 1-2 sentence narrative-framing guideline: + + > When `` has `comparison_against = "baseline"`, regressors should be described as "regressed vs the operator's current production baseline" — not "vs the runner-up trial". Lead the narrative with this baseline framing when present, since it answers the approver's "does this change PROD?" question directly. + +- Notes: No code change; prompt-only edit. Existing tests that snapshot the system prompt may need a fixture refresh (see §14 test strategy). + +### FR-8: `StudyDetail.baseline_trial_id` exposure +- Requirement: + - The system **MUST** add `baseline_trial_id: str | None` to the `StudyDetail` Pydantic schema at `backend/app/api/v1/schemas.py:668-698`, populated by `studies.py:_detail` at line 121. + - The `TrialDetail` schema at `backend/app/api/v1/schemas.py:724-737` **MUST** add `is_baseline: bool` so the frontend can render the badge / filter behavior described in OQ-1. +- Notes: Forward-only — no migration story for downstream consumers. The frontend types regenerate from the FastAPI OpenAPI schema. + +### FR-9: Frontend trial-listing filters baseline trials by default +- Requirement: + - The trial-listing UI (`ui/src/components/studies/trials-table` or wherever the table lives — see §14 verification) **MUST** filter out `is_baseline=true` rows from the default view. + - A new toggle / chip "Show baseline trial" **MUST** be available; when enabled, the baseline trial appears at the top of the table with a distinct "Baseline" badge. + - The default filtered state is the unsurprising behavior: the baseline trial is a single one-shot that doesn't represent Optuna's exploration, so showing it inline with 100+ Optuna trials would confuse the trial-number ordering. +- Notes: This is the resolution of OQ-1 (default = filter out). + +### FR-11: Downstream consumers MUST filter `is_baseline=FALSE` from Optuna-trial-counting paths + +- Requirement: + - The following code paths today aggregate or select from the `trials` table and assume every row is an Optuna trial. Each **MUST** filter `WHERE is_baseline = FALSE` after this feature lands: + 1. `backend/app/db/repo/trial.py:aggregate_trials_summary` — used by the orchestrator's stop-condition check (`_stop`), the `StudyDetail.trials_summary` API field, and the digest worker's "top trials" computation. Filtering on `is_baseline=FALSE` ensures `summary.total`, `summary.complete`, `summary.best_primary_metric`, and `summary.best_trial_id` describe ONLY the Optuna trials (the baseline appears under its own surface). + 2. `backend/app/db/repo/trial.py:list_complete_trials_for_confidence` (or however the 4-query read pattern's Q2 is named — see `backend/app/services/study_confidence.py:fetch_study_confidence`) — the `complete_trials_summary` input to `compute_study_confidence` MUST exclude baseline. Including the baseline would corrupt `runner_up_gap`, `convergence`, and `late_trial_stddev` aggregates. + 3. `backend/app/db/repo/trial.py:list_top_trials` (digest worker's top-10 list at `backend/workers/digest.py:_compute_top_trials`) — operators read this as "the top Optuna trials"; baseline appearing inline would conflate the two surfaces. + 4. Parameter-importance computation (`optuna.importance.get_param_importances`) — operates on Optuna's RDB, not the app trials table, so already safe by construction. Documented for completeness. + 5. `auto_followup.compute_first_decile_extremum` (renamed from `compute_first_decile_max` per FR-5) — already takes its iterable input from the caller. Caller MUST filter `is_baseline=FALSE` when fetching. + - The orchestrator's `_last_n_all_failed` and `_last_n_all_zero` helpers (`backend/workers/orchestrator.py:320-371`) use `ORDER BY Trial.optuna_trial_number DESC LIMIT n`. In steady state (≥ N Optuna trials present), the `is_baseline=TRUE` row at `optuna_trial_number=-1` sorts last and never enters the window. **However**, during the brief window between baseline-trial completion and the first Optuna trial reaching terminal, the only matching row could be the baseline (because the helpers don't filter on status). **MUST add `WHERE is_baseline = FALSE` to both helpers** to prevent a failed baseline from triggering a spurious "5 consecutive failures" alert before any Optuna trial has even run. +- Notes: Each repo helper either adds the filter inline or accepts a new `include_baseline: bool = False` kwarg. The inline filter is preferred — operators never want baseline in these aggregates, so the kwarg is unused complexity. + +### FR-12: Single stamping helper `services.study_state.stamp_baseline_trial` + +- Requirement: + - The system **MUST** add a service function `services.study_state.stamp_baseline_trial(db, study_id, trial_id, primary_metric) -> bool` that: + 1. Loads the candidate `Trial` row by `id = trial_id`; raises `BaselineTrialNotFound` if missing. + 2. Asserts `trial.study_id == study_id` and `trial.is_baseline == TRUE` and `trial.status == 'complete'`; raises `InvalidBaselineTrialState` on violation. + 3. Executes idempotent UPDATE: `UPDATE studies SET baseline_trial_id = $1, baseline_metric = $2 WHERE id = $3 AND baseline_trial_id IS NULL`. + 4. Returns `True` if a row was updated (this caller stamped), `False` if a sibling already stamped (race-tolerant — the return value is informational, not load-bearing). + 5. Commits the transaction (or leaves commit to the caller — locked in Story 1.4 implementation, defaulting to leave-to-caller for the existing `study_state` precedent at `services/study_state.py`). + - The orchestrator (FR-2 step 7), `resume_study` (FR-2 implied via idempotency notes in §9), and `run_baseline_trial` (FR-10 step 7) **MUST** all stamp through this helper. No direct `UPDATE studies SET baseline_trial_id = ...` statements outside this helper. +- Notes: Mirrors the existing `services.study_state.complete_study` pattern — single chokepoint for state-mutation, easy unit-testable, easy to add audit_log emission at MVP2. + +### FR-10: `run_baseline_trial` worker function +- Requirement: + - The system **MUST** ship a new Arq job `run_baseline_trial(ctx, study_id, trial_id, params)` in `backend/workers/baseline.py` that: + 1. Loads the `Study` row + cluster + template + queries + qrels (same lookups as `run_trial`). + 2. Builds the adapter via `build_adapter(cluster)`. + 3. Renders the template via `adapter.render(template, params, q.query_text)` for each query. + 4. Calls `adapter.search_batch(target, native_queries, top_k, strict_errors=False, timeout=trial_timeout_s)`. + 5. Scores via `score(qrels, run_dict, metrics_set)` (same metric set as `run_trial`). + 6. Persists a `Trial` row with `is_baseline=TRUE` and `optuna_trial_number = -1` (NOT-NULL sentinel filler — see §4 Anti-patterns; the canonical discriminator is `is_baseline=TRUE`). + 7. On `status='complete'`: BEFORE returning, the worker **MUST** call the new service helper `services.study_state.stamp_baseline_trial(db, study_id, trial_id, primary_metric)` (FR-12) to durably stamp `studies.baseline_trial_id = trial_id` and `studies.baseline_metric = primary_metric`. The stamp is idempotent via `WHERE baseline_trial_id IS NULL` — if the orchestrator already stamped (fast path), the worker UPDATE is a no-op. This covers the late-completion case where the orchestrator's wait phase timed out but the worker eventually succeeded. + 8. Returns normally on success or persisted failure (Arq treats as success unless infrastructure raised). + - The worker **MUST** use `trial_id`-based idempotency (NOT `(study_id, optuna_trial_number)`): on entry, check for an existing terminal `trials` row with `id = trial_id` (the orchestrator pre-generates the UUIDv7 in FR-2 and passes it as a job argument). If found, no-op and return. Rationale: `(study_id, -1)` is a poor uniqueness key because the orchestrator may re-enqueue baseline trials on retry — but `trial_id` is uniquely generated per orchestrator-decision-point. + - The worker **MUST** be registered in `backend/workers/main.py` `WorkerSettings.functions`. +- Notes: The worker is `~80 LOC` (smaller than `run_trial` — no Optuna interaction, no reconciliation paths, no consecutive-failure tracking). + +## 8) API and data contract baseline + +### 8.1 Endpoint surface + +No new endpoints. Existing surfaces gain new response fields: + +| Method | Path | Response field added | Purpose | +|---|---|---|---| +| `POST` | `/api/v1/studies` | request body gains `config.baseline_params: dict \| None`; response gains `baseline_trial_id: string \| null` | FR-6 + FR-8 | +| `GET` | `/api/v1/studies/{id}` | response gains `baseline_trial_id: string \| null` | FR-8 | +| `GET` | `/api/v1/studies/{id}/trials` | each row gains `is_baseline: bool` | FR-8 | + +No new error codes introduced. + +### 8.2 Contract rules + +- Existing `ConfidenceShape.per_query_outcomes.comparison_against` Literal already includes both `"runner_up"` and `"baseline"` wire values — no contract break. +- `StudyDetail.baseline_trial_id` is nullable; clients **MUST** tolerate `null` and **MUST NOT** assume the FK target exists (denormalized, not enforced — same rationale as `best_trial_id`). +- `TrialDetail.is_baseline` is non-nullable bool (DB default `FALSE`). + +### 8.3 Response examples + +`GET /api/v1/studies/{id}` success (post-baseline-trial): + +```json +{ + "id": "0192f24c-bce0-7e58-a6e8-b9c6a4def888", + "name": "boost-titles-tune", + "cluster_id": "0192f24c-bce0-7000-...", + "target": "products", + "template_id": "0192f24c-bce0-7100-...", + "query_set_id": "0192f24c-bce0-7200-...", + "judgment_list_id": "0192f24c-bce0-7300-...", + "search_space": {"params": {"boost_title": {"type": "float", "low": 0.5, "high": 10.0}}}, + "objective": {"metric": "ndcg", "k": 10, "direction": "maximize"}, + "config": {"max_trials": 100, "baseline_params": {"boost_title": 1.5}}, + "status": "completed", + "failed_reason": null, + "optuna_study_name": "0192f24c-bce0-7e58-a6e8-b9c6a4def888", + "parent_study_id": null, + "baseline_metric": 0.512, + "baseline_trial_id": "0192f24c-bce0-7500-...", + "best_metric": 0.671, + "best_trial_id": "0192f24c-bce0-7800-...", + "created_at": "2026-05-25T10:00:00Z", + "started_at": "2026-05-25T10:00:01Z", + "completed_at": "2026-05-25T10:08:32Z", + "trials_summary": {"total": 100, "complete": 98, "failed": 2, "pruned": 0, "best_primary_metric": 0.671}, + "confidence": { + "headline": {"metric": "ndcg", "value": 0.671, "k": 10, "n_queries": 200}, + "ci_95": {"low": 0.652, "high": 0.689, "method": "bootstrap_n1000", "n_samples": 200}, + "runner_up_gap": {"value": 0.012, "classification": "robust_plateau", "top10_within": 0.004, "runner_up_metric": 0.659}, + "late_trial_stddev": {"value": 0.008, "window_size": 20, "min_window_required": 10}, + "convergence": {"best_at_trial": 42, "total_trials": 100, "regime": "early_held"}, + "per_query_outcomes": { + "improved": 137, + "unchanged": 51, + "regressed": 12, + "comparison_against": "baseline", + "top_regressors": [ + {"query_id": "0192f24c-bce0-7900-...", "query_text": "red shoes", "winner_score": 0.41, "comparison_score": 0.78, "delta": -0.37} + ] + } + } +} +``` + +Non-auth failure example (from `backend/app/api/errors.py`): + +```json +{ + "detail": { + "error_code": "VALIDATION_ERROR", + "message": "baseline_params must contain JSON-serializable primitives", + "retryable": false + } +} +``` + +Auth failure example: N/A — MVP1 has no auth. + +### 8.4 Enumerated value contracts + +| Field | Accepted values (exact) | Backend source of truth | Frontend call site(s) | +|---|---|---|---| +| `confidence.per_query_outcomes.comparison_against` | `runner_up`, `baseline` | `backend/app/domain/study/confidence.py:114` (`ComparisonAgainst` Literal) | `ui/src/lib/enums.ts:83-87` (`COMPARISON_AGAINST_VALUES`); rendered in `ui/src/components/studies/confidence-panel.tsx:98,113` | +| `trials.is_baseline` | `true`, `false` (bool) | `backend/app/db/models/trial.py` (new column, FR-1) | `ui/src/lib/types.ts` (regenerated); filter chip in trials-table (FR-9) | + +No new wire values introduced. + +### 8.5 Error code catalog + +No new error codes. The existing `VALIDATION_ERROR` (422) covers malformed `baseline_params` payloads. + +## 9) Data model and state transitions + +### New/changed entities + +**Modified table: `studies`** +- Add `baseline_trial_id String(36) NULL` — denormalized "FK" to the baseline trial in this study (matches the `best_trial_id` pattern at study.py:99-103; not an enforced FK because the row is stamped post-creation by the orchestrator). + +**Modified table: `trials`** +- Add `is_baseline BOOLEAN NOT NULL DEFAULT FALSE` — marker so the trials-listing UI / Optuna-RDB-join code paths can filter the baseline out. + +**No new tables.** + +### Required invariants + +- For every `studies` row with `baseline_trial_id IS NOT NULL`, the referenced `trials` row exists, has `study_id = studies.id`, `is_baseline = TRUE`, AND `status = 'complete'`. Enforced at write time by the FR-12 service helper (single stamping path; no DB-level constraint because the FK isn't formal). +- For every `trials` row with `is_baseline = TRUE` AND `status = 'complete'`, the corresponding `studies` row converges to `baseline_trial_id = trials.id` once a stamping path runs (orchestrator fast-path in FR-2 OR worker self-stamp in FR-10). Briefly inconsistent during the wait phase of FR-2; converges within `_REPLENISH_TICK_S` after the trial reaches terminal OR at the worker's own commit time, whichever fires first. **Failed baseline trials (`is_baseline=TRUE AND status='failed'`) intentionally violate this — `baseline_trial_id` stays NULL by design per §4 principle "failed baseline must not fail the study".** +- A study has at most ONE **complete** baseline trial — enforced at the DB level by the partial unique index `uq_trials_study_baseline_complete` (FR-1). Failed baseline trials may coexist (e.g., one failure followed by a successful retry from a re-run, out of scope today); the partial index's `status = 'complete'` predicate scopes uniqueness to the canonical-success case only. **Resume-race guarantee**: orchestrator double-enqueue is prevented at three layers — (a) `_job_id=f"baseline:{study_id}"` Arq deduplication (FR-2 step 4); (b) the partial unique index on `(study_id) WHERE is_baseline AND status='complete'` (FR-1); (c) the FR-12 stamping helper's `WHERE baseline_trial_id IS NULL` predicate. Any one of the three would prevent corruption; all three in series make it impossible. +- Baseline `Trial` rows have `optuna_trial_number = -1` and `is_baseline = TRUE`. Optuna's RDB never queries these (it uses its own storage); any join from app `trials` to Optuna's RDB MUST filter `WHERE is_baseline = FALSE`. + +### State transitions + +No new state machines. The `Trial` rows for baseline use the existing `complete | failed | pruned` enum (CHECK constraint already in place). + +### Idempotency/replay behavior + +- The baseline-trial enqueue uses an explicit `trial_id` (UUIDv7) generated by the orchestrator. If `run_baseline_trial` retries due to Arq infrastructure failure (e.g., DB unreachable mid-INSERT), the worker checks for `(study_id, trial_id, is_baseline=TRUE)` and no-ops on existing terminal rows (mirrors the FR-1a clause in `run_trial`). +- The orchestrator's wait phase is idempotent — re-entering `start_study` for a study that already has a stamped `baseline_trial_id` skips FR-2 entirely and proceeds directly to the Optuna phase. +- A `resume_study` invocation after worker restart: + - If a `Trial` row exists with `is_baseline=TRUE AND status='complete'` AND `baseline_trial_id IS NULL`: call `services.study_state.stamp_baseline_trial` (FR-12 enforces the precondition checks; idempotent via the `WHERE baseline_trial_id IS NULL` predicate). + - If only failed/pruned baseline rows exist: skip baseline (per §4 principle; do NOT attempt a retry) and proceed to Optuna. + - If no baseline row exists at all: run baseline from scratch via FR-2. The deterministic Arq `_job_id=f"baseline:{study_id}"` guarantees that re-enqueue is a no-op when an original baseline job is still queued/running (eliminating the double-baseline race that would otherwise occur if the orchestrator crashed between `enqueue_job` and `INSERT INTO trials`). + +## 10) Security, privacy, and compliance + +- **Threats**: + - T1: `baseline_params` accepts arbitrary dict — could be used to log PII or secrets. **Mitigation**: Pydantic schema constrains to JSON primitives; the field is stored in `studies.config` JSONB, which already absorbs every other config knob. No NEW exposure. + - T2: Baseline trial makes the same engine queries as Optuna trials — could leak unprivileged data if the operator has misconfigured cluster auth. **Mitigation**: Existing engine-adapter auth is unchanged; this feature does NOT add a new auth path. Same engine, same target, same query template. + - T3: A long-running baseline trial could DoS the worker pool. **Mitigation**: FR-2 caps the orchestrator's wait at `min(600, max(60, trial_timeout_s + 30))` seconds (formula in FR-2 step 5); the worker honors `studies.config.trial_timeout_s` directly. Failed baselines don't fail the study (principle in §4); late completions self-stamp via FR-10 + FR-12 without blocking Optuna. +- **Controls**: All existing controls (per-trial timeout, qrels-loader sanitization, prompt redaction in logs) apply unchanged. +- **Secrets/key handling**: N/A — no new secrets. +- **Auditability**: N/A in MVP1 (audit_log lands at MVP2). Logged events: `baseline_skipped`, `baseline_failed`, `baseline_wait_timeout`, `baseline_stamped` — structlog only, not audit_log. +- **Data retention**: Baseline trial rows persist for the lifetime of the study (cascade-delete on study delete, same as Optuna trials). + +## 11) UX flows and edge cases + +### Information architecture + +- **Navigation placement**: No new navigation. All UX changes live in the existing study-detail page (`ui/src/app/studies/[id]/page.tsx`), the existing trials-table component, and the existing ConfidencePanel. +- **Labeling taxonomy**: + - "Baseline trial" — the new is_baseline=TRUE row in the trials list. + - "vs baseline" — the new ConfidencePanel label replacing "vs runner-up" when the data flips. +- **Content hierarchy**: ConfidencePanel renders the comparison label inline next to the per-query outcomes counts (lines 98 + 113 in confidence-panel.tsx) — no layout change. +- **Progressive disclosure**: Trial-listing UI defaults to filtering out the baseline row (FR-9). A "Show baseline trial" toggle/chip reveals it at the top of the table. +- **Relationship to existing pages**: Extends. No new pages. + +### Tooltips and contextual help + +| Element | Tooltip / help text | Trigger | Placement | +|---------|-------------------|---------|-----------| +| ConfidencePanel "vs baseline" label | "Compared against your production baseline — the no-tuning trial run with your declared `baseline_params` (or template defaults) before Optuna started." (`confidence.comparison_against` — UPDATE existing glossary entry) | `focus` on the InfoTooltip icon | `top` | +| ConfidencePanel "vs runner-up" label | Existing text retained: "Compared against the runner-up: the second-best trial in this study. Useful when no production baseline was configured." | `focus` | `top` | +| Trials-table "Baseline" badge | "A single non-Optuna trial run before Optuna started. Used as the comparison reference for the confidence outcomes." (new glossary key `trials.is_baseline`) | `hover` on the badge | `right` | +| Trials-table "Show baseline trial" toggle | "Show the one-time baseline trial inline with Optuna trials. Hidden by default because its trial number (-1) doesn't fit the Optuna sequence." | `hover` on the toggle | `top` | +| `baseline_params` form field (if surfaced in `feat_create_study_search_space_builder` modal — OQ-2) | "Optional: explicit params for the baseline trial. Leave blank to use the parent study's winner (or template defaults if no parent)." | inline helper | below field | + +**Glossary keys to add to `ui/src/lib/glossary.ts`:** + +- `trials.is_baseline` — new entry. +- `confidence.comparison_against` — REPLACE existing entry (line 676) with bi-state text that explains both wire values. + +### Primary flows + +1. **Operator creates a study with no `baseline_params`** (the most common case for studies forked from a digest followup): orchestrator resolves tier (d) parent_proposal config → enqueues baseline trial → waits → stamps `baseline_metric` + `baseline_trial_id` → Optuna runs → digest renders with `comparison_against="baseline"`. PR body shows `delta_pct`. Operator sees per-query regressors against THEIR PRODUCTION CONFIG. +2. **Operator manually creates a study with explicit `baseline_params`**: same as above but tier (b) fires. The resolved params are the operator's literal dict (e.g., `{"boost_title": 1.0}`). +3. **Operator creates a study with NO parent + NO baseline_params**: tier (a) fires (template defaults — middle-of-range). The baseline trial runs against the deterministic middle of every declared param. + +### Edge/error flows + +- **Baseline trial fails** (cluster unreachable, scorer crash, timeout): `Trial` row written with `status='failed'`, `is_baseline=TRUE`, `error=`. `baseline_trial_id IS NULL`. Orchestrator proceeds. ConfidencePanel + auto-followup gate fall back to runner-up / first-decile-max. +- **Operator-supplied `baseline_params` contains a key NOT in the template's `declared_params`**: the `adapter.render()` call ignores extraneous keys (verified at the adapter Protocol level — render's contract is "use what's declared"). No error. +- **Operator-supplied `baseline_params` is missing a key the template requires**: `adapter.render()` raises `KeyError` or similar template-render error. The baseline trial fails with a clear error message. Treated like any other baseline failure (fall back). The operator can re-run the study with a fixed `baseline_params`. +- **`parent_proposal.study_trial_id` points at a deleted trial** (cascade race): tier (d) resolver logs + falls through to tier (c) → tier (b) → tier (a). No hard error. +- **Worker restart mid-baseline-trial**: `resume_study` re-enters orchestrator; if `baseline_trial_id IS NULL` AND no `is_baseline=TRUE` trial row exists for this study, baseline is re-run. If a row exists, the orchestrator just re-stamps (idempotent). +- **Empty `declared_params`** (template has zero params): resolver returns `None`; baseline skipped. + +## 12) Given/When/Then acceptance criteria + +### AC-1: orchestrator runs baseline trial first when `parent_proposal_id` is set +- Given a parent proposal exists with a known `study_trial_id` pointing at a trial with `params={"boost_title": 2.0}` +- And the operator creates a study with `parent.proposal_id = , parent.followup_index = 0` +- When `start_study` runs +- Then the first row inserted into `trials` for this study has `is_baseline=TRUE`, `params={"boost_title": 2.0}`, `optuna_trial_number=-1` +- And `studies.baseline_trial_id` is set to that trial's ID +- And `studies.baseline_metric` is set to that trial's `primary_metric` +- And the row is inserted BEFORE any Optuna trial is enqueued. + +### AC-2: orchestrator falls back to template defaults when no parent + no `baseline_params` +- Given a study has `parent_study_id=NULL`, `parent_proposal_id=NULL`, `config={"baseline_params": null}` (or absent) +- And `search_space.params = {"boost_title": {"type": "float", "low": 0.5, "high": 10.0}, "operator": {"type": "categorical", "choices": ["and", "or"]}}` +- When `start_study` runs +- Then the baseline trial's `params = {"boost_title": 5.25, "operator": "and"}` (middle of range for float `(0.5 + 10.0) / 2 = 5.25`; lower-midpoint categorical via `(len-1) // 2 = 0` → `"and"`) +- And `studies.baseline_trial_id` is stamped on completion. + +### AC-3: failed baseline does NOT fail the study +- Given a study is created +- And the engine adapter raises `ClusterUnreachableError` on the baseline trial's `search_batch` +- When `start_study` runs +- Then a `Trial` row exists with `is_baseline=TRUE, status='failed', error='cluster unreachable:
'` +- And `studies.baseline_trial_id IS NULL` +- And `studies.baseline_metric IS NULL` +- And the Optuna polling loop proceeds and trials run normally. + +### AC-4: confidence analytics switch to baseline comparison when baseline is set +- Given a study has `baseline_trial_id` set to a complete trial with `per_query_metrics={"q1": {"ndcg@10": 0.4}, "q2": {"ndcg@10": 0.6}}` +- And the winner trial has `per_query_metrics={"q1": {"ndcg@10": 0.7}, "q2": {"ndcg@10": 0.5}}` +- And the per-query `ndcg` threshold is `0.01` (from `REGRESSOR_THRESHOLDS` at `backend/app/domain/study/confidence.py:61-67` — shipped in Phase 1, `feat_pr_metric_confidence` D-2) +- When `compute_study_confidence` runs +- Then `per_query_outcomes.comparison_against == "baseline"` +- And `per_query_outcomes.improved == 1` (q1: 0.7 vs 0.4, +0.3 > threshold) +- And `per_query_outcomes.regressed == 1` (q2: 0.5 vs 0.6, -0.1 < -threshold). + +### AC-5: confidence falls back to runner-up when baseline_trial_id is NULL +- Given a study has `baseline_trial_id=NULL` +- And the runner-up trial has `per_query_metrics` populated +- When `compute_study_confidence` runs +- Then `per_query_outcomes.comparison_against == "runner_up"`. + +### AC-6: confidence falls back to runner-up when baseline trial has no per_query_metrics +- Given a study has `baseline_trial_id` set BUT the referenced trial has `per_query_metrics IS NULL` (e.g., the baseline failed mid-score) +- When `compute_study_confidence` runs +- Then `per_query_outcomes.comparison_against == "runner_up"`. + +### AC-7: auto-followup gate uses baseline_metric when set +- Given parent study has `best_metric=0.65, baseline_metric=0.55` +- When `evaluate_chain_gate(parent, complete_trials, epsilon=0.005)` runs +- Then `outcome.lift == 0.10` (computed as best_metric - baseline_metric, not first_decile_max) +- And `outcome.decision == ChainGateDecision.ENQUEUE` (lift > epsilon). + +### AC-8: auto-followup gate falls back to first-decile-max when baseline_metric is NULL +- Given parent study has `best_metric=0.65, baseline_metric=NULL, objective.direction="maximize"` +- And `compute_first_decile_extremum(complete_trials, direction="maximize") == 0.45` +- When `evaluate_chain_gate(parent, complete_trials, direction="maximize")` runs +- Then `outcome.lift == 0.20` (computed against first decile, existing FR-2a behavior) +- And `outcome.first_decile_extremum == 0.45`. + +### AC-9: `StudyDetail` response exposes `baseline_trial_id` +- Given a study has `baseline_trial_id="0192...-7500"` +- When `GET /api/v1/studies/{id}` is called +- Then the response body contains `"baseline_trial_id": "0192...-7500"` (or `null` when not set). + +### AC-10: trials-listing UI hides baseline row by default +- Given a study has 100 Optuna trials + 1 baseline trial +- When the operator opens `/studies/{id}` and the trials table loads +- Then 100 rows are visible +- And no row shows `optuna_trial_number=-1` +- When the operator clicks "Show baseline trial" toggle +- Then 101 rows are visible +- And the baseline row appears at the top with a "Baseline" badge. + +### AC-11: digest user prompt renders correct comparison_against +- Given a study has `baseline_trial_id` set and both winner + baseline have `per_query_metrics` +- When the digest worker renders the user prompt +- Then the `` block contains `comparison_against: baseline`. + +### AC-12: PR body emits "vs baseline" in confidence section +- Given a study has confidence with `comparison_against="baseline"` and 12 regressors +- When `open_pr` worker renders the PR body +- Then the body contains a line like `12 regressed (vs baseline)`. + +### AC-13: migration round-trips cleanly +- Given the migration `0020_studies_baseline_trial` is applied (`alembic upgrade head`) +- When `alembic downgrade -1` is run +- Then both `studies.baseline_trial_id` and `trials.is_baseline` columns are removed +- And the schema matches the state at `0019_digests_suggested_followups_jsonb` +- When `alembic upgrade head` is re-run +- Then both columns are restored +- And the migration is idempotent on re-run with the columns already present (`alembic upgrade head` twice does not raise). + +### AC-14: `baseline_params` operator override resolves to tier (b) +- Given a study has `parent_study_id=NULL, parent_proposal_id=NULL, config.baseline_params={"boost_title": 1.2}` +- When `start_study` runs and `resolve_baseline_params` is called +- Then the resolver returns `{"boost_title": 1.2}` (not the template midpoint). + +### AC-16: late-completing baseline trial self-stamps via worker (FR-10 + FR-12) +- Given an orchestrator wait phase times out at `wait_s` seconds with `baseline_trial_id IS NULL` +- And the orchestrator proceeds to Optuna phase +- And the baseline `run_baseline_trial` job eventually completes successfully at `wait_s + 30` seconds +- When the worker calls `services.study_state.stamp_baseline_trial` +- Then `studies.baseline_trial_id` becomes set to the trial ID +- And `studies.baseline_metric` becomes set to the trial's `primary_metric` +- And the next `compute_study_confidence` call (e.g., from `GET /api/v1/studies/{id}`) renders `comparison_against = "baseline"`. + +### AC-17: aggregate_trials_summary excludes baseline trial (FR-11) +- Given a study has 5 Optuna trials (`is_baseline=FALSE`) and 1 baseline trial (`is_baseline=TRUE, status='complete'`) +- And the baseline trial has the highest `primary_metric` (an edge case where the operator's production config beats every Optuna trial) +- When `aggregate_trials_summary(db, study_id)` is called +- Then `summary.total == 5`, `summary.complete == 5` +- And `summary.best_trial_id` points at the best Optuna trial (NOT the baseline) +- And `summary.best_primary_metric` is the best Optuna trial's metric. + +### AC-18: evaluate_chain_gate is direction-aware (FR-5 minimize) +- Given a study has `objective={"metric": "ndcg", "direction": "minimize"}` (hypothetical — minimize objectives are wire-supported per `schemas.py:226` even though MVP1 examples are maximize-only) +- And `parent.best_metric = 0.30, parent.baseline_metric = 0.50` (lower is better, winner beats baseline by 0.20) +- When `evaluate_chain_gate(parent, complete_trials, direction='minimize', epsilon=0.005)` runs +- Then `outcome.lift == 0.20` (direction-normalized — always positive when winner beats baseline) +- And `outcome.decision == ChainGateDecision.ENQUEUE`. + +### AC-15: baseline trial's `_compute_metric_delta` populates `delta_pct` +- Given `study.baseline_metric=0.50, study.best_metric=0.60` +- When `_compute_metric_delta(study)` runs (existing code path, no change) +- Then the result is `{"ndcg@10": {"baseline": 0.50, "achieved": 0.60, "delta_pct": 20.0}}`. + +## 13) Non-functional requirements + +- **Performance**: Baseline trial adds at most one engine round-trip per study creation — typically 1–5 seconds. Acceptable since study creation is operator-driven (not user-facing latency). +- **Reliability**: Failed baselines do not fail studies. The orchestrator's wait timeout (150s) prevents indefinite blocking. +- **Operability**: New structured-log events: `baseline_skipped`, `baseline_failed`, `baseline_stamped`, `baseline_wait_timeout`. Operators can grep these to triage. Add a 1–2 line entry in the study-lifecycle runbook. +- **Accessibility/usability**: The "Show baseline trial" toggle uses the existing `` primitive in `ui/src/components/ui/` — accessibility properties inherit. + +## 14) Test strategy requirements (spec-level) + +- **Unit tests** (`backend/tests/unit/`) — all pure-Python, mocked externals, no DB: + - `domain/study/test_baseline_resolver.py` — 4-tier fallback resolver, every tier transition, empty params handling. Inputs are `SimpleNamespace` stand-ins; no DB session. **Mocked**. + - `domain/study/test_confidence.py` — extend with baseline branch tests for `compute_study_confidence` (AC-4, AC-5, AC-6). Pure-Python — pass `SimpleNamespace` trial rows. **Mocked**. + - `domain/study/test_auto_followup.py` — extend with baseline branch tests for `evaluate_chain_gate` (AC-7, AC-8) AND direction-awareness tests for the minimize objective case (FR-5). **Mocked**. + - `workers/test_baseline_trial.py` — `run_baseline_trial` happy path + failure path. Adapter/score/qrels-loader mocked via `monkeypatch`. **Mocked**. + - `services/test_stamp_baseline_trial.py` — service helper FR-12: stamp success, stamp idempotent (already-stamped no-ops), stamp with invalid trial state raises. **Mocked**. +- **Integration tests** (`backend/tests/integration/`) — real Postgres + real Redis + real Arq workers (service containers in CI; `make test-integration` locally). Adapter HTTP calls mocked via `monkeypatch` per existing convention: + - `test_orchestrator_baseline_trial.py` — **REAL BACKEND**: real Postgres + Arq orchestrator + Arq baseline worker. Studies create → baseline enqueues → terminal row written → orchestrator stamps → Optuna trials enqueue. Adapter mocked at the `search_batch` boundary (returns fixed hits). Asserts AC-1, AC-2, AC-3. + - `test_baseline_late_completion_stamp.py` — **REAL BACKEND**: simulate the wait-timeout case by forcing the worker to delay until after the orchestrator's wait expires (`asyncio.sleep` injected via env-var fault seam in `run_baseline_trial`). Asserts the worker's FR-10 step 7 self-stamp lands the field even after orchestrator's wait gave up. + - `test_studies_api_baseline.py` — **REAL BACKEND**: `GET /api/v1/studies/{id}` exposes `baseline_trial_id` (AC-9). + - `test_studies_api_confidence_baseline.py` — **REAL BACKEND**: seeds a study with a complete baseline trial + winner trial both having `per_query_metrics`; asserts API confidence shape with `comparison_against='baseline'` (AC-4, AC-5, AC-6). + - `test_create_study_baseline_params.py` — **REAL BACKEND**: `POST /api/v1/studies` with explicit `config.baseline_params`; assert it persists into `studies.config` and the orchestrator picks tier (b) (AC-14). + - `test_baseline_migration_round_trip.py` — **REAL BACKEND**: real Alembic against real Postgres. Upgrade/downgrade/upgrade (AC-13). + - `test_baseline_resume.py` — **REAL BACKEND**: orchestrator restart mid-baseline-trial resumes correctly (uses Arq job re-enqueue + the `resume_study` path). + - `test_trials_aggregate_excludes_baseline.py` — **REAL BACKEND**: insert a study with 5 Optuna trials + 1 baseline trial; assert `aggregate_trials_summary` returns `total=5`, NOT 6 (FR-11 invariant). +- **Contract tests** (`backend/tests/contract/`): + - `test_pr_body_confidence_section.py` — add new fixture with `baseline_trial_id` set; assert PR body contains "vs baseline" (AC-12). Existing 2 fixtures retained for `runner_up` regression coverage. + - `test_study_detail_baseline_trial_id_field.py` — assert `StudyDetail.baseline_trial_id` is in the response schema and is `string | null`. +- **E2E tests** (`ui/tests/e2e/`) — real-backend (no `page.route()` mocking per CLAUDE.md): + - Extend existing `studies-flow.spec.ts` (or whichever study-detail E2E file exists — plan-gen verifies). Test setup: seed the study via API (`page.request.post('/api/v1/studies', { data: { ..., config: { ..., baseline_params: {...} } } })`) since OQ-2 defers surfacing `baseline_params` in the create-study UI. Then UI assertions: (a) wait for completion, (b) navigate to study detail, (c) assert ConfidencePanel renders "vs baseline" label, (d) click "Show baseline trial" toggle, (e) assert baseline row appears with "Baseline" badge. + +**Test coverage gate**: 80% backend (current MVP1 standard). New code in `baseline_resolver.py` + `workers/baseline.py` MUST be ≥ 90% covered (no fallback-only branches; every path is testable). + +## 15) Documentation update requirements + +- **`docs/01_architecture/data-model.md`**: Update §"studies" with `baseline_trial_id` column; update §"trials" with `is_baseline` column. +- **`docs/01_architecture/optimization.md`** (if exists): add a 2-3 sentence note about the baseline trial being non-Optuna. +- **`docs/03_runbooks/study-lifecycle-debugging.md`** (if exists; otherwise the runbook for `feat_study_lifecycle`): document the 4 new log event types (`baseline_skipped`, `baseline_failed`, `baseline_stamped`, `baseline_wait_timeout`) and what each implies. +- **`prompts/digest_narrative.system.md`**: per FR-7. +- **`architecture.md`** (root): no change. +- **`state.md`**: update once feature merges (post-impl, not pre-impl). +- **`CLAUDE.md`**: no new absolute rules. The "don't bypass orchestrator" rule already covers `run_baseline_trial`'s no-Optuna interaction. + +## 16) Rollout and migration readiness + +- **Feature flags / staged rollout**: None. The feature is gated by the presence of the migration; pre-migration studies are unaffected. +- **Migration/backfill expectations**: Forward-only. No backfill. Existing studies stay `baseline_trial_id IS NULL` permanently (would require a re-study to populate retroactively, which is operationally cheaper than a migration backfill). +- **Operational readiness**: The orchestrator's wait phase is the new failure surface. The runbook update (§15) documents the four log events. +- **Release gate**: Standard MVP1 CI green + 80% coverage + GPT-5.5 cross-model review + Gemini PR adjudication. + +## 17) Traceability matrix + +| FR ID | Acceptance Criteria IDs | Planned stories/tasks | Test files/suites | Docs to update | +|---|---|---|---|---| +| FR-1 | AC-13 | Story 1.1 (migration) | `test_baseline_migration_round_trip.py` | data-model.md §studies + §trials | +| FR-2 | AC-1, AC-2, AC-3 | Story 1.2 (orchestrator) + Story 1.4 (resume) | `test_orchestrator_baseline_trial.py`, `test_baseline_resume.py` | runbook | +| FR-3 | AC-1, AC-2, AC-14 | Story 1.3 (resolver) | `test_baseline_resolver.py` | (none) | +| FR-4 | AC-4, AC-5, AC-6 | Story 2.1 (confidence) | `test_confidence.py`, `test_studies_api_confidence_baseline.py` | (none) | +| FR-5 | AC-7, AC-8, AC-18 | Story 2.2 (auto_followup gate) | `test_auto_followup.py` | auto-followup runbook | +| FR-6 | AC-14 | Story 1.5 (request schema) | `test_create_study_baseline_params.py` | (none) | +| FR-7 | AC-11 | Story 2.3 (system prompt) | snapshot test in `test_digest_prompt_render.py` | digest_narrative.system.md (the prompt itself IS the doc) | +| FR-8 | AC-9 | Story 2.4 (StudyDetail schema) | `test_study_detail_baseline_trial_id_field.py` | (none) | +| FR-9 | AC-10 | Story 3.1 (frontend trials-table filter) | E2E test | (none) | +| FR-10 | AC-1, AC-3, AC-16 | Story 1.4 (worker) | `test_baseline_trial.py`, `test_baseline_late_completion_stamp.py` | (none) | +| FR-11 | AC-17 | Story 1.6 (repo filter updates) | `test_trials_aggregate_excludes_baseline.py` + extensions of existing aggregate tests | (none) | +| FR-12 | AC-1, AC-16 | Story 1.5 (service helper) | `test_stamp_baseline_trial.py` | (none) | + +## 18) Definition of feature done + +This feature is complete when: + +- [ ] All acceptance criteria (AC-1 through AC-18) pass in CI. +- [ ] All test layers (unit/integration/contract/e2e) are green. +- [ ] Backend coverage ≥ 80% global; new files (`baseline_resolver.py`, `workers/baseline.py`) ≥ 90%. +- [ ] Documentation updates per §15 are merged. +- [ ] Rollout gates from §16 are satisfied. +- [ ] No open questions remain in §19. +- [ ] Migration round-trips cleanly via `alembic upgrade head && alembic downgrade -1 && alembic upgrade head`. +- [ ] The PR body of the next study run against a `baseline_params`-equipped study shows non-None `delta_pct` (manual smoke test). + +## 19) Open questions and decision log + +### Open questions + +- **OQ-1: Default filter behavior for the baseline trial in the trials-listing UI.** Locked: FILTER OUT by default (FR-9), with a "Show baseline trial" toggle to reveal. The baseline trial's `optuna_trial_number=-1` would confuse the Optuna trial-number ordering if shown inline. **Resolution**: confirmed FR-9 default = filter out. +- **OQ-2: Surface `baseline_params` in the create-study UI?** The `feat_create_study_search_space_builder` modal is the natural place. Locked decision: **defer to a follow-up idea** — `chore_create_study_baseline_params_ui` will be captured if the operator's first-time discovery of `baseline_params` requires UI affordance. For MVP, `baseline_params` is an advanced operator override accessible only via the API + the chat agent's `create_study` tool. +- **OQ-3: Should the digest worker LLM call know about baseline failures explicitly?** When `baseline_trial_id` is NULL because the baseline failed, should `` say "N/A (no baseline trial)" or "N/A (baseline trial failed: )"? Locked: the current "N/A (no baseline trial)" text is sufficient; the operator can dig into the failed `Trial` row if they want details. **Resolution**: no change to the digest user prompt for this edge. + +All open questions resolved before plan-gen. + +### Decision log + +- **2026-05-25 — D-1**: Baseline trial uses real `Trial` rows with `is_baseline=TRUE` and `optuna_trial_number=-1` rather than a separate `baseline_trials` table. Rationale: keeps cascade-delete semantics simple, reuses every existing per-trial column (params, primary_metric, metrics, per_query_metrics, error, started_at, ended_at), and Optuna never queries the app trials table (it uses its own RDB), so the negative sentinel cannot pollute Optuna's state. +- **2026-05-25 — D-2**: Baseline-params resolver uses a 4-tier fallback (parent_proposal → parent_study → operator-supplied → template defaults), matching the operator's "what did I change?" mental model. Rationale: for digest-executable followups (the most common case once `feat_auto_followup_studies` chains start landing), the operator's mental baseline IS the parent proposal's config. Falling all the way through to template defaults is the safety net. +- **2026-05-25 — D-3**: Auto-followup gate switches from `first_decile_max` to `parent.baseline_metric` when the latter is set. Rationale: the `auto_followup.py:9-11` module docstring explicitly promised this. Backward-compatible because `parent.baseline_metric` is NULL for every study created before this feature lands. +- **2026-05-25 — D-4**: Baseline trial runs synchronously before Optuna, not in parallel. Rationale: a one-shot fast trial (~1-5s) doesn't need parallelism, and serial ordering simplifies the trial-counter / `_count_in_flight` invariants the orchestrator already depends on. +- **2026-05-25 — D-5**: Baseline trials use `optuna_trial_number = -1` as a sentinel filler for the NOT-NULL column; `is_baseline=TRUE` is the canonical discriminator. Rationale: the column is `NOT NULL` (declared in `feat_study_lifecycle` Phase 1) and Optuna's RDB never reads the app trials table. **Idempotency**: `run_trial` keeps its existing `(study_id, optuna_trial_number)` idempotency (Optuna trial numbers are always non-negative); `run_baseline_trial` uses `trial_id`-based idempotency (the orchestrator pre-generates a UUIDv7 in FR-2 and passes it as a job argument). The two code paths are disjoint and cannot collide. +- **2026-05-25 — D-6**: Failed baseline trials do not fail the study. Rationale: the baseline is informational. Failing the study because production-config-baseline failed would be a regression vs today (where studies run fine with no baseline). The fall-back paths in FR-4 + FR-5 handle missing baseline data gracefully. +- **2026-05-25 — D-7**: `baseline_params` lives in `studies.config` JSONB, not a top-level `studies.baseline_params` column. Rationale: every other study-tunable lives in `config`; growing the top-level schema for an optional advanced override is debt. +- **2026-05-25 — D-8**: Baseline trial uses the same per-trial timeout as Optuna trials (`studies.config.trial_timeout_s` or `Settings.studies_default_timeout_s` fallback). Rationale: no second knob. +- **2026-05-25 — D-9**: No backfill. Existing studies stay `baseline_trial_id IS NULL`. Rationale: backfilling would require re-running the engine queries for every historical study, which has runtime cost without comparable value (operators care about NEW studies, not historical confidence retroactively). +- **2026-05-25 — D-10**: Per-trial timeout for baseline is honored as-is (no separate baseline timeout). Rationale: same as D-8 — single knob, no debt. +- **2026-05-25 — D-11**: Audit-log emission is deferred. Rationale: MVP1 has no audit_log table; will be a sweep at MVP2. +- **2026-05-25 — D-12** (added after GPT-5.5 cycle-1 review F2/F14): The baseline-trial stamping path is single-chokepoint via `services.study_state.stamp_baseline_trial` (FR-12). The orchestrator's fast-path stamp (FR-2 step 7), the worker's self-stamp (FR-10 step 7), and the `resume_study` re-stamp all route through the same helper. Rationale: prevents three slightly-different UPDATE statements drifting; matches the existing `services/study_state.py` pattern; positions us for audit-event emission at MVP2. +- **2026-05-25 — D-13** (added after GPT-5.5 cycle-1 review F10): The orchestrator's wait phase is best-effort — if it times out, the `run_baseline_trial` worker is left running and stamps the study on its own success via FR-10/FR-12. Rationale: cancelling the in-flight Arq job is harder than letting it self-stamp; the operator gets baseline data eventually rather than losing it; if the worker never completes, the worker layer's own per-trial timeout fires and the row lands as `status='failed'` (which we already handle gracefully per §4 principle). +- **2026-05-25 — D-14** (added after GPT-5.5 cycle-1 review F15): `evaluate_chain_gate` becomes direction-aware in this feature. Rationale: we're touching the same lines anyway for the baseline switch; closing the latent minimize-direction bug in `feat_auto_followup_studies` while we're here is cheap (one extra `direction` argument + a sign flip). Rejecting "implement-over-defer" guidance in CLAUDE.md would have required capturing as `bug_auto_followup_minimize_direction` — but that's strictly worse than inlining the fix. +- **2026-05-25 — D-15** (added after GPT-5.5 cycle-1 review F8): The trials-aggregate read paths (`aggregate_trials_summary`, `list_top_trials`, `list_complete_trials_for_confidence`) ALL filter `is_baseline = FALSE` inline (FR-11). Rationale: operators NEVER want baseline in these aggregates — adding a kwarg `include_baseline=False` adds API surface without value, and an unfiltered query would corrupt confidence + auto-followup downstream. +- **2026-05-25 — D-16** (added after GPT-5.5 cycle-3 review F1): Double-baseline-on-resume race is prevented by three independent layers: (a) Arq `_job_id` deduplication (FR-2 step 4); (b) partial unique index `uq_trials_study_baseline_complete` (FR-1); (c) FR-12 stamping helper's `WHERE baseline_trial_id IS NULL` predicate. Rationale: a single layer would be sufficient in steady-state, but defense in depth makes the invariant hold across Arq driver changes (job-id format may change), partial-index maintenance (rebuilds), and edge-case race windows. The marginal complexity is one partial index + one `_job_id` argument. diff --git a/docs/02_product/planned_features/feat_study_baseline_trial/idea.md b/docs/02_product/planned_features/feat_study_baseline_trial/idea.md index 5bb7dd46..5e53d06f 100644 --- a/docs/02_product/planned_features/feat_study_baseline_trial/idea.md +++ b/docs/02_product/planned_features/feat_study_baseline_trial/idea.md @@ -7,17 +7,17 @@ **Depends on:** Phase 1 of `feat_pr_metric_confidence` (PR #180) — merged. Phase 2 is purely additive — no migration to undo, no API contract break. -**Still-needed verification (2026-05-22):** confirmed against `main` HEAD — -- `studies.baseline_metric` column exists at [`backend/app/db/models/study.py:76`](../../../../backend/app/db/models/study.py) but `grep -rn 'baseline_metric *=' backend/workers/ backend/app/services/` returns zero write sites. The column stays `NULL` forever in production. -- `backend/workers/digest.py:706` reads `study.baseline_metric` and passes it to the LLM prompt (so the digest narrative always renders with `baseline=None`). -- `ComparisonAgainst = Literal["runner_up", "baseline"]` exists at [`backend/app/domain/study/confidence.py:114`](../../../../backend/app/domain/study/confidence.py) but only `"runner_up"` is emitted (hardcoded at line 624 with comment `# FR-3 locked for Phase 1`). -- `studies.baseline_trial_id` column does NOT exist on the Study model. +**Still-needed verification (re-confirmed 2026-05-25 against `main` HEAD `ba224865`):** +- `studies.baseline_metric` column exists at [`backend/app/db/models/study.py:95`](../../../../backend/app/db/models/study.py#L95). No code path WRITES to it during a study workflow (`grep -rn 'baseline_metric' backend/workers backend/app/services` returns only **read** sites: [`digest.py:517`](../../../../backend/workers/digest.py#L517) reads it into a local `baseline` var, [`digest.py:948`](../../../../backend/workers/digest.py#L948) passes it as a kwarg to the LLM prompt, [`api/v1/studies.py:139`](../../../../backend/app/api/v1/studies.py#L139) serializes it onto the StudyDetail response). The column stays `NULL` forever in production. +- `backend/workers/digest.py:517` reads `study.baseline_metric` into a local + line 948 passes it through to the digest-narrative LLM prompt; both render `baseline=None` today. +- `ComparisonAgainst = Literal["runner_up", "baseline"]` exists at [`backend/app/domain/study/confidence.py:114`](../../../../backend/app/domain/study/confidence.py#L114) but only `"runner_up"` is emitted (hardcoded at [`confidence.py:624`](../../../../backend/app/domain/study/confidence.py#L624) with comment `# FR-3 locked for Phase 1`). +- `studies.baseline_trial_id` column does NOT exist on the Study model (`grep` returns zero matches across the codebase). -All 4 still-needed signals from the original draft remain accurate on 2026-05-22. +All 4 still-needed signals from the original draft remain accurate; line citations refreshed against `main` HEAD on 2026-05-25. ## Problem -`studies.baseline_metric` exists as a column on the `studies` table (declared in `feat_study_lifecycle` Phase 1, [`backend/app/db/models/study.py:76`](../../../../backend/app/db/models/study.py#L76)) with the docstring "single non-Optuna trial run before Optuna starts; populated by the orchestrator (Phase 2)." However, **the orchestrator was never updated to populate this column** — `grep -rn "baseline_metric *=" backend/workers/ backend/app/services/` returns zero write sites. In production, `study.baseline_metric` is always `None`, and the PR body's `## Metric delta` section shows `baseline=None → achieved=X` with no `delta_pct`. +`studies.baseline_metric` exists as a column on the `studies` table (declared in `feat_study_lifecycle` Phase 1, [`backend/app/db/models/study.py:95`](../../../../backend/app/db/models/study.py#L95)) with the docstring "single non-Optuna trial run before Optuna starts; populated by the orchestrator (Phase 2)." However, **the orchestrator was never updated to populate this column** — grep across `backend/workers/`, `backend/app/services/`, `backend/app/api/` finds only **read** sites (digest worker + studies API response serialization), no path that assigns to `study.baseline_metric` during a workflow. In production, `study.baseline_metric` is always `None`, and the PR body's `## Metric delta` section shows `baseline=None → achieved=X` with no `delta_pct`. Phase 1 of `feat_pr_metric_confidence` ships per-query analytics that compare the winner against the **runner-up #2 trial** instead of a true baseline. That comparison answers "is the winner robust or fragile vs other tried configs?" but does NOT answer "does this config regress queries that the operator's current production search behavior gets right?" — which is the more directly actionable approver question. @@ -33,7 +33,8 @@ Phase 2 closes this gap by: - **Real product-design surface.** The semantics of "baseline" need a spec-shaped decision. Options include: - **(a) Template defaults.** The baseline trial uses the query template's `declared_params` with each param's middle-of-range value (`(low + high) / 2` for floats, the median choice for categoricals). Simple, deterministic, but may not reflect the operator's actual production config. - **(b) Operator-supplied baseline.** The create-study request body gains an optional `baseline_params: dict[str, Any] | None` field. When provided, the orchestrator runs a baseline trial with those params before Optuna starts. When absent, no baseline runs (status quo). - - **(c) Previous study's winner.** If the study has `parent_study_id` (fork lineage, MVP2 surface), the baseline is the parent's winning trial's params. When no parent, no baseline runs. + - **(c) Previous study's winner.** If the study has `parent_study_id` (fork lineage — now MVP1-active via `feat_auto_followup_studies` PR #223, merged 2026-05-24), the baseline is the parent's winning trial's params. When no parent, no baseline runs. **Note (refreshed 2026-05-25):** original draft marked this "MVP2 surface" because parent_study_id was unused at draft time; that's now obsolete. + - **(d) Parent proposal's config.** If the study has `parent_proposal_id` + `parent_proposal_followup_index` (the digest-executable-followups lineage added by `feat_digest_executable_followups` PR #225, merged 2026-05-24; see [`study.py:82-91`](../../../../backend/app/db/models/study.py#L82-L91)), the baseline is the config that the parent proposal would apply (i.e., the parent study's best trial). Distinct from (c): (c) chains studies; (d) chains across the digest→study→proposal→new-study handoff. Most directly answers "what does this followup CHANGE vs the current best?" for the digest-executable-followups flow. - **Statistical design surface.** Once baseline data exists, the per-query delta semantics flip from "vs runner-up" to "vs production behavior" — the regressor framing changes from "winner sacrificed this query to other tried configs" to "winner makes this query worse than production." Both are valid signals; spec needs to lock which is the default surface (likely baseline when available, runner-up otherwise). - **Compounding orchestrator complexity.** Adding a non-Optuna trial path means the orchestrator needs to (a) not increment Optuna's trial counter for the baseline, (b) handle baseline-trial failure differently than Optuna trial failure (a failed baseline should NOT block the study; just skip the comparison surface), (c) handle the baseline-trial timeout window separately from per-trial Optuna timeouts. @@ -41,8 +42,8 @@ Phase 2 closes this gap by: ### Capability 1 — Migration: add `studies.baseline_trial_id` -- Alembic migration `00NN_studies_baseline_trial_id` (next available revision after Phase 1's `0015`). -- Schema: `baseline_trial_id String(36) NULL`. Not a formal FK (per the same rationale as `best_trial_id` in [`study.py:80-84`](../../../../backend/app/db/models/study.py#L80) — orchestrator stamps it after baseline trial completes; no enforce-at-DB constraint). +- Alembic migration `00NN_studies_baseline_trial_id` (next available revision after the current head `0019_digests_suggested_followups_jsonb`; so `0020_*`). +- Schema: `baseline_trial_id String(36) NULL`. Not a formal FK (per the same rationale as `best_trial_id` in [`study.py:99`](../../../../backend/app/db/models/study.py#L99) — orchestrator stamps it after baseline trial completes; no enforce-at-DB constraint). - Reversible `downgrade()` drops the column. Round-trip verified. - No backfill — existing studies stay `baseline_trial_id IS NULL` and continue to show `comparison_against = "runner_up"` per Phase 1 fallback. @@ -87,10 +88,12 @@ Phase 2 closes this gap by: - **Builds on** [`feat_pr_metric_confidence`](../../../00_overview/implemented_features/2026_05_21_feat_pr_metric_confidence/feature_spec.md) (Phase 1 of this feature, shipped 2026-05-21 as PR #180). Phase 1 must merge first so the `ConfidenceShape` and `compute_study_confidence` infrastructure exists for Phase 2 to extend. - **Composes with** [`feat_study_lifecycle`](../../../00_overview/implemented_features/2026_05_10_feat_study_lifecycle/feature_spec.md) — Phase 2 retroactively implements the "Phase 2" baseline-trial work that the study_lifecycle spec promised but deferred. Now it's a separate feature with its own spec cycle. - **Composes with** [`feat_create_study_search_space_builder`](../../../00_overview/implemented_features/2026_05_20_feat_create_study_search_space_builder/feature_spec.md) — if Phase 2 picks design option (b) (operator-supplied baseline_params), the create-study modal gains a new optional input. The search-space builder is the natural place for that input. +- **Composes with** [`feat_digest_executable_followups`](../../../00_overview/implemented_features/2026_05_24_feat_digest_executable_followups/feature_spec.md) (PR #225, merged 2026-05-24) — added the `studies.parent_proposal_id` + `parent_proposal_followup_index` lineage that powers option (d) "parent proposal's config" baseline. The most directly actionable baseline for a digest-executable followup study is "the config the parent proposal would have shipped"; before Phase 2, that comparison is impossible. +- **Composes with** [`feat_auto_followup_studies`](../../../00_overview/implemented_features/2026_05_24_feat_auto_followup_studies/feature_spec.md) (PR #223, merged 2026-05-24) — promoted `studies.parent_study_id` from "MVP2 surface" to MVP1-active via auto-enqueued follow-up studies. The original draft's option (c) marker is updated above. ## Open questions for /spec-gen (Phase 2) -1. **Baseline semantics** — Which of (a) template defaults, (b) operator-supplied, (c) parent-study winner is the locked default? Recommended: (b) operator-supplied with a fallback to (a) template defaults when not provided. +1. **Baseline semantics** — Which of (a) template defaults, (b) operator-supplied, (c) parent-study winner, **(d) parent-proposal config** is the locked default? Recommended: a multi-tier fallback — (d) parent_proposal config when the study has `parent_proposal_id` set (auto-followup / digest-executable-followups studies); (c) parent_study winner when `parent_study_id` is set (manual forks); (b) operator-supplied when the create-study request carries `baseline_params`; (a) template-defaults as final fallback. This ordering matches the operator's "what did I change?" mental model — for a digest-executable followup the most actionable baseline is "the config that the parent proposal would have shipped." Spec needs to confirm the order. 2. **Synchronous vs async baseline** — Does the orchestrator BLOCK on the baseline trial completing before enqueueing Optuna trials, or does it dispatch both in parallel? Recommended: synchronous (the baseline is a one-shot fast trial; Optuna can wait the extra 2-5 seconds). 3. **Baseline-trial failure handling** — Does a failed baseline fail the study OR proceed without baseline data? Recommended: proceed without (the baseline is informational, not load-bearing; failing the entire study because production-config-baseline failed would be a regression). 4. **`optuna_trial_number = -1` sentinel** — How does the existing trial-listing UI handle a trial with `optuna_trial_number = -1`? The Optuna RDB may not tolerate negative trial numbers. Alternative: a separate `baseline_trials` table; or a `trials.is_baseline` boolean. Recommended: investigate during Phase 2 spec — likely a `trials.is_baseline BOOLEAN NOT NULL DEFAULT FALSE` flag is cleaner than a sentinel. diff --git a/docs/02_product/planned_features/feat_study_baseline_trial/implementation_plan.md b/docs/02_product/planned_features/feat_study_baseline_trial/implementation_plan.md new file mode 100644 index 00000000..096f0d3d --- /dev/null +++ b/docs/02_product/planned_features/feat_study_baseline_trial/implementation_plan.md @@ -0,0 +1,922 @@ +# Implementation Plan — `feat_study_baseline_trial` + +**Date:** 2026-05-25 +**Status:** Draft — pending GPT-5.5 cross-model review +**Primary spec:** [`feature_spec.md`](feature_spec.md) +**Policy sources:** [`CLAUDE.md`](../../../../CLAUDE.md), [`architecture.md`](../../../../architecture.md), [`state.md`](../../../../state.md) + +--- + +## 0) Planning principles + +- Spec traceability first: every story maps to FR IDs and the spec's ACs. +- Phase gates are hard stops — failing tests within a phase block the next phase. +- Fail-loud tests: assert explicit status/shape/error_code; never use bare-assertion `assert response.ok`. +- Backend-first ordering: migration → repo → domain → service → worker → orchestrator → API → frontend. +- Each story is independently verifiable — a story's DoD is the test layer that proves it. + +## 1) Scope traceability (FR → epics) + +| FR ID | Epic / Story | Notes | +|---|---|---| +| FR-1 (migration) | Epic 1 / Story 1.1 | 0020_studies_baseline_trial: `studies.baseline_trial_id` + `trials.is_baseline` + partial unique index `uq_trials_study_baseline_complete` | +| FR-2 (orchestrator) | Epic 1 / Story 1.7 | Inserts baseline phase between search-space parse and Optuna polling | +| FR-3 (resolver) | Epic 1 / Story 1.2 | 4-tier fallback: parent_proposal → parent_study → operator-supplied → template defaults | +| FR-4 (confidence) | Epic 2 / Story 2.1 | One-line conditional at `confidence.py:624` + new keyword arg | +| FR-5 (auto-followup) | Epic 2 / Story 2.2 | Direction-aware lift; rename `compute_first_decile_max` → `compute_first_decile_extremum` | +| FR-6 (`baseline_params`) | Epic 1 / Story 1.5 | New `StudyConfigSpec.baseline_params` typed `dict[str, primitives] \| None` | +| FR-7 (digest prompt) | Epic 2 / Story 2.3 | System-prompt 1-2 sentence addition + glossary tooltip refresh | +| FR-8 (`StudyDetail`) | Epic 1 / Story 1.5 | Expose `baseline_trial_id` + `is_baseline` via API schemas | +| FR-9 (UI filter) | Epic 3 / Story 3.1 | trials-table baseline filter + "Show baseline" toggle + Baseline badge | +| FR-10 (worker) | Epic 1 / Story 1.3 | `run_baseline_trial` Arq job + self-stamp on completion | +| FR-11 (repo filters) | Epic 1 / Story 1.6 | `is_baseline=FALSE` filters on aggregate / list / complete-trial reads | +| FR-12 (stamp helper) | Epic 1 / Story 1.4 | `services.study_state.stamp_baseline_trial` chokepoint | + +**Deferred phases:** None — feature is single-phase by design (see spec §3 "Phase boundaries"). + +## 2) Delivery structure + +Structure: **Epic → Story → Tasks → DoD**. Stories are sequential within an epic; epics are gated by phase gates (full test suite + cross-model review). + +### Conventions (RelyLoop-specific) + +- All repo functions take `db: AsyncSession` as first arg; use `db.flush()` (caller commits). +- Services are async; long-running services create a `job_run` record at start where applicable (N/A for this feature). +- Domain layer is pure — no DB access, no side effects (resolver is the exception; it takes `db` for parent-row lookups but does NO writes). +- Models use `Mapped[]` typed columns, `String(36)` UUIDs, `TIMESTAMPTZ` for time. +- Routers return typed Pydantic response models; errors use the `_err(status, code, msg, retryable)` helper at `backend/app/api/v1/studies.py:113`. +- Config via `pydantic-settings`; never hardcode model names (CLAUDE.md Absolute Rule #8). +- All `__init__.py` exports updated via `__all__`. +- Migrations include `downgrade()` + idempotency guards + round-trip verification (CLAUDE.md Absolute Rule #5). +- Test layering: unit → integration → contract → E2E. Mocked tests use `monkeypatch`; real-backend integration tests run against service-container Postgres in CI. + +### AI Agent Execution Protocol + +0. Load context: read `CLAUDE.md`, `architecture.md`, `state.md` before starting Story 1.1. +1. Read story scope: outcome + files + interfaces + DoD. +2. Implement backend in order: model → migration → repo → domain → service → worker → router → schemas. +3. Run touched-layer tests before moving to next story. +4. Implement frontend (if applicable). +5. Run E2E scope for touched paths. +6. Update docs in same PR. +7. Verify migration round-trip after Story 1.1. +8. After final story, update `state.md` and any architecture topical docs. + +--- + +## Epic 1 — Foundation: schema + worker + service helpers (FR-1, FR-3, FR-6, FR-8, FR-10, FR-11, FR-12, FR-2) + +The backend foundation lands first. Every Epic 2 / Epic 3 story depends on the columns + helpers from Epic 1. + +### Story 1.1 — Migration 0020 + ORM + `repo.create_trial(is_baseline=…)` (FR-1, AC-13) + +**Outcome:** `studies.baseline_trial_id` and `trials.is_baseline` columns exist in the DB. The partial unique index `uq_trials_study_baseline_complete` is in place. The ORM models reflect the new columns. `repo.create_trial` accepts the new `is_baseline` kwarg (default `False`, so all existing callers are byte-compatible). Migration round-trips cleanly AND is idempotent on re-run. + +**New files** + +| File | Purpose | +|---|---| +| `migrations/versions/0020_studies_baseline_trial.py` | Alembic migration adding both columns + partial unique index. Reversible. | + +**Modified files** + +| File | Change | +|---|---| +| `backend/app/db/models/study.py` | Add `baseline_trial_id: Mapped[str \| None] = mapped_column(String(36), nullable=True)` after `best_trial_id` at line 99-103. Mirror the docstring pattern. | +| `backend/app/db/models/trial.py` | Add `is_baseline: Mapped[bool] = mapped_column(Boolean, nullable=False, server_default=text("FALSE"))` to the `Trial` class. Update class docstring with the column's purpose. | +| `backend/app/db/repo/trial.py` | Extend `create_trial` signature to accept `is_baseline: bool = False` (default keeps every existing caller byte-compatible). Plumb into the INSERT statement. Update `__all__` if its arg list is documented anywhere. (Plan F2: this lands here so Story 1.4 can use the new kwarg without a circular dependency.) | + +**Endpoints:** N/A. + +**Key interfaces:** N/A — schema-only story. + +**Pydantic schemas:** N/A — schema-only story. + +**Tasks** + +1. Run `ls migrations/versions/` and confirm `0019_digests_suggested_followups_jsonb.py` is the current head. +2. Create `migrations/versions/0020_studies_baseline_trial.py` with `revision = "0020_studies_baseline_trial"`, `down_revision = "0019_digests_suggested_followups_jsonb"`. +3. In `upgrade()`: idempotently add `studies.baseline_trial_id String(36) NULL` via `op.execute("DO $$ BEGIN IF NOT EXISTS (SELECT 1 FROM information_schema.columns WHERE table_name = 'studies' AND column_name = 'baseline_trial_id') THEN ALTER TABLE studies ADD COLUMN baseline_trial_id VARCHAR(36); END IF; END $$;")`. +4. Idempotently add `trials.is_baseline BOOLEAN NOT NULL DEFAULT FALSE` via the same guard pattern. +5. Idempotently create the partial unique index: `CREATE UNIQUE INDEX IF NOT EXISTS uq_trials_study_baseline_complete ON trials (study_id) WHERE is_baseline = TRUE AND status = 'complete';`. +6. In `downgrade()`: DROP INDEX `uq_trials_study_baseline_complete`, DROP COLUMN `trials.is_baseline`, DROP COLUMN `studies.baseline_trial_id` (idempotency-guarded with `IF EXISTS`). +7. Update `backend/app/db/models/study.py:Study` to add the column with docstring. +8. Update `backend/app/db/models/trial.py:Trial` to add the column + import `Boolean` + `text` from sqlalchemy. +9. Verify round-trip: `.venv/bin/alembic upgrade head && .venv/bin/alembic downgrade -1 && .venv/bin/alembic upgrade head`. +10. Run `make test-integration` — confirm existing tests pass with new columns. + +**Definition of Done** + +- `alembic upgrade head` succeeds against a fresh Postgres. +- `alembic downgrade -1 && alembic upgrade head` round-trips cleanly (AC-13). +- `alembic upgrade head` is idempotent — re-running with the columns + index already present does not raise (covered by an explicit test in `test_baseline_migration_round_trip.py` that runs `op.run_migrations()` twice via the alembic Python API and asserts no exception). (Plan F7.) +- New integration test `backend/tests/integration/test_baseline_migration_round_trip.py` asserts: + - `studies.baseline_trial_id` column exists with `VARCHAR(36)` type and is nullable. + - `trials.is_baseline` column exists with `BOOLEAN NOT NULL DEFAULT FALSE`. + - `uq_trials_study_baseline_complete` index exists with correct WHERE clause (verify via `pg_indexes` query). + - Idempotent re-run: invoking the migration logic twice does not raise. +- Existing `backend/tests/integration/test_study_lifecycle_migration.py` updated to include the new columns in its column-list assertion (line 424). +- `repo.create_trial` accepts `is_baseline=True` and persists the column (unit test in `backend/tests/integration/test_create_trial_is_baseline.py`). +- `make test-integration` green. + +--- + +### Story 1.2 — `resolve_baseline_params` domain helper (FR-3, AC-1, AC-2, AC-14) + +**Outcome:** A pure-domain async function resolves baseline params via the 4-tier fallback (parent_proposal → parent_study → operator-supplied → template defaults). Caller (Story 1.7's orchestrator) passes a `Study` row + `db`; resolver returns `dict | None`. + +**New files** + +| File | Purpose | +|---|---| +| `backend/app/domain/study/baseline_resolver.py` | 4-tier fallback resolver + per-tier helpers (`_resolve_from_parent_proposal`, `_resolve_from_parent_study`, `_resolve_from_operator_supplied`, `_resolve_from_template_defaults`). | +| `backend/tests/unit/domain/study/test_baseline_resolver.py` | Unit tests for every tier transition + edge cases (empty params, missing parent, log emission). | + +**Modified files** + +| File | Change | +|---|---| +| `backend/app/domain/study/__init__.py` | Export `resolve_baseline_params` via `__all__`. | + +**Endpoints:** N/A. + +**Key interfaces** + +```python +# backend/app/domain/study/baseline_resolver.py +async def resolve_baseline_params(db: AsyncSession, study: Study) -> dict[str, Any] | None: + """4-tier fallback resolver per spec FR-3. + + Returns None when the search-space has no declared params AND no + explicit tier resolved (i.e., baseline trial should be skipped). + """ + +def _template_midpoint(search_space: SearchSpace) -> dict[str, Any]: + """Pure helper: return middle-of-range for every declared param. + + Float: (low + high) / 2.0 (or sqrt(low * high) for log=true). + Int: (low + high) // 2. + Categorical: choices[(len(choices) - 1) // 2] (lower midpoint). + """ +``` + +**Pydantic schemas:** N/A. + +**Tasks** + +1. Create `baseline_resolver.py` with the four tier helpers + the orchestrator function. +2. Tier (d) `_resolve_from_parent_proposal`: if `study.parent_proposal_id` set, load `Proposal` via `repo.get_proposal(db, study.parent_proposal_id)`, then load the `Trial` at `proposal.study_trial_id` via `repo.get_trial`. Return `trial.params` if both exist; else log `event_type="baseline_resolve_parent_proposal_missing"` and return `None` (caller falls through). +3. Tier (c) `_resolve_from_parent_study`: if `study.parent_study_id` set, load `Study` + the trial at `parent.best_trial_id`. Return `trial.params` or `None`. +4. Tier (b) `_resolve_from_operator_supplied`: read `study.config.get("baseline_params")`. Return as-is (dict already typed by Pydantic at create-time). +5. Tier (a) `_template_midpoint`: parse `study.search_space` via `SearchSpace.model_validate`; iterate `params` and apply the type-discriminator midpoint formula. +6. `resolve_baseline_params` chains them: try (d) → (c) → (b) → (a). Return `None` if (a) returns `{}` (empty declared_params). +7. Write unit tests covering: each tier hit individually, fall-through cascades, missing parent trial (cascade-delete race), empty declared params, log emission verification via `caplog`. + +**Definition of Done** + +- Unit test coverage ≥ 95% on `baseline_resolver.py`. +- 12+ unit tests covering all 4 tiers + 6+ edge cases (deleted parent trial, deleted parent study, empty search space, log emission, log redaction). +- Resolver portions of AC-1, AC-2, AC-14 covered by unit tests (the full end-to-end "orchestrator stamps + Optuna proceeds" parts of AC-1/AC-2 are covered by Story 1.7's integration tests). (Plan F10.) + +--- + +### Story 1.3 — `stamp_baseline_trial` service helper (FR-12, AC-1, AC-16) + +(Reordered before the worker per plan F1 so Story 1.4 can call it without forward dependency.) + +**Outcome:** A single service-layer chokepoint stamps `studies.baseline_trial_id` + `baseline_metric`. Idempotent. Used by the orchestrator (FR-2 step 7), the worker self-stamp (FR-10 step 7), and the resume path (§9 idempotency). + +**New files** + +| File | Purpose | +|---|---| +| `backend/tests/unit/services/test_stamp_baseline_trial.py` | Mocked-DB unit tests (uses `monkeypatch` on the SQL execution). | +| `backend/tests/integration/test_stamp_baseline_trial_integration.py` | Real-Postgres integration test. | + +**Modified files** + +| File | Change | +|---|---| +| `backend/app/services/study_state.py` | Add `stamp_baseline_trial(db, study_id, trial_id, primary_metric) -> bool` + custom exceptions `BaselineTrialNotFound`, `InvalidBaselineTrialState`. | + +**Endpoints:** N/A. + +**Key interfaces** + +```python +# backend/app/services/study_state.py +class BaselineTrialNotFound(Exception): ... +class InvalidBaselineTrialState(Exception): ... + +async def stamp_baseline_trial( + db: AsyncSession, + study_id: str, + trial_id: str, + primary_metric: float, +) -> bool: + """Stamp studies.baseline_trial_id + baseline_metric. + + Returns True if this caller stamped, False if a sibling already + stamped (race-tolerant). Raises BaselineTrialNotFound if the trial + row is missing; raises InvalidBaselineTrialState if the row's + is_baseline / status / study_id don't match expectations. + + Idempotent via WHERE baseline_trial_id IS NULL predicate. + Commit is left to the caller; both the orchestrator and the worker + MUST call `await db.commit()` after this returns to durably land + the stamp. + """ +``` + +**Pydantic schemas:** N/A. + +**Tasks** + +1. Add the two exception classes at the top of `study_state.py` (next to existing `InvalidStateTransition`). +2. Implement `stamp_baseline_trial`: load trial → assert `study_id`, `is_baseline=TRUE`, `status='complete'` → execute the idempotent UPDATE. +3. Use SQLAlchemy `text()` with **named bind parameters** (NOT asyncpg `$1, $2`): `text("UPDATE studies SET baseline_trial_id = :trial_id, baseline_metric = :primary_metric WHERE id = :study_id AND baseline_trial_id IS NULL RETURNING id")` invoked via `await db.execute(stmt, {"trial_id": ..., "primary_metric": ..., "study_id": ...})`. The `.rowcount` or `.fetchone()` tells us whether we stamped. (Plan F8.) +4. Return `True` on stamp (1 row affected), `False` if already-stamped (0 rows). +5. Write unit tests: happy path, race (already-stamped no-ops returning False), BaselineTrialNotFound, InvalidBaselineTrialState for each precondition (wrong study_id, is_baseline=FALSE, status≠'complete'). +6. Write integration test: insert real `studies` + `trials` rows; call stamp helper + commit; assert UPDATE landed; call stamp helper again; assert idempotent. + +**Definition of Done** + +- Unit test coverage ≥ 95% on the new helper. +- 8+ unit tests + 3+ integration tests (real Postgres). +- AC-1, AC-16 covered by the contract that Story 1.4's worker self-stamp + Story 1.7's orchestrator stamp will depend on. + +--- + +### Story 1.4 — `run_baseline_trial` worker (FR-10, AC-1, AC-3, AC-16) + +(Reordered after the stamp helper per plan F1.) + +**Outcome:** A new Arq job runs the baseline trial: renders the template, executes the engine query, scores, and persists a `Trial` row with `is_baseline=TRUE, optuna_trial_number=-1`. On completion, self-stamps `studies.baseline_trial_id` + `baseline_metric` via Story 1.3's helper, then commits. Idempotent via the pre-generated `trial_id`. Registered in `WorkerSettings.functions`. Includes a test-only fault seam for the late-completion integration test (plan F9). + +**New files** + +| File | Purpose | +|---|---| +| `backend/workers/baseline.py` | `run_baseline_trial(ctx, study_id, trial_id, params)` Arq job. | +| `backend/tests/unit/workers/test_baseline_trial.py` | Mocked-adapter unit tests for `run_baseline_trial`. | + +**Modified files** + +| File | Change | +|---|---| +| `backend/workers/main.py` | Add `run_baseline_trial` to `WorkerSettings.functions`. | + +**Endpoints:** N/A. + +**Key interfaces** + +```python +# backend/workers/baseline.py +async def run_baseline_trial( + ctx: dict[str, Any], + study_id: str, + trial_id: str, + params: dict[str, Any], +) -> None: + """One-shot non-Optuna baseline trial. See spec FR-10.""" + +async def _existing_baseline_terminal_row( + db: AsyncSession, trial_id: str +) -> Trial | None: + """trial_id-based idempotency check (FR-10).""" +``` + +**Pydantic schemas:** N/A. + +**Tasks** + +1. Create `backend/workers/baseline.py` mirroring `backend/workers/trials.py` structure but stripped of all Optuna interaction. +2. On entry: idempotency check on `trial_id` (load `Trial` by id; if terminal, return no-op). +3. Load `Study`, `Cluster`, `QueryTemplate`, queries, qrels (same lookups as `run_trial`). +4. Build adapter via `build_adapter(cluster)`. +5. Render queries via `adapter.render(template, params, q.query_text)` for each query. +6. Resolve trial_timeout: `study.config.trial_timeout_s` or `Settings.studies_default_timeout_s`. +7. Call `adapter.search_batch(target, native_queries, top_k, strict_errors=False, timeout=trial_timeout_s)`. +8. Score via `score(qrels, run_dict, metrics_set)` (same metric set as `run_trial`: `{objective_key} | DEFAULT_SECONDARY_METRICS | study.config.secondary_metrics`). +9. INSERT the `Trial` row via `repo.create_trial(..., optuna_trial_number=-1, is_baseline=True, ...)` — need to extend `repo.create_trial` to accept `is_baseline` kwarg (default `False`). +10. On success: call `services.study_state.stamp_baseline_trial(db, study_id, trial_id, primary_metric)` (Story 1.3's helper) AND `await db.commit()` to durably land the stamp. (Plan F4: explicit commit required.) +11. On failure: persist `Trial` row with `status='failed'`, `is_baseline=TRUE`, `error=str(exc)[:500]`, then `await db.commit()`. Return normally (Arq treats as success). +12. Catch `IntegrityError` from the partial unique index — log + return (another worker already landed a complete baseline; this is the duplicate-INSERT-after-Arq-_job_id-bypass edge case). +13. Catch `SAOperationalError` and re-raise for Arq retry. +14. Wrap adapter aclose + structlog contextvars unbind in `try/finally`. +15. Add a **test-only fault seam** before the score step: `if os.environ.get("FEAT_STUDY_BASELINE_TRIAL_FAULT") == "delay_before_score": await asyncio.sleep(float(os.environ.get("FEAT_STUDY_BASELINE_TRIAL_FAULT_DELAY_S", "5")))`. Used by `test_baseline_late_completion_stamp.py` to force the orchestrator's wait to time out while the worker eventually completes. (Plan F9.) +16. Register in `backend/workers/main.py` `WorkerSettings.functions`. +17. Write unit tests with mocked adapter / scorer / qrels-loader: happy path, scorer raises, adapter raises, IntegrityError swallowed cleanly, structlog contextvars set + unset, the fault-seam delay path. + +**Definition of Done** + +- Unit test coverage ≥ 90% on `backend/workers/baseline.py`. +- 10+ unit tests covering happy path + 6+ failure paths. +- `run_baseline_trial` registered in `WorkerSettings.functions` (verify via `from backend.workers.main import WorkerSettings; assert run_baseline_trial in WorkerSettings.functions`). +- Worker self-stamp + commit lands the `studies.baseline_trial_id` on successful baseline completion (integration test seeds a fixture and asserts the UPDATE). +- Fault seam exercised by `test_baseline_late_completion_stamp.py`. +- AC-1, AC-3, AC-16 covered. + +--- + +### Story 1.5 — Schema updates: `baseline_params` request, `baseline_trial_id` response, `is_baseline` trial row (FR-6, FR-8, AC-9, AC-14) + +**Outcome:** `POST /api/v1/studies` accepts `config.baseline_params: dict[str, primitive] | None`; `GET /api/v1/studies/{id}` and `GET /api/v1/studies/{id}/trials` include the new fields. + +**New files** + +| File | Purpose | +|---|---| +| `backend/tests/contract/test_baseline_schemas.py` | Contract tests for the new request/response fields. | + +**Modified files** + +| File | Change | +|---|---| +| `backend/app/api/v1/schemas.py` | Add `baseline_params: dict[str, str \| int \| float \| bool \| None] \| None = None` to `StudyConfigSpec` (line 557-595). Add `baseline_trial_id: str \| None` to `StudyDetail` (line 668-698). Add `is_baseline: bool` to `TrialDetail` (line 724-737). | +| `backend/app/api/v1/studies.py` | Update `_detail` (line 121) to include `baseline_trial_id=row.baseline_trial_id`. Update `_trial_detail` (locate via grep) to include `is_baseline=row.is_baseline`. | +| `ui/src/lib/types.ts` | Regenerate from FastAPI OpenAPI schema (the types file is generated; rerun the generator). | + +**Endpoints** + +| Method | Path | Request body change | Response body change | Error codes | +|---|---|---|---|---| +| `POST` | `/api/v1/studies` | `config.baseline_params: dict[str, primitive] \| null` (optional) | `baseline_trial_id: str \| null` added to response | `VALIDATION_ERROR` (422) for non-primitive values | +| `GET` | `/api/v1/studies/{id}` | — | `baseline_trial_id: str \| null` added | — | +| `GET` | `/api/v1/studies/{id}/trials` | — | each row gets `is_baseline: bool` | — | + +**Key interfaces** + +```python +# backend/app/db/repo/trial.py +async def create_trial( + db: AsyncSession, + *, + id: str, + study_id: str, + optuna_trial_number: int, + params: dict[str, Any], + primary_metric: float | None, + metrics: dict[str, Any], + per_query_metrics: dict[str, Any] | None = None, + duration_ms: int | None, + status: str, + error: str | None, + started_at: datetime | None, + ended_at: datetime | None, + is_baseline: bool = False, # NEW +) -> Trial: ... +``` + +**Pydantic schemas** + +```python +class StudyConfigSpec(BaseModel): + # ... existing fields ... + baseline_params: dict[str, str | int | float | bool | None] | None = None + """feat_study_baseline_trial FR-6: explicit baseline params (tier b + of the resolver fallback). Stored in studies.config JSONB.""" + +class StudyDetail(BaseModel): + # ... existing fields ... + baseline_trial_id: str | None # NEW + +class TrialDetail(BaseModel): + # ... existing fields ... + is_baseline: bool # NEW +``` + +**Tasks** + +1. Add `baseline_params` field to `StudyConfigSpec`. Verify Pydantic rejects nested-dict values via a contract test. +2. Add `baseline_trial_id` to `StudyDetail`. Update `_detail` constructor in `studies.py:121`. +3. Add `is_baseline` to `TrialDetail`. Locate the trial-detail builder in `studies.py` (search for `TrialDetail(`) and add the field. (Repo signature already extended in Story 1.1.) +4. Write contract tests asserting: + - `POST /api/v1/studies` accepts `config.baseline_params={"foo": 1, "bar": "x"}`. + - `POST /api/v1/studies` rejects `config.baseline_params={"nested": {"dict": 1}}` with 422 `VALIDATION_ERROR`. + - `GET /api/v1/studies/{id}` response contains `baseline_trial_id` key (null when unset). + - `GET /api/v1/studies/{id}/trials` rows include `is_baseline` (false when unset). +5. Re-run `pnpm typecheck` in `ui/` after the OpenAPI types regenerate. + +**Definition of Done** + +- 6+ contract tests covering the new fields. +- TypeScript build (`pnpm build`) green with regenerated types. +- AC-9, AC-14 covered. + +--- + +### Story 1.6 — Repo filter updates: `is_baseline=FALSE` on aggregate / list reads (FR-11, AC-17) + +**Outcome:** Trials-aggregate read paths exclude baseline rows by default. Operators never see baseline mixed into Optuna trial counts, best-trial selection, or confidence inputs. + +**New files** + +| File | Purpose | +|---|---| +| `backend/tests/integration/test_trials_aggregate_excludes_baseline.py` | Asserts FR-11 filter behavior on real Postgres. | + +**Modified files** + +| File | Change | +|---|---| +| `backend/app/db/repo/trial.py` | Add `AND is_baseline = FALSE` to ONLY these helpers (plan-cycle-2 F1: narrow scope so the trials-listing API can still return baselines): `aggregate_trials_summary`, `list_complete_trials_for_confidence` (or whatever Q2 of the 4-query pattern is named; verify via grep), `list_top_trials` (the digest worker's top-10 helper), AND the auto-followup-input fetch (the repo helper that feeds `compute_first_decile_extremum`; verify via grep for callers of `compute_first_decile_max`). **DO NOT** add the filter to the helper backing `GET /api/v1/studies/{id}/trials` (likely named `list_trials_for_study` or similar — verify via grep). That endpoint MUST return baseline rows so the Story 3.1 UI toggle can reveal them. | +| `backend/workers/orchestrator.py` | Add `AND Trial.is_baseline == False` to `_last_n_all_failed` (line 320-337) and `_last_n_all_zero` (line 339-371) SELECT queries. Document the rationale inline (per spec FR-11 paragraph 2). | + +**Endpoints:** N/A. + +**Key interfaces** + +```python +# backend/app/db/repo/trial.py +@dataclass +class TrialsSummary: ... # unchanged + +async def aggregate_trials_summary(db: AsyncSession, study_id: str) -> TrialsSummary: + """Aggregate counts + best-trial selection for Optuna trials ONLY. + + FR-11: filters is_baseline=FALSE inline. Baseline trials are reported + via the separate StudyDetail.baseline_trial_id surface. + """ +``` + +**Tasks** + +1. Identify the in-scope helpers per the modified-files table above. **Explicitly do NOT** filter the trials-listing endpoint's repo helper (plan-cycle-2 F1). Add the filter inline to the in-scope helpers only. +2. Locate every direct SQL query in `backend/workers/` that reads `trials` (likely just the two orchestrator helpers). Add the filter + an inline comment citing FR-11. +3. Locate the digest worker's top-trials lookup (`backend/workers/digest.py` around the `_compute_top_trials` call). If it uses a repo function, the filter inherits; if direct SQL, add inline. +4. Update existing tests that assert `aggregate_trials_summary` results — they should still pass if no baseline row is seeded, but the test fixtures may need updating once Story 1.7 lands and integration tests seed baselines. +5. Write `test_trials_aggregate_excludes_baseline.py`: insert a study with 5 Optuna trials + 1 baseline trial (with the highest primary_metric); assert `summary.total == 5`, `summary.best_trial_id` is the best Optuna trial (NOT the baseline). Also assert the auto-followup fetch helper excludes baseline (the `first_decile_extremum` input list does not contain the baseline row). + +**Definition of Done** + +- AC-17 covered by integration test. +- Existing aggregate tests pass unchanged (no baseline rows seeded ⇒ no behavior difference). +- A new test asserts the trials-listing API helper (`list_trials_for_study` or equivalent) returns BOTH Optuna and baseline rows — the filter is NOT applied here (plan-cycle-2 F1 regression guard). + +--- + +### Story 1.7 — Orchestrator integration: baseline phase before Optuna (FR-2, AC-1, AC-2, AC-3) + +**Outcome:** `start_study` runs the baseline phase between search-space parse and Optuna polling. Synchronous wait with bounded timeout; calls FR-12 stamp helper on success; logs structured events for skip/fail/timeout. Resume path re-stamps existing complete baselines via the same helper. + +**New files** + +| File | Purpose | +|---|---| +| `backend/tests/integration/test_orchestrator_baseline_trial.py` | Real-backend end-to-end: study creation → baseline enqueue → wait → stamp → Optuna phase. | +| `backend/tests/integration/test_baseline_late_completion_stamp.py` | Worker self-stamp covers the timeout case. | +| `backend/tests/integration/test_baseline_resume.py` | resume_study handles all four baseline-row-state cases. | + +**Modified files** + +| File | Change | +|---|---| +| `backend/workers/orchestrator.py` | Insert baseline-resolution + enqueue + wait + stamp phase between line 170 (search-space parse) and line 173 (polling loop start). Use the polling pattern from line 182-188 (fresh session per tick). Reuse `_REPLENISH_TICK_S = 1.0` constant. Add helpers `_resolve_and_enqueue_baseline`, `_wait_for_baseline_trial`. | + +**Endpoints:** N/A. + +**Key interfaces** + +```python +# backend/workers/orchestrator.py +from typing import Literal +from dataclasses import dataclass + +@dataclass(frozen=True) +class BaselineEnqueueResult: + """Discriminated result from _resolve_and_enqueue_baseline. + + kind: + - "skipped": params resolution returned None — no baseline runs; proceed to Optuna immediately. + - "enqueued": fresh job accepted; wait by trial_id. + - "deduped": Arq rejected as duplicate (an earlier orchestrator invocation already + enqueued for this study); wait by study_id since the original trial_id is unknown. + """ + kind: Literal["skipped", "enqueued", "deduped"] + trial_id: str | None = None # set when kind == "enqueued" + +async def _resolve_and_enqueue_baseline( + db: AsyncSession, + arq_pool: ArqRedis, + study: Study, +) -> BaselineEnqueueResult: + """Resolve params via FR-3, enqueue baseline job with deterministic _job_id. + + (Plan-cycle-2 F2: the result type is a discriminated union, NOT a bare + str | None, so the caller can distinguish 'no baseline' from 'deduped'.) + """ + +async def _wait_for_baseline_trial_by_id( + session_factory: async_sessionmaker, + study_id: str, + trial_id: str, + wait_s: float, +) -> Trial | None: + """Poll the trials table by trial_id until terminal. + + Used when BaselineEnqueueResult.kind == 'enqueued'. + """ + +async def _wait_for_baseline_trial_by_study( + session_factory: async_sessionmaker, + study_id: str, + wait_s: float, +) -> Trial | None: + """Poll the trials table by study_id for any terminal is_baseline=TRUE row. + + Used when BaselineEnqueueResult.kind == 'deduped' — the trial_id from + the original enqueue is unknown to this orchestrator invocation, so + we observe any complete or failed baseline trial for the study. + """ +``` + +**Tasks** + +1. Insert new section "B'. Baseline phase" between sections B and C of `start_study` (or equivalent positioning per the current orchestrator structure). +2. Call `_resolve_and_enqueue_baseline(db, arq_pool, study)`. The helper internally calls `resolve_baseline_params` (Story 1.2) and enqueues with `_job_id=f"baseline:{study.id}"`. Returns a `BaselineEnqueueResult`. +3. **Dispatch on `result.kind`** (plan-cycle-2 F2): + - `"skipped"` (resolver returned None): log `event_type="baseline_skipped"` and proceed immediately to Optuna phase. Do NOT call wait helpers. + - `"enqueued"`: call `_wait_for_baseline_trial_by_id(..., trial_id=result.trial_id, wait_s)`. + - `"deduped"`: log `event_type="baseline_enqueue_deduped"` and call `_wait_for_baseline_trial_by_study(..., wait_s)`. +4. On terminal Trial row with `status='complete'` (from either wait helper): call `services.study_state.stamp_baseline_trial` (Story 1.3); then `await db.commit()` (plan F4); log `event_type="baseline_stamped"`. +5. On terminal Trial row with `status='failed'`: log `event_type="baseline_failed"` with the trial's `error` text; leave `baseline_trial_id IS NULL`; proceed. +6. On wait timeout: log `event_type="baseline_wait_timeout"`; leave NULL; proceed (worker will self-stamp later). +7. Resume path: in the orchestrator's existing entry logic, BEFORE the baseline phase, check whether a complete baseline row exists for the study. If yes + unstamped, call stamp helper + commit. If failed/pruned only, skip baseline. If none, run normally. +10. Write `test_orchestrator_baseline_trial.py` (real-backend, mock adapter at `search_batch`): creates a study → runs `start_study` → asserts baseline trial row written, baseline_trial_id stamped, Optuna trials enqueue afterwards. Covers AC-1, AC-2, AC-3 with 4 separate fixtures (each tier of the resolver hit). +11. Write `test_baseline_late_completion_stamp.py`: uses the Story 1.4 fault seam (`FEAT_STUDY_BASELINE_TRIAL_FAULT=delay_before_score` + `FEAT_STUDY_BASELINE_TRIAL_FAULT_DELAY_S=120`) to force the worker to outlast the orchestrator's wait; asserts orchestrator's wait times out, baseline_trial_id is NULL, then worker eventually completes and self-stamps. +12. Write `test_baseline_resume.py`: 4 scenarios (no baseline row → run from scratch; complete unstamped → re-stamp via helper; failed only → skip; complete already-stamped → idempotent). +13. Write `test_baseline_enqueue_deduped.py`: simulate the dedupe path by pre-enqueueing a `_job_id=f"baseline:{study_id}"` job, then invoking `start_study` and asserting: + - `_resolve_and_enqueue_baseline` returns `BaselineEnqueueResult(kind="deduped", trial_id=None)`. + - The orchestrator calls `_wait_for_baseline_trial_by_study`, NOT `_wait_for_baseline_trial_by_id`. + - On the original job's eventual completion, the stamp helper lands the FK on the studies row. +14. Write a unit test for the `BaselineEnqueueResult` discriminated-union — asserts the 3 kinds are mutually exclusive and the trial_id is non-None only for `"enqueued"` (regression guard for plan-cycle-2 F2). + +**Definition of Done** + +- AC-1, AC-2, AC-3, AC-16 covered by integration tests. +- New structured-log events emitted: `baseline_skipped`, `baseline_failed`, `baseline_stamped`, `baseline_wait_timeout`, `baseline_enqueue_deduped`. +- `make test-integration` green. + +--- + +### Phase Gate 1 — Foundation tests green + cross-model review + +**Hard gate**: cannot start Epic 2 until all of the following pass: + +1. `make test-unit` green. +2. `make test-integration` green. +3. `make test-contract` green. +4. `make typecheck` green. +5. `make lint` green. +6. Coverage on new files (`baseline_resolver.py`, `workers/baseline.py`, `stamp_baseline_trial` helper) ≥ 90%. +7. Migration round-trip clean. +8. GPT-5.5 phase-gate review pass with no High findings on the implementation diff. + +--- + +## Epic 2 — Consumer activation: confidence + auto-followup + digest flip (FR-4, FR-5, FR-7) + +The existing data-driven consumers flip from `runner_up` to `baseline` once the Epic 1 surfaces are populated. Each story is a tightly-scoped surgical change. + +### Story 2.1 — `compute_study_confidence` baseline branch + PR body contract test (FR-4, AC-4, AC-5, AC-6, AC-12) + +**Outcome:** When `study.baseline_trial_id` is set AND the baseline trial has `per_query_metrics`, the confidence orchestrator emits `comparison_against = "baseline"` (FR-4). Falls back to `"runner_up"` otherwise. The existing 5 tests asserting `runner_up` remain green (regression coverage). The PR body (rendered from `compute_study_confidence` output via `backend/workers/git_pr.py:513`) automatically emits "vs baseline" — covered by a new contract test fixture (plan F5). + +**New files**: None. + +**Modified files** + +| File | Change | +|---|---| +| `backend/app/domain/study/confidence.py` | Add `baseline_trial: Any \| None = None` kwarg to `compute_study_confidence`. At line 624, replace literal `comparison_against="runner_up"` with the FR-4 conditional. Update docstring. | +| `backend/app/services/study_confidence.py` | The fetch glue runs an additional query: `Q-1a: baseline_trial` (load by `study.baseline_trial_id` if non-NULL). Pass into `compute_study_confidence`. | +| `backend/tests/unit/domain/study/test_confidence.py` | Add 3+ tests for the baseline branch + 2+ regression tests for the fallback path. | +| `backend/tests/integration/test_studies_api_confidence.py` | Add an integration test with baseline+winner both having `per_query_metrics`; assert API response shows `comparison_against="baseline"`. | +| `backend/tests/contract/test_pr_body_confidence_section.py` | **Add a new fixture (plan F5)** with `baseline_trial_id` set + both winner + baseline have `per_query_metrics` + 12 regressors; assert PR body contains `"12 regressed (vs baseline)"`. The existing 2 `runner_up` fixtures stay green. AC-12 coverage. | + +**Endpoints:** N/A — response shape unchanged (`comparison_against` is already typed `Literal["runner_up", "baseline"]`). + +**Key interfaces** + +```python +# backend/app/domain/study/confidence.py +def compute_study_confidence( + *, + study_objective: dict[str, Any], + study_best_metric: float | None, + winner_trial: Any | None, + runner_up_trial: Any | None, + baseline_trial: Any | None = None, # NEW + complete_trials_summary: list[tuple[float, int]], + query_text_by_id: dict[str, str] | None = None, +) -> ConfidenceShape | None: + """... (FR-4: baseline branch when baseline_trial is non-None AND has per_query_metrics)""" +``` + +**Tasks** + +1. Add `baseline_trial: Any | None = None` to the `compute_study_confidence` signature. +2. At line 608 (the `if runner_up_trial is not None and winner_per_query and runner_up_trial.per_query_metrics:` block), wrap in a new conditional: prefer the baseline branch first. +3. Baseline branch: if `baseline_trial is not None and baseline_trial.per_query_metrics and winner_per_query`: call `compute_outcome_summary` with `comparison_per_query=baseline_trial.per_query_metrics`; build `PerQueryOutcomesShape(comparison_against="baseline", ...)`. +4. Fallback (runner-up): keep the existing branch verbatim. +5. Update `backend/app/services/study_confidence.py:fetch_study_confidence` to add a 5th query (or join into existing): load baseline trial by `study.baseline_trial_id`. Pass into `compute_study_confidence`. +6. Write unit tests covering AC-4 (baseline branch hits), AC-5 (baseline_trial_id NULL → runner_up), AC-6 (baseline trial has no per_query_metrics → runner_up). +7. Write integration test covering the full happy path via the API. +8. Confirm the 5 existing tests asserting `runner_up` still pass (regression coverage). + +**Definition of Done** + +- AC-4, AC-5, AC-6, AC-12 covered. +- All existing tests asserting `runner_up` still pass. +- Coverage on the new branch ≥ 95%. + +--- + +### Story 2.2 — `evaluate_chain_gate` direction-aware + lift-over-baseline (FR-5, AC-7, AC-8, AC-18) + +**Outcome:** Auto-followup gate computes lift against the explicit baseline (`parent.baseline_metric`) when set, falls back to first-decile-extremum otherwise. Direction-aware: minimize objectives invert the sign so "better than baseline" is always positive. Renames `compute_first_decile_max` → `compute_first_decile_extremum` (forward-only). + +**New files**: None. + +**Modified files** + +| File | Change | +|---|---| +| `backend/app/domain/study/auto_followup.py` | Rename `compute_first_decile_max` → `compute_first_decile_extremum`. Add `direction: Literal["maximize", "minimize"]` kwarg (default `"maximize"`). Modify `evaluate_chain_gate` to take `direction` (also default `"maximize"`), prefer `parent.baseline_metric` over first-decile when set, sign-flip lift for minimize. Rename `ChainGateOutcome.first_decile_max` → `first_decile_extremum`. Update module docstring (delete the "When feat_study_baseline_trial ships..." sentence). | +| `backend/workers/auto_followup.py` (or wherever `evaluate_chain_gate` is called from) | Pass `direction=parent.objective.get("direction", "maximize")` to the gate. | +| `backend/tests/unit/domain/study/test_auto_followup.py` | Update existing tests for the rename. Add tests covering baseline-branch (AC-7), fallback (AC-8), and minimize direction (AC-18). | + +**Endpoints:** N/A. + +**Key interfaces** + +```python +# backend/app/domain/study/auto_followup.py +def compute_first_decile_extremum( + complete_trials: Iterable[Any], + direction: Literal["maximize", "minimize"] = "maximize", +) -> float | None: ... + +def evaluate_chain_gate( + parent: Any, + complete_trials: Iterable[Any], + *, + direction: Literal["maximize", "minimize"] = "maximize", + epsilon: float = 0.005, +) -> ChainGateOutcome: ... + +@dataclass(frozen=True) +class ChainGateOutcome: + decision: ChainGateDecision + lift: float | None = None + first_decile_extremum: float | None = None # RENAMED + epsilon: float = 0.005 +``` + +**Tasks** + +1. Rename the helper function + the dataclass field. Update all call sites and tests (only inside `auto_followup.py` + its tests + 1 worker entry point). +2. Add `direction` kwarg. For minimize: `compute_first_decile_extremum` returns `min` of the first decile (not `max`); `evaluate_chain_gate` computes lift as `baseline_metric - best_metric` instead of `best_metric - baseline_metric`. +3. The gate decision (`if lift > epsilon: ENQUEUE`) stays unchanged — the lift is direction-normalized. +4. Update the module docstring per FR-5 ("FR-2b activated: when `parent.baseline_metric IS NOT NULL`, lift is computed directly against the baseline. Direction-aware via the `direction` argument (added 2026-05-25)."). +5. Update worker caller(s) to pass `direction=parent.objective.get("direction", "maximize")`. +6. Write/update unit tests covering AC-7 (baseline branch), AC-8 (fallback w/ rename), AC-18 (minimize direction). +7. Capture a 1-line entry in `docs/03_runbooks/auto-followup-debugging.md` noting the direction-awareness now in place. + +**Definition of Done** + +- AC-7, AC-8, AC-18 covered. +- All existing `feat_auto_followup_studies` tests still pass after rename. +- Runbook updated. + +--- + +### Story 2.3 — Digest system prompt + ConfidencePanel glossary update (FR-7) + +**Outcome:** The digest narrative LLM receives explicit baseline framing guidance. The ConfidencePanel tooltip glossary entry reflects both wire values. + +**New files**: None. + +**Modified files** + +| File | Change | +|---|---| +| `prompts/digest_narrative.system.md` | Add 1-2 sentence guidance per spec FR-7: "When `` has `comparison_against = 'baseline'`, regressors should be described as 'regressed vs the operator's current production baseline' — not 'vs the runner-up trial'. Lead the narrative with this baseline framing when present." | +| `ui/src/lib/glossary.ts` | Update `confidence.comparison_against` entry (line 676) with bi-state text that explains both wire values. Add new entry `trials.is_baseline` per spec UX tooltip table. | +| `backend/tests/unit/workers/test_digest_prompt_render.py` | Add a snapshot test for the system prompt's baseline-mention sentence. Add a `baseline`-branch render test (the existing 2 `runner_up` tests stay green). | +| `ui/src/__tests__/lib/glossary.test.ts` (if exists; else new) | Assert the new glossary entries' shape. | + +**Endpoints:** N/A. + +**Tasks** + +1. Edit `prompts/digest_narrative.system.md` near the existing line 36-37 (which already mentions both wire values). Add the FR-7 narrative-framing sentence. +2. Update `ui/src/lib/glossary.ts` `confidence.comparison_against` entry. Add `trials.is_baseline` entry. +3. Update any existing snapshot tests for the system prompt to include the new sentence. +4. Add a unit test for the digest user-prompt render when `comparison_against=baseline` is in the confidence payload. + +**Definition of Done** + +- AC-11, AC-12 covered. +- Existing digest prompt tests pass (with updated snapshot). +- Glossary entries renderable in UI (verify via the existing glossary test pattern). + +--- + +### Phase Gate 2 — Activation tests green + cross-model review + +**Hard gate**: cannot start Epic 3 until: + +1. All of Epic 1's gate criteria still pass. +2. `make test-unit && make test-integration && make test-contract` green. +3. The 5 tests originally asserting `runner_up == "runner_up"` still pass (regression). +4. The new baseline-branch tests pass. +5. GPT-5.5 phase-gate review pass with no High findings. + +--- + +## Epic 3 — Frontend: trials-table baseline filter + Baseline badge + ConfidencePanel data-driven flip (FR-9) + +### Story 3.1 — trials-table baseline filter toggle + Baseline badge (FR-9, AC-10) + +**Outcome:** The trials-table on the study-detail page filters out baseline rows by default. A "Show baseline trial" toggle reveals it at the top of the table with a "Baseline" badge. The ConfidencePanel label flip happens automatically via the data-driven branch in `confidence-panel.tsx` (already implemented — verify via the existing `confidence-panel.test.tsx:136` test). + +**New files**: None. + +**Modified files** + +| File | Change | +|---|---| +| `ui/src/components/studies/trials-table.tsx` | Add a `showBaseline: boolean` state (default `false`). Add a `` toggle labeled "Show baseline trial". Filter the trials data by `is_baseline === false` unless toggled. When toggled, prepend the baseline row at the top with a `Baseline` next to the trial-number cell. | +| `ui/src/components/studies/trials-table.column-config.tsx` | Conditional column rendering: if `row.is_baseline` true, render "Baseline" instead of `optuna_trial_number=-1`. | +| `ui/src/__tests__/components/studies/trials-table.test.tsx` (or new) | Assert default filter, toggle visibility, baseline badge rendering. | +| `ui/src/components/studies/confidence-panel.tsx` | No code change — the existing data-driven branch handles the flip (verify the existing test at line 136 still passes). Optionally update inline-help tooltips per spec §11. | + +**UI element inventory** + +| Element | Type | Label | Data source | Interaction | +|---|---|---|---|---| +| "Show baseline trial" toggle | `` | "Show baseline trial" | local state `showBaseline` | `onCheckedChange` flips visibility | +| "Baseline" badge | `` | "Baseline" | row.is_baseline | non-interactive | + +**State dependency analysis** + +State being added: `showBaseline: boolean` (local to ``). +Referenced by: only the filter computation + the toggle widget. No cross-component side effects. + +**Tasks** + +1. Locate the trials-table component (`ui/src/components/studies/trials-table.tsx`). +2. Add `showBaseline` useState. +3. Wrap the data filter: `const visibleTrials = showBaseline ? [baselineRow, ...optunaTrials] : optunaTrials` (where `baselineRow` is the trial with `is_baseline=true` if present). +4. Add the toggle component above the table. +5. Update the trial-number cell column-config to render "Baseline" for `is_baseline=true` rows. +6. Write a vitest unit test asserting the default-filter behavior and the toggle. +7. Run `pnpm typecheck` and `pnpm test`. + +**Definition of Done** + +- AC-10 covered. +- `pnpm typecheck && pnpm test` green. +- Existing tests (e.g., `study-action-bar-cascade.test.tsx`, `auto-followup-chain-panel.test.tsx`, `confidence-panel.test.tsx:136`) all pass. + +--- + +### Story 3.2 — E2E test for baseline trial flow (FR-9 coverage, AC-10, AC-12) + +**Outcome:** A real-backend Playwright test seeds a study via API with `config.baseline_params`, waits for completion, asserts ConfidencePanel renders "vs baseline" and the trials-table toggle reveals the Baseline badge. + +**New files** + +| File | Purpose | +|---|---| +| `ui/tests/e2e/baseline-trial.spec.ts` | Playwright E2E: API-seeded study with `baseline_params` → wait → assert UI. | + +**Modified files**: None. + +**Tasks** + +1. Use existing E2E test fixtures + helpers (cluster, query set, template, judgment list seeds). +2. POST a study via `page.request.post('/api/v1/studies', { data: { ..., config: { ..., baseline_params: { ... } } } })`. +3. Poll for study completion (existing helper). +4. Navigate to `/studies/{id}`. +5. Assert ConfidencePanel renders "vs baseline" (locator `text=/vs baseline/i`). +6. Click "Show baseline trial" toggle (locator `text=Show baseline trial`). +7. Assert "Baseline" badge appears (locator `text=Baseline >> first-row`). +8. Run `pnpm playwright test ui/tests/e2e/baseline-trial.spec.ts` against a real backend. + +**Definition of Done** + +- AC-10 covered by E2E. +- Test runs against real backend (no `page.route()` mocking per CLAUDE.md). +- E2E suite green. + +--- + +### Phase Gate 3 — Frontend + E2E green + +1. `pnpm typecheck && pnpm test && pnpm build` green. +2. `pnpm playwright test` (full suite) green. +3. Visual sanity check of the trials-table toggle on the dev stack. + +--- + +## Epic 4 — Documentation + final cross-model + close-out + +### Story 4.1 — Documentation updates (spec §15) + +**Outcome:** Architecture / runbook / data-model docs reflect the new columns + workflow. + +**Modified files** + +| File | Change | +|---|---| +| `docs/01_architecture/data-model.md` | Update §"studies" with `baseline_trial_id` row. Update §"trials" with `is_baseline` row + the `uq_trials_study_baseline_complete` partial index. | +| `docs/01_architecture/optimization.md` (if exists; else add a section to `mvp1-overview.md`) | Add a 2-3 sentence section "Baseline trial" describing the non-Optuna baseline phase. | +| `docs/03_runbooks/study-lifecycle-debugging.md` | Document the 5 new log event types: `baseline_skipped`, `baseline_failed`, `baseline_stamped`, `baseline_wait_timeout`, `baseline_enqueue_deduped`. Add a section "Baseline trial troubleshooting". | +| `docs/03_runbooks/auto-followup-debugging.md` | Note the direction-awareness fix in `evaluate_chain_gate` (FR-5). | +| `state.md` | Update active priorities + Alembic head (`0019` → `0020`) once merged. (This happens AFTER the impl-execute Step 8 finalization, not during this story.) | +| `architecture.md` | No change (the topical doc updates above suffice). | +| `CLAUDE.md` | No change (no new absolute rules). | + +**Tasks** + +1. Update `data-model.md` (search for the existing `studies` + `trials` table sections; add new column rows in the existing tables). +2. Update `study-lifecycle-debugging.md` with the new log events. +3. Update `auto-followup-debugging.md` with the direction-awareness note. +4. Update `optimization.md` (or `mvp1-overview.md` if optimization.md doesn't exist). + +**Definition of Done** + +- All docs in spec §15 updated. +- No `` markers left behind. + +--- + +### Story 4.2 — Final phase gate + GPT-5.5 review + close-out + +**Outcome:** Implementation is fully verified end-to-end and ready for PR. + +**Tasks** + +1. Run the full test suite: `make test && cd ui && pnpm test && pnpm playwright test`. +2. Run the full lint + typecheck stack: `make lint && make typecheck && cd ui && pnpm typecheck && pnpm lint`. +3. Verify coverage gate: `pytest --cov` ≥ 80%. New files ≥ 90%. +4. Verify migration round-trip on a fresh DB. +5. GPT-5.5 final review on the diff (impl-execute Step 7). +6. Adjudicate any Gemini Code Assist comments after PR opens (impl-execute Step 6). +7. Update `state.md` post-merge (impl-execute Step 8.5). + +**Definition of Done** + +- All test suites green. +- Coverage gate passes. +- Migration round-trips. +- GPT-5.5 final review has zero open High findings. +- PR open with the full test evidence in the description. + +--- + +## 3) Execution tracker + +| Story | Status | Started | Completed | Tests passing | Notes | +|---|---|---|---|---|---| +| 1.1 — Migration 0020 + ORM + `repo.create_trial(is_baseline=…)` | pending | — | — | — | — | +| 1.2 — `resolve_baseline_params` | pending | — | — | — | — | +| 1.3 — `stamp_baseline_trial` helper | pending | — | — | — | reordered before worker (plan F1) | +| 1.4 — `run_baseline_trial` worker | pending | — | — | — | reordered after helper (plan F1) | +| 1.5 — Request/response schemas | pending | — | — | — | — | +| 1.6 — Repo filter updates | pending | — | — | — | — | +| 1.7 — Orchestrator integration | pending | — | — | — | — | +| **Phase Gate 1** | pending | — | — | — | — | +| 2.1 — Confidence baseline branch | pending | — | — | — | — | +| 2.2 — Auto-followup gate | pending | — | — | — | — | +| 2.3 — Digest prompt + glossary | pending | — | — | — | — | +| **Phase Gate 2** | pending | — | — | — | — | +| 3.1 — trials-table baseline filter | pending | — | — | — | — | +| 3.2 — E2E test | pending | — | — | — | — | +| **Phase Gate 3** | pending | — | — | — | — | +| 4.1 — Documentation | pending | — | — | — | — | +| 4.2 — Final close-out | pending | — | — | — | — | + +## 4) Documentation update workstream (covered in Story 4.1) + +See Story 4.1 for the explicit doc-update list. The standalone workstream tracker is the story's DoD. + +## 5) Rollout + +- No feature flags. +- Forward-only (no backfill). +- Migration is additive — safe to land mid-cycle. +- Operator-visible change after PR merge: PR bodies will start showing real `delta_pct` for new studies, ConfidencePanel will start showing "vs baseline" for new studies. + +## 6) Risk register + +| Risk | Severity | Mitigation | +|---|---|---| +| Resume-race double-baseline | High (data correctness) | 3-layer defense per spec D-16 (`_job_id` dedupe + partial unique index + idempotent UPDATE predicate) | +| Long baseline trial blocks Optuna start | Medium (UX latency) | Wait timeout formula (FR-2 step 5) caps at 600s; worker self-stamp covers late completions | +| Existing tests asserting `comparison_against == "runner_up"` start failing | Low (regression coverage erosion) | New fixtures don't set `baseline_trial_id`, so existing fixtures hit the FR-4 fallback branch unchanged | +| Auto-followup gate direction inversion latent bug uncovered | Low | This feature fixes the bug; no regression risk | +| OpenAPI types regeneration breaks frontend build | Low | Caught by Story 1.5 + `pnpm typecheck` in Phase Gate 1 | diff --git a/docs/02_product/planned_features/feat_study_baseline_trial/pipeline_status.md b/docs/02_product/planned_features/feat_study_baseline_trial/pipeline_status.md new file mode 100644 index 00000000..d82b4c0a --- /dev/null +++ b/docs/02_product/planned_features/feat_study_baseline_trial/pipeline_status.md @@ -0,0 +1,36 @@ +# Pipeline status — `feat_study_baseline_trial` + +| Stage | Status | Date | Artifact | +|---|---|---|---| +| IDEA | ✅ Complete | 2026-05-22 (preflight-patched 2026-05-25) | [`idea.md`](idea.md) | +| SPEC | ✅ Complete | 2026-05-25 | [`feature_spec.md`](feature_spec.md) | +| PLAN | ✅ Complete | 2026-05-25 | [`implementation_plan.md`](implementation_plan.md) | +| IMPLEMENT | — | — | — | +| DONE | — | — | — | + +## SPEC stage details + +- **Cross-model review cycles**: 3 (max). Convergence reached. + - **Cycle 1 (Opus → GPT-5.5)**: 15 findings raised; 14 accepted + patched, 1 rejected with cited counter-evidence (CHECK constraint really does not include `'running'`; the worker only INSERTs at terminal state — verified at `backend/app/db/models/trial.py:48-51`). + - **Cycle 2 (with rejection log)**: 9 new findings (all genuinely new — no repeats); all accepted + patched. Included a forward-looking direction-aware fix for `evaluate_chain_gate` that closes a latent minimize-direction bug in `feat_auto_followup_studies`. + - **Cycle 3 (convergence check)**: 1 High-severity new finding — resume-race that could allow double-baseline-trial-INSERT. Accepted; patched with defense-in-depth (Arq `_job_id` dedupe + partial unique index + FR-12 stamping idempotency predicate). +- **Functional requirements**: 12 FRs (FR-1 through FR-12). +- **Acceptance criteria**: 18 ACs (AC-1 through AC-18). +- **Phases**: 1 (no further sub-phases). +- **Open questions remaining**: 0 (OQ-1 / OQ-2 / OQ-3 resolved inside the spec). +- **Touched surfaces**: 1 migration (0020), 1 new worker module (`backend/workers/baseline.py`), 1 new domain module (`backend/app/domain/study/baseline_resolver.py`), 1 new service helper (`services.study_state.stamp_baseline_trial`), orchestrator change (`backend/workers/orchestrator.py:start_study`), confidence one-line change (`confidence.py:624`), auto-followup gate (`auto_followup.py:91-169` direction-aware), digest system prompt (`prompts/digest_narrative.system.md`), API schema (`StudyDetail.baseline_trial_id`, `TrialDetail.is_baseline`), trials repo filter updates (`aggregate_trials_summary` + siblings), trials-table UI filter toggle. +- **Code estimate**: ~600-900 LOC backend; ~50 LOC frontend; ~400 LOC tests. + +## PLAN stage details + +- **Cross-model review cycles**: 3 (max). Convergence reached. + - **Cycle 1**: 10 findings (all accepted + patched) — story ordering issues, missing commits, AC-12 PR body coverage, FR-11 auto-followup filter, migration idempotency, SQLAlchemy placeholder convention, late-completion fault seam, resolver DoD scoping. + - **Cycle 2**: 2 new High findings (both accepted + patched) — overzealous repo filter would have hidden baseline from the trials-listing API; `_resolve_and_enqueue_baseline` return type couldn't distinguish skipped from deduped. + - **Cycle 3**: clean (`{"findings": []}`). Convergence. +- **Epics**: 4 (Foundation / Activation / Frontend / Close-out). +- **Stories**: 14 (1.1–1.7, 2.1–2.3, 3.1–3.2, 4.1–4.2) with explicit phase gates between epics. +- **Test estimate**: 30+ unit, 8+ integration, 4+ contract, 1 E2E. + +## Next action + +User invoked `--auto` — pipeline advances autonomously to IMPLEMENT stage (Epic 1 execution). diff --git a/migrations/versions/0020_studies_baseline_trial.py b/migrations/versions/0020_studies_baseline_trial.py new file mode 100644 index 00000000..03164536 --- /dev/null +++ b/migrations/versions/0020_studies_baseline_trial.py @@ -0,0 +1,108 @@ +"""studies_baseline_trial. + +Revision ID: 0020 +Revises: 0019 +Create Date: 2026-05-25 00:00:00.000000 + +feat_study_baseline_trial Story 1.1 / FR-1 — adds the two columns + one +partial unique index that activate the deferred-Phase-2 baseline-trial +work from feat_pr_metric_confidence: + +- ``studies.baseline_trial_id String(36) NULL`` — denormalized FK to the + baseline ``trials`` row (not a formal FK; same pattern as + ``best_trial_id``). +- ``trials.is_baseline BOOLEAN NOT NULL DEFAULT FALSE`` — marker for the + off-band non-Optuna baseline trial. +- ``uq_trials_study_baseline_complete`` — partial unique index on + ``trials (study_id) WHERE is_baseline = TRUE AND status = 'complete'`` + enforcing at-most-one-complete-baseline-per-study at the DB level + (defense against orchestrator double-enqueue on resume; one of the + three layers in feat_study_baseline_trial decision-log D-16). + +Per CLAUDE.md Absolute Rule #5, ships ``downgrade()`` and round-trips +cleanly. Both upgrade and downgrade are idempotent via ``IF [NOT] EXISTS`` +guards so re-running the migration is a no-op (no manual cleanup +required). +""" + +from collections.abc import Sequence + +from alembic import op + +revision: str = "0020" +down_revision: str | None = "0019" +branch_labels: str | Sequence[str] | None = None +depends_on: str | Sequence[str] | None = None + + +def upgrade() -> None: + # 1. studies.baseline_trial_id String(36) NULL — idempotent. + op.execute( + """ + DO $$ + BEGIN + IF NOT EXISTS ( + SELECT 1 FROM information_schema.columns + WHERE table_name = 'studies' AND column_name = 'baseline_trial_id' + ) THEN + ALTER TABLE studies ADD COLUMN baseline_trial_id VARCHAR(36); + END IF; + END $$; + """ + ) + + # 2. trials.is_baseline BOOLEAN NOT NULL DEFAULT FALSE — idempotent. + op.execute( + """ + DO $$ + BEGIN + IF NOT EXISTS ( + SELECT 1 FROM information_schema.columns + WHERE table_name = 'trials' AND column_name = 'is_baseline' + ) THEN + ALTER TABLE trials ADD COLUMN is_baseline BOOLEAN NOT NULL DEFAULT FALSE; + END IF; + END $$; + """ + ) + + # 3. Partial unique index — at most one COMPLETE baseline per study. + # CREATE INDEX IF NOT EXISTS makes this naturally idempotent. + op.execute( + """ + CREATE UNIQUE INDEX IF NOT EXISTS uq_trials_study_baseline_complete + ON trials (study_id) + WHERE is_baseline = TRUE AND status = 'complete'; + """ + ) + + +def downgrade() -> None: + # Reverse order: index first, then trials column, then studies column. + op.execute("DROP INDEX IF EXISTS uq_trials_study_baseline_complete;") + op.execute( + """ + DO $$ + BEGIN + IF EXISTS ( + SELECT 1 FROM information_schema.columns + WHERE table_name = 'trials' AND column_name = 'is_baseline' + ) THEN + ALTER TABLE trials DROP COLUMN is_baseline; + END IF; + END $$; + """ + ) + op.execute( + """ + DO $$ + BEGIN + IF EXISTS ( + SELECT 1 FROM information_schema.columns + WHERE table_name = 'studies' AND column_name = 'baseline_trial_id' + ) THEN + ALTER TABLE studies DROP COLUMN baseline_trial_id; + END IF; + END $$; + """ + ) diff --git a/prompts/digest_narrative.system.md b/prompts/digest_narrative.system.md index 89518177..60412bdc 100644 --- a/prompts/digest_narrative.system.md +++ b/prompts/digest_narrative.system.md @@ -31,12 +31,22 @@ The user message contains XML-delimited blocks: when its sub-field is null (FR-7 graceful-degradation contract). For studies still running, or studies whose winner trial predates the `per_query_metrics` migration, the block may be absent or partial. -9. `` (only when both the winner trial and the runner-up - trial have per-query metrics) — `improved` / `unchanged` / `regressed` - counts, the `comparison_against` reference (`runner_up` in MVP1; `baseline` - when Phase 2 ships), and up to 5 named regressor rows - (`query_text: winner_score → comparison_score (delta)`). Omitted entirely - when the comparison data isn't available. +9. `` (only when the winner trial has per-query metrics + AND a comparison trial — baseline OR runner-up — also has per-query + metrics) — `improved` / `unchanged` / `regressed` counts, the + `comparison_against` reference (`runner_up` OR `baseline`), and up to 5 + named regressor rows (`query_text: winner_score → comparison_score + (delta)`). Omitted entirely when the comparison data isn't available. + + **Narrative framing rule (feat_study_baseline_trial FR-7)**: when + `comparison_against = "baseline"`, regressors are queries that got + WORSE versus the operator's current production baseline — describe + them as "regressed vs the operator's current production baseline", + NOT "vs the runner-up trial". This is the more actionable framing + for approvers because it answers "does this PR change PROD?" directly. + Lead with this framing in the narrative's first sentence when present. + When `comparison_against = "runner_up"`, keep the existing "vs the + runner-up trial" framing — this is the no-baseline fallback. 10. `` (only when the worker passes it) — the parent study's `search_space` JSONB body (the same `{params: {name: {type, low, high, log?} | {type, low, high} | {type, choices: [...]}}}` shape diff --git a/ui/src/app/studies/[id]/page.tsx b/ui/src/app/studies/[id]/page.tsx index b2e833c1..4c929ec9 100644 --- a/ui/src/app/studies/[id]/page.tsx +++ b/ui/src/app/studies/[id]/page.tsx @@ -1,8 +1,10 @@ 'use client'; import Link from 'next/link'; -import { Suspense, use } from 'react'; +import { Suspense, use, useMemo, useState } from 'react'; import { DetailPageShell } from '@/components/common/detail-page-shell'; +import { Badge } from '@/components/ui/badge'; +import { Button } from '@/components/ui/button'; import { Card, CardContent, CardHeader, CardTitle } from '@/components/ui/card'; import { AutoFollowupChainPanel } from '@/components/studies/auto-followup-chain-panel'; import { ConfidencePanel } from '@/components/studies/confidence-panel'; @@ -75,23 +77,7 @@ export function StudyDetailView({ studyId }: { studyId: string }) { - - - Trials - - - - - + {study.status === 'completed' && digestQ.data && ( ; + urlState: ReturnType; + tableId: string; +}) { + const [showBaseline, setShowBaseline] = useState(false); + const data = trialsQ.data?.data ?? []; + const baselineRows = useMemo(() => data.filter((r) => r.is_baseline), [data]); + const visibleRows = useMemo( + () => (showBaseline ? data : data.filter((r) => !r.is_baseline)), + [data, showBaseline], + ); + + return ( + + + Trials + {baselineRows.length > 0 && ( +
+ + {showBaseline && ( + + Baseline + + )} +
+ )} +
+ + + +
+ ); +} + export default function StudyDetailPage({ params }: RouteProps) { const { id } = use(params); // `useSearchParams` (inside useDataTableUrlState) requires a Suspense boundary diff --git a/ui/src/lib/glossary.ts b/ui/src/lib/glossary.ts index e585886b..46029f28 100644 --- a/ui/src/lib/glossary.ts +++ b/ui/src/lib/glossary.ts @@ -690,9 +690,14 @@ export const glossary = { }, 'confidence.comparison_against': { short: - 'Reference for per-query comparison. Runner-up = second-best trial. Baseline = no-tuning trial (Phase 2).', + 'Comparison reference. "Baseline" = the no-tuning baseline trial (preferred). "Runner-up" = the second-best Optuna trial (fallback).', ariaLabel: 'More information about the comparison reference', }, + 'trials.is_baseline': { + short: + 'A no-tuning trial run before Optuna started, using your production params. Used as the comparison reference for the confidence outcomes.', + ariaLabel: 'More information about baseline trials', + }, // --------------------------------------------------------------------------- // feat_auto_followup_studies Story 3.1 — 4 chain-panel + wizard entries. diff --git a/ui/src/lib/types.ts b/ui/src/lib/types.ts index 441e584d..35462076 100644 --- a/ui/src/lib/types.ts +++ b/ui/src/lib/types.ts @@ -2066,8 +2066,27 @@ export interface components { /** * OpenAICapabilities * @description Cached results of the OpenAI capability check (Story 3.3 populates Redis). + * + * Step 1 (``models_endpoint``) is reported first because it gates the rest: + * when it fails, the other three are reported as ``"untested"``. The + * ``models_endpoint_status_code`` field is required-but-nullable + * (per ``bug_openai_capability_check_incapable_on_valid_key`` spec §19 D-3/D-8) + * — always present in the JSON, ``null`` when not applicable. This lets + * operators distinguish ``401 -> bad key``, ``429 -> quota``, + * ``5xx -> upstream outage``, ``null -> network unreachable / cache miss``. */ OpenAICapabilities: { + /** + * Models Endpoint + * @description GET /models probe outcome. 'ok' / 'fail' are projected from CapabilityResult.models_endpoint; 'untested' is the cache-miss default, matching the existing chat / function_calling / structured_output cache-miss handling. + * @enum {string} + */ + models_endpoint: 'ok' | 'fail' | 'untested'; + /** + * Models Endpoint Status Code + * @description HTTP status code from the GET /models probe when it HTTP-failed (>= 400). null for the success path, network-class failure (timeout / DNS / connection-refused), or cache miss. Required-but-nullable: the JSON key is always present with explicit null when no value, never omitted. + */ + models_endpoint_status_code: number | null; /** * Chat * @description Chat completion probe result @@ -2932,6 +2951,14 @@ export interface components { started_at: string | null; /** Ended At */ ended_at: string | null; + /** + * Is Baseline + * @description feat_study_baseline_trial FR-8 — TRUE only for the + * off-band non-Optuna baseline trial. Generated manually pending + * API-container rebuild + types:gen. + * @default false + */ + is_baseline?: boolean; }; /** * TrialListResponse