From b7ee204bdc17b5c19a532f6e5709c21d3cd2c7f2 Mon Sep 17 00:00:00 2001 From: SoundMindsAI Date: Thu, 21 May 2026 17:32:47 -0400 Subject: [PATCH 1/9] docs(feat-study-target-mismatch-guard): preflight + spec from /pipeline --auto Idea preflight applied 5 patches (line-ref fix, capability gaps closed, locked decisions section). Spec generated and reviewed through 3 cycles of GPT-5.5 cross-model review (17 findings accepted, 1 rejected with cited counter-evidence at create-study-modal.tsx:508). Major change vs the source idea: cycle-1 surfaced a cross-cluster judgment-list-reuse gap. FR-1b + JUDGMENT_CLUSTER_MISMATCH added so the validator catches both target-name mismatches AND physical-cluster mismatches (doc IDs are cluster-scoped; same target name across two clusters still produces zero overlap). Co-Authored-By: Claude Opus 4.7 --- docs/00_overview/DASHBOARD.md | 2 +- docs/00_overview/MVP1_DASHBOARD.md | 29 +- docs/00_overview/dashboard.html | 2 +- docs/00_overview/mvp1_dashboard.html | 61 +- .../feature_spec.md | 545 ++++++++++++++++++ .../idea.md | 45 +- .../pipeline_status.md | 20 + 7 files changed, 654 insertions(+), 50 deletions(-) create mode 100644 docs/02_product/planned_features/feat_study_target_judgment_mismatch_guard/feature_spec.md create mode 100644 docs/02_product/planned_features/feat_study_target_judgment_mismatch_guard/pipeline_status.md diff --git a/docs/00_overview/DASHBOARD.md b/docs/00_overview/DASHBOARD.md index ae9dd479..1c32a122 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-21**. Click a release na | Release | Theme | Progress | Status | |---|---|---|---| -| [MVP1 / v0.1](MVP1_DASHBOARD.md) | The Loop | 57 / 57 scoped done · 6 remaining | **In progress** | +| [MVP1 / v0.1](MVP1_DASHBOARD.md) | The Loop | 57 / 58 scoped done · 7 remaining | **In progress** | | [MVP2 / v0.2](MVP2_DASHBOARD.md) | Observable | 1 / 1 scoped done · 1 remaining | **In progress** | | MVP3 / v0.3 | Production Stacks | — | **Not yet scoped** | | MVP4 / v0.4 | Multi-tenant, Multi-LLM | — | **Not yet scoped** | diff --git a/docs/00_overview/MVP1_DASHBOARD.md b/docs/00_overview/MVP1_DASHBOARD.md index b76016c5..d2abe7f0 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-21** (latest mtime of any planned ## Next up -All scoped MVP1 features shipped 🎉 +**[feat_study_target_judgment_mismatch_guard](../02_product/planned_features/feat_study_target_judgment_mismatch_guard/feature_spec.md)** — Feature, currently in **Spec** -Pull from the Idea backlog or capture a new feature spec. +> `POST /api/v1/studies` rejects the mismatch at create time with a specific machine-readable error code (`JUDGMENT_TARGET_MISMATCH`, 422). + +Spec exists; run /pipeline to generate the implementation plan + ship + +```bash +/pipeline docs/02_product/planned_features/feat_study_target_judgment_mismatch_guard --auto +``` ## MVP1 Progress | Metric | Value | |---|---| -| Scoped items done | **57 / 57** (100%) — feat_/infra_/chore_/epic_ past idea stage | +| Scoped items done | **57 / 58** (98%) — feat_/infra_/chore_/epic_ past idea stage | | Pending work | **14** items (every not-done feat/infra/chore/bug across all priorities) | | → P0 — do next | **2** unblocking / paying daily cost | | → P1 | **6** high-value, ready when P0 clears | | → P2 (default) | 6 important to file, not blocking | | → Backlog | 0 captured for record, not planned | | Open bugs | 0 | -| Legacy "Path to MVP1" | 6 items — scoped-not-done + bugs + chore-ideas only (excludes feat/infra ideas) | -| Backlog ideas | 8 idea-only feat/infra (not yet scoped into MVP1) | +| Legacy "Path to MVP1" | 7 items — scoped-not-done + bugs + chore-ideas only (excludes feat/infra ideas) | +| Backlog ideas | 7 idea-only feat/infra (not yet scoped into MVP1) | | In flight | 0 feature(s) actively shipping | ## Pipeline @@ -109,15 +115,16 @@ _None._ _None._ -### Spec (0) +### Spec (1) -_None._ +| Priority | Feature | Type | One-liner | Depends on | Status | +|---|---|---|---|---|---| +| P0 | [feat_study_target_judgment_mismatch_guard](../02_product/planned_features/feat_study_target_judgment_mismatch_guard/feature_spec.md) | Feature | `POST /api/v1/studies` rejects the mismatch at create time with a specific machine-readable error code (`JUDGMENT_TARGET_MISMATCH`, 422). | — | [PR #163](https://github.com/SoundMindsAI/relyloop/pull/163) | -### Idea (14) +### Idea (13) | Priority | Feature | Type | One-liner | Depends on | Status | |---|---|---|---|---|---| -| P0 | [feat_study_target_judgment_mismatch_guard](../02_product/planned_features/feat_study_target_judgment_mismatch_guard/idea.md) | Feature | The existing POST `/studies` validator at [`backend/app/api/v1/studies.py:238`](../../backend/app/api/v1/studies.py#L238) enforces `judgment_list.query_set_id == body.query_set_id` (cross-set anti-enu | — | Idea — surfaced post-merge of `feat_pr_metric_confidence` | | P0 | [chore_e2e_test_rows_isolation](../02_product/planned_features/chore_e2e_test_rows_isolation/idea.md) | Chore | Multiple seed paths emit rows into the operator-visible `clusters` / `query_sets` / `query_templates` / `judgment_lists` / `studies` tables: | — | Idea — root cause behind the "study2 ran 1000 zero-metric trials" incident | | P1 | [feat_home_first_run_demo_nudge](../02_product/planned_features/feat_home_first_run_demo_nudge/idea.md) | Feature | The auto-seed chore solves the "fresh stack = empty dropdowns" problem. It does NOT solve: | — | Idea — product-design-shaped follow-up paired with the auto-seed-on-make-up chore | | P1 | [feat_orchestrator_zero_streak_abort](../02_product/planned_features/feat_orchestrator_zero_streak_abort/idea.md) | Feature | The two create-time guards close the bulk of the "all 1000 trials score 0" surface, but they don't cover every path: | — | Idea — defense-in-depth tier behind the create-time fail-fast guards | @@ -143,6 +150,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_target_judgment_mismatch_guard["study target judgment mismatch guard"] + class feat_study_target_judgment_mismatch_guard spec; infra_foundation["foundation"] class infra_foundation done; feat_study_lifecycle["study lifecycle"] @@ -279,6 +288,7 @@ graph LR feat_query_inline_crud --> chore_tutorial_polish feat_studies_ui --> chore_tutorial_polish feat_study_lifecycle --> chore_tutorial_polish + feat_study_target_judgment_mismatch_guard --> chore_tutorial_polish infra_adapter_elastic --> chore_tutorial_polish infra_ci_smoke_makeup --> chore_tutorial_polish infra_dashboard_regen_pre_commit_conflict --> chore_tutorial_polish @@ -308,6 +318,7 @@ graph LR feat_query_inline_crud --> feat_chat_agent feat_studies_ui --> feat_chat_agent feat_study_lifecycle --> feat_chat_agent + feat_study_target_judgment_mismatch_guard --> feat_chat_agent infra_adapter_elastic --> feat_chat_agent infra_ci_smoke_makeup --> feat_chat_agent infra_dashboard_regen_pre_commit_conflict --> feat_chat_agent diff --git a/docs/00_overview/dashboard.html b/docs/00_overview/dashboard.html index 3ed99a66..5e9ab17c 100644 --- a/docs/00_overview/dashboard.html +++ b/docs/00_overview/dashboard.html @@ -384,7 +384,7 @@

Releases

The Loop
-
57 / 57 scoped done · 6 remaining
+
57 / 58 scoped done · 7 remaining
In progress
diff --git a/docs/00_overview/mvp1_dashboard.html b/docs/00_overview/mvp1_dashboard.html index c344e761..4fbed695 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 Spec
+ +
`POST /api/v1/studies` rejects the mismatch at create time with a specific machine-readable error code (`JUDGMENT_TARGET_MISMATCH`, 422).
+
Spec exists; run /pipeline to generate the implementation plan + ship
+ /pipeline docs/02_product/planned_features/feat_study_target_judgment_mismatch_guard --auto
@@ -395,11 +395,11 @@

RelyLoop MVP1 Dashboard

MVP1 Progress

-
+
Scoped items done
-
57 / 57
-
100% of feat_/infra_/chore_/epic_ items past idea stage
-
+
57 / 58
+
98% of feat_/infra_/chore_/epic_ items past idea stage
+
Pending work
@@ -435,14 +435,14 @@

MVP1 Progress

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

Pipeline

-

Idea 14

- -
- -
- Feature - P0 - -
-
The existing POST `/studies` validator at [`backend/app/api/v1/studies.py:238`](../../backend/app/api/v1/studies.py#L238) enforces `judgment_list.query_set_id == body.query_set_id` (cross-set anti-enu
- - -
- +

Idea 13

@@ -649,7 +636,19 @@

Idea 14

-

Spec 0

+

Spec 1

+ +
+ +
+ Feature + P0 + PR #163 +
+
`POST /api/v1/studies` rejects the mismatch at create time with a specific machine-readable error code (`JUDGMENT_TARGET_MISMATCH`, 422).
+ + +
@@ -1575,6 +1574,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_target_judgment_mismatch_guard["study target judgment mismatch guard"] + class feat_study_target_judgment_mismatch_guard spec; infra_foundation["foundation"] class infra_foundation done; feat_study_lifecycle["study lifecycle"] @@ -1711,6 +1712,7 @@

Dependency graph (feat_ + infra_)

feat_query_inline_crud --> chore_tutorial_polish feat_studies_ui --> chore_tutorial_polish feat_study_lifecycle --> chore_tutorial_polish + feat_study_target_judgment_mismatch_guard --> chore_tutorial_polish infra_adapter_elastic --> chore_tutorial_polish infra_ci_smoke_makeup --> chore_tutorial_polish infra_dashboard_regen_pre_commit_conflict --> chore_tutorial_polish @@ -1740,6 +1742,7 @@

Dependency graph (feat_ + infra_)

feat_query_inline_crud --> feat_chat_agent feat_studies_ui --> feat_chat_agent feat_study_lifecycle --> feat_chat_agent + feat_study_target_judgment_mismatch_guard --> feat_chat_agent infra_adapter_elastic --> feat_chat_agent infra_ci_smoke_makeup --> feat_chat_agent infra_dashboard_regen_pre_commit_conflict --> feat_chat_agent @@ -1780,6 +1783,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_target_judgment_mismatch_guard["study target judgment mismatch guard"] + class feat_study_target_judgment_mismatch_guard spec; infra_foundation["foundation"] class infra_foundation done; feat_study_lifecycle["study lifecycle"] @@ -1916,6 +1921,7 @@

Dependency graph (feat_ + infra_)

feat_query_inline_crud --> chore_tutorial_polish feat_studies_ui --> chore_tutorial_polish feat_study_lifecycle --> chore_tutorial_polish + feat_study_target_judgment_mismatch_guard --> chore_tutorial_polish infra_adapter_elastic --> chore_tutorial_polish infra_ci_smoke_makeup --> chore_tutorial_polish infra_dashboard_regen_pre_commit_conflict --> chore_tutorial_polish @@ -1945,6 +1951,7 @@

Dependency graph (feat_ + infra_)

feat_query_inline_crud --> feat_chat_agent feat_studies_ui --> feat_chat_agent feat_study_lifecycle --> feat_chat_agent + feat_study_target_judgment_mismatch_guard --> feat_chat_agent infra_adapter_elastic --> feat_chat_agent infra_ci_smoke_makeup --> feat_chat_agent infra_dashboard_regen_pre_commit_conflict --> feat_chat_agent diff --git a/docs/02_product/planned_features/feat_study_target_judgment_mismatch_guard/feature_spec.md b/docs/02_product/planned_features/feat_study_target_judgment_mismatch_guard/feature_spec.md new file mode 100644 index 00000000..3c97e80d --- /dev/null +++ b/docs/02_product/planned_features/feat_study_target_judgment_mismatch_guard/feature_spec.md @@ -0,0 +1,545 @@ +# Feature Specification — feat_study_target_judgment_mismatch_guard + +**Date:** 2026-05-21 +**Status:** Draft +**Owners:** RelyLoop maintainers +**Related docs:** +- [idea.md](idea.md) +- [api-conventions.md](../../../01_architecture/api-conventions.md) +- [data-model.md](../../../01_architecture/data-model.md) +- Sibling: [feat_study_preflight_overlap_probe](../feat_study_preflight_overlap_probe/idea.md) +- Sibling: [feat_orchestrator_zero_streak_abort](../feat_orchestrator_zero_streak_abort/idea.md) +- Upstream: [chore_e2e_test_rows_isolation](../chore_e2e_test_rows_isolation/idea.md) + +**Depends on:** None. Feature is purely additive (no migration, no new external dep). Builds on the existing studies POST validator at [`backend/app/api/v1/studies.py:240-247`](../../../../backend/app/api/v1/studies.py#L240-L247) and the existing judgment-list listing filter pattern at [`backend/app/db/repo/judgment_list.py:58-140`](../../../../backend/app/db/repo/judgment_list.py#L58-L140). + +--- + +## 1) Purpose + +- **Problem:** Operators can create a study whose `study.target` (the index/collection the study queries) does not match the `judgment_list.target` (the index the judgments were authored against). When these mismatch, the judgment doc IDs cannot intersect search results from the study's target — pytrec_eval scores 0 on every (params, query) pair by construction. The orchestrator burns the entire trial budget producing `best_metric=0.0`. Real incident: study `019e4be6-207e-7c32-9889-f6c3003f57c2` ran 1000 trials in 4.5 minutes with `best_metric=0.0` because `study.target = "docs-articles"` but `judgment_list.target = "e2e-target"` (an E2E-test leftover). +- **Outcome:** `POST /api/v1/studies` rejects the mismatch at create time with a specific machine-readable error code (`JUDGMENT_TARGET_MISMATCH`, 422). The create-study modal pre-filters the judgment-list dropdown to only judgment lists matching the selected target, so the operator can't even submit the mismatched pair. The backend rejection is the defense-in-depth net for non-modal callers (chat-agent `create_study` tool, direct API users). +- **Non-goal:** Detecting *same-target-but-stale* mismatches (re-indexed corpus, judgments authored on a rotated index, etc.) — that's the sibling [feat_study_preflight_overlap_probe](../feat_study_preflight_overlap_probe/idea.md). Detecting mid-flight signal loss is the sibling [feat_orchestrator_zero_streak_abort](../feat_orchestrator_zero_streak_abort/idea.md). This feature covers ONLY the deterministic string-equality case. + +## 2) Current state audit + +### Existing implementations + +- **`backend/app/api/v1/studies.py:240-247`** — the `POST /api/v1/studies` cross-entity check that enforces `judgment_list.query_set_id == body.query_set_id`. Returns generic `VALIDATION_ERROR` (422). The new target-mismatch check sits directly after this, using the same `_err(...)` helper at [`studies.py:74-78`](../../../../backend/app/api/v1/studies.py#L74-L78). No structural change to the handler; one additional `if` block. +- **`backend/app/db/repo/judgment_list.py:58-112`** — `list_judgment_lists()` already accepts `query_set_id` + `cluster_id` filters (added by `bug_judgment_lists_listing_ignores_query_set_filter`, PR #163). The new `?target=` filter follows the same shape: parameter on the function signature → `WHERE` clause guarded by `if target is not None`. +- **`backend/app/db/repo/judgment_list.py:115-140`** — `count_judgment_lists()` mirrors the listing filters for `X-Total-Count` consistency. Must add `target` symmetrically. +- **`backend/app/api/v1/judgments.py:113-122`** — `_summary()` builder for the list response. Currently omits `target`. Detail at [`judgments.py:125-146`](../../../../backend/app/api/v1/judgments.py#L125-L146) includes `target=row.target`. The summary must add the same field so frontends can filter and label by target without fetching detail per row. +- **`backend/app/api/v1/schemas.py:760-769`** — `JudgmentListSummary` Pydantic model. Add `target: str` field (the underlying ORM column at [`backend/app/db/models/judgment_list.py:46`](../../../../backend/app/db/models/judgment_list.py#L46) is `Text, nullable=False`, so the Pydantic field is non-optional). +- **`backend/app/api/v1/judgments.py:339-383`** — `list_judgment_lists_endpoint` route. Add `target` query parameter alongside the existing `query_set_id` + `cluster_id` params (those are `Annotated[str | None, Query(min_length=1, max_length=36)]` because they're UUIDv7-shaped FKs). `target` is free-form so its bound is `min_length=1, max_length=255` (the ES/OpenSearch index-name ceiling — the underlying `Text` column has no length limit at the DB layer). +- **`ui/src/components/studies/create-study-modal.tsx:190-193`** — current `useJudgmentLists` call passes only `{ query_set_id, limit: 200 }`. Extend to `{ query_set_id, cluster_id, target, limit: 200 }`. `cluster_id` adoption is symmetric with the new `target` adoption (wire param exists, modal already has `clusterId` in scope at line 142; passing both is cheaper than passing one without the other). +- **`ui/src/components/studies/create-study-modal.tsx:594-597`** — existing `query_set_id`-change handler that resets `judgment_list_id` to `''`. New mirror handler on `target` change (at the existing Step-1 target picker at lines 539-561) must call `form.setValue('judgment_list_id', '')` for the same reason (a stale judgment_list_id from a prior target survives in the form state otherwise). +- **`ui/src/lib/api/judgments.ts:30-35`** — `JudgmentListsFilter` TypeScript interface. Add `target?: string | undefined` field. +- **`ui/src/lib/api/judgments.ts:37-51`** — `useJudgmentLists` hook. Destructure + pass `target` through `params` (mirrors the existing `query_set_id` + `cluster_id` pattern). +- **`ui/src/lib/types.ts`** — generated from live OpenAPI; must be regenerated after the `JudgmentListSummary` shape change. + +### Navigation and link impact + +| Source file | Current link target | New link target | +|---|---|---| +| (none) | (no URL/route changes) | — | + +### Existing test impact + +| Test file | Pattern | Count | Required change | +|---|---|---|---| +| `backend/tests/contract/test_studies_error_codes.py` | Asserts error envelopes for `POST /api/v1/studies` failure paths | (no change) | Add 1 new case for `JUDGMENT_TARGET_MISMATCH` (422). | +| `backend/tests/contract/test_studies_api_contract.py` | Asserts response shapes for studies endpoints | (no change) | (No change needed — this spec doesn't alter the success path of POST /studies.) | +| `backend/tests/contract/test_judgments_api_contract.py` | Asserts `JudgmentListSummary` OpenAPI shape | 1 | Update fixture to expect the new `target` field on the summary; assert OpenAPI surface lists `target` as required. | +| `backend/tests/integration/test_studies_api.py` | Studies POST integration tests (target='stub-index' at line 73) | (no change) | Add 1 new case that seeds a study + judgment list with mismatched `target` values → asserts 422 + `JUDGMENT_TARGET_MISMATCH`. | +| `backend/tests/integration/test_judgments_api.py` | Tests for `GET /api/v1/judgment-lists` listing | 1 | Add 1 new case asserting `?target=X` filters correctly across 2 lists with different targets. | +| `ui/src/components/studies/__tests__/create-study-modal*.test.tsx` | Create-study modal unit tests | (no change) | Add 1 vitest case asserting the dropdown filters by `target`; add 1 case asserting `target`-change resets `judgment_list_id`. | +| `ui/src/components/query-sets/associated-judgment-lists.tsx` (callers using `useJudgmentLists` without `target`) | Calls `useJudgmentLists({ query_set_id, limit: 50 })` | 1 | No change — `target` is optional; existing callers continue to work unfiltered (additive change). | +| `ui/src/app/page.tsx:66` (dashboard count widget) | Calls `apiClient.get('/api/v1/judgment-lists', ...)` for `X-Total-Count` header | 1 | No change — header-only call, additive `target` field on summary is ignored. | + +### Existing behaviors affected by scope change + +- **Existing studies with mismatched targets:** Current: pass create-time validation (only `query_set_id` cross-check exists); orchestrator runs all trials at 0.0 metric; finalize cleanly. New: such studies cannot be created via `POST /api/v1/studies`. Existing queued/running rows that already passed the (weaker) prior check are NOT retroactively rejected. **Decision needed: no** — locked in idea.md "Locked decisions". Mid-flight detection of pre-existing rows is owned by [feat_orchestrator_zero_streak_abort](../feat_orchestrator_zero_streak_abort/idea.md). +- **`JudgmentListSummary` wire shape:** Current: 7 fields (`id, name, description, query_set_id, cluster_id, status, created_at`). New: 8 fields with `target: str` added. **Decision needed: no** — additive for tolerant JSON consumers (the existing TanStack Query callers + the dashboard count widget at `ui/src/app/page.tsx:66` ignore unknown fields), but NOT no-impact for strict consumers: the OpenAPI snapshot at `backend/tests/contract/test_openapi_surface.py` and the generated TS types at `ui/src/lib/types.ts` BOTH must be regenerated in the same PR. Any future external client generating from the OpenAPI must also re-pull. +- **`GET /api/v1/judgment-lists` query params:** Current: 7 params (`cursor, limit, since, q, sort, query_set_id, cluster_id`). New: 8 params with `target` added. **Decision needed: no** — additive optional param; existing callers ignore. +- **`useJudgmentLists` TanStack queryKey:** Current key shape `['judgment-lists', { query_set_id, cluster_id, cursor, limit }]`. New: adds `target` to the key. Existing cache entries (keyed without `target`) coexist; new entries with `target` set are scoped separately. No cache invalidation needed. **Decision needed: no**. + +--- + +## 3) Scope + +### In scope + +- **(B1) Backend create-time rejection — target.** New `if judgment_list.target != body.target: raise _err(422, "JUDGMENT_TARGET_MISMATCH", ...)` block immediately after the existing `query_set_id` cross-check at `studies.py:240-247` and after the new cluster_id check (B1b). +- **(B1b) Backend create-time rejection — cluster_id.** New `if judgment_list.cluster_id != body.cluster_id: raise _err(422, "JUDGMENT_CLUSTER_MISMATCH", ...)` block between the `query_set_id` cross-check and the new target check. Closes the cross-cluster judgment-list reuse gap surfaced by GPT-5.5 cycle-1 review. +- **(B2) Backend listing surface extension.** Add `target: str` field to `JudgmentListSummary`; extend `_summary()` builder to populate it. Add `?target=` query param to `GET /api/v1/judgment-lists`; thread to `list_judgment_lists` + `count_judgment_lists` repo functions. +- **(B3) Error-code registration.** New `JUDGMENT_CLUSTER_MISMATCH` + `JUDGMENT_TARGET_MISMATCH` rows added to `docs/01_architecture/api-conventions.md` alongside the existing studies-endpoint codes (`SEARCH_SPACE_UNKNOWN_PARAM`, `SEARCH_SPACE_MISSING_DECLARED_PARAM`). +- **(F1) Frontend dropdown filtering.** Update create-study modal's `useJudgmentLists` call to pass `{ query_set_id, cluster_id, target, limit: 200 }`. The backend `?target=` filter does the work server-side; the dropdown only sees matching rows. +- **(F2) Frontend cascade reset.** When EITHER `target` OR `cluster_id` changes via the Step-1 picker, reset `judgment_list_id` to `''`. Two reset triggers: target change AND cluster change. Mirrors the existing `query_set_id`-change reset at line 596. Without the cluster trigger, an operator who picks cluster A → target `products` → judgment list → returns to Step 1 → switches cluster to B (target name still `products`) → submits would see the backend FR-1b 422 (because `judgment_list.cluster_id = A ≠ B = body.cluster_id`) — the cascade reset closes the loop in-UI. +- **(F3) Frontend empty-state copy.** When zero judgment lists match, render an empty-state via the existing `EntitySelect.emptyState` prop (precedent at [`create-study-modal.tsx:556-561`](../../../../ui/src/components/studies/create-study-modal.tsx#L556-L561) for the targets dropdown's `target_filter`-aware empty state). Copy: `"No judgment lists for target \"{target}\" on this cluster + query set. Generate a new one from /judgments."` with a `cta` linking to `/judgments`. +- **(F4) Frontend type regeneration.** Regenerate `ui/src/lib/types.ts` from live OpenAPI after the `JudgmentListSummary` shape change. Add `target?: string` to the `JudgmentListsFilter` interface in `ui/src/lib/api/judgments.ts`. + +### Out of scope + +- **Stale-judgment / overlap detection** — same target name, doc IDs disjoint (post-reindex, etc.). Owned by [feat_study_preflight_overlap_probe](../feat_study_preflight_overlap_probe/idea.md). +- **Mid-flight zero-streak abort** — orchestrator-level catch for studies that pass create-time gates but produce all-zero trials anyway. Owned by [feat_orchestrator_zero_streak_abort](../feat_orchestrator_zero_streak_abort/idea.md). +- **Retroactive rejection of pre-existing studies.** Pre-existing queued/running rows with mismatched targets continue executing. +- **Renaming the existing `VALIDATION_ERROR` code** returned by the `query_set_id` cross-check at `studies.py:242-247`. `api-conventions.md` line 196 forbids renaming shipped codes; the inconsistency (one specific, one generic) is accepted in exchange for backwards compatibility. +- **Enforcing `query_set.cluster_id == body.cluster_id` at `POST /studies`.** The studies handler at [`backend/app/api/v1/studies.py:228-247`](../../../../backend/app/api/v1/studies.py#L228-L247) does NOT currently cross-check that the query_set's `cluster_id` matches the study's `cluster_id`. This pre-existing gap is independent of the judgment-list mismatch this feature closes — queries themselves are plain text strings and don't have cluster-scoped doc IDs, so a `query_set.cluster_id` mismatch doesn't directly cause the zero-signal failure mode this feature targets. If a follow-up wants to close this contract inconsistency, file as `bug_studies_query_set_cluster_consistency`. The create-study modal already cascade-resets `query_set_id` on cluster change (verified at [`create-study-modal.tsx:508`](../../../../ui/src/components/studies/create-study-modal.tsx#L508)), so the operator can't accidentally trigger this from the UI. +- **Migration.** No schema changes; `judgment_lists.target` already exists. +- **Audit-event emission.** Pre-MVP2 — `audit_log` table not present yet. +- **Display of target in the dropdown rows.** The dropdown item label remains `j.name` (per [`create-study-modal.tsx:608`](../../../../ui/src/components/studies/create-study-modal.tsx#L608)) — adding a target subtitle is an ergonomic improvement deferred to a follow-up `chore_*` if needed; the empty-state copy already surfaces the target value to the operator. +- **Deep-link prefill on the empty-state CTA.** The `/judgments` page in MVP1 does not support URL prefill query params for the judgment-generation modal (verified — the route's create modal opens with empty defaults). Deep-linking `/judgments?cluster_id=...&query_set_id=...&target=...` would require a separate follow-up adding prefill support to the judgments-generate flow. For this feature, the CTA href is the static `/judgments` and the operator copies the target value mentally from the empty-state copy. + +### API convention check + +- **Endpoint prefix convention:** `/api/v1/` for business endpoints. Verified in [`backend/app/api/v1/studies.py:188`](../../../../backend/app/api/v1/studies.py#L188) (POST `/studies`) and [`backend/app/api/v1/judgments.py:335`](../../../../backend/app/api/v1/judgments.py#L335) (GET `/judgment-lists`). No new endpoints introduced. +- **Router file:** `backend/app/api/v1/studies.py` (B1); `backend/app/api/v1/judgments.py` (B2 endpoint). +- **HTTP methods:** No new endpoints; the existing POST `/studies` and GET `/judgment-lists` keep their methods. +- **Non-auth error envelope shape:** `{ "detail": { "error_code": "", "message": "", "retryable": } }` per the `_err(...)` helper at [`studies.py:74-78`](../../../../backend/app/api/v1/studies.py#L74-L78) (identical to [`judgments.py:86-90`](../../../../backend/app/api/v1/judgments.py#L86-L90)). The new `JUDGMENT_TARGET_MISMATCH` follows this exact shape. +- **Auth error shape:** N/A in MVP1 (no auth surface). + +### Phase boundaries + +Single phase. The entire feature ships in one PR. No deferred phases. + +## 4) Product principles and constraints + +- **Fail fast on deterministic problems.** A string-equality mismatch costs one comparison; catch it at the API boundary, not 4.5 minutes into the trial budget. +- **Backend contract, frontend UX.** The 422 is the contract; the modal filter is a UX prefetch. Chat-agent and direct API callers fall through to the same backend gate (no separate enforcement). +- **Specific over generic error codes** when the failure has a deterministic recovery path. The frontend can render a targeted helper UI (link to judgment generation against the right target) instead of a generic toast. +- **Don't rename shipped error codes.** `VALIDATION_ERROR` returned by the existing `query_set_id` cross-check stays as-is per `api-conventions.md:196`. + +### Anti-patterns + +- **Do not** make the frontend filter judgment lists client-side after over-fetching — duplicates the count-semantics work the backend already does, and the dropdown's "no matches" empty state becomes inconsistent with `X-Total-Count`. Use the wire filter. +- **Do not** add `target` only to `JudgmentListDetail` and require a per-row detail fetch on the frontend — that's N+1 latency on the modal open. Add it to the summary. +- **Do not** rename the existing `VALIDATION_ERROR` returned for the `query_set_id` mismatch — would silently break any external caller (or future chat-agent error handler) branching on that code. +- **Do not** raise the new error before the FK-existence and `query_set_id` checks. The order in [`studies.py:206-247`](../../../../backend/app/api/v1/studies.py#L206-L247) is meaningful (FK first → query_set consistency → target consistency); reordering changes which 404 vs 422 the caller sees for ambiguous failures. +- **Do not** retroactively reject pre-existing studies. The check fires only at `POST /studies`; existing rows are out-of-scope. + +## 5) Assumptions and dependencies + +- **`judgment_lists.target` is non-nullable.** Confirmed at [`backend/app/db/models/judgment_list.py:46`](../../../../backend/app/db/models/judgment_list.py#L46) (`Mapped[str] = mapped_column(Text, nullable=False)`). Every row has a target, so the equality check is always well-defined. +- **`studies.target` is non-nullable.** Confirmed at [`backend/app/db/models/study.py:50`](../../../../backend/app/db/models/study.py#L50). +- **`CreateStudyRequest.target` is required.** Confirmed at [`studies.py:259`](../../../../backend/app/api/v1/studies.py#L259) (`target=body.target` passed unconditionally to `repo.create_study`). +- **No FTS impact.** `judgment_lists.search_vector` is `GENERATED ALWAYS AS … STORED` over `name + target` per migration `0012_search_vector_judgment_lists.py`. Adding a wire `?target=` filter is orthogonal to the existing FTS `?q=` path — operators can use either. +- **OpenAPI surface lock.** `backend/tests/contract/test_openapi_surface.py` snapshots the OpenAPI schema. The summary's new `target` field will fail this snapshot until updated — must be in the same PR. + +## 6) Actors and roles + +- **Primary actor:** Relevance engineer (per umbrella spec §6). +- **Role model:** N/A — single-tenant install, no auth surface (MVP1). +- **Permission boundaries:** N/A. + +### Authorization + +N/A — single-tenant install, no auth surface (per [`docs/01_architecture/tech-stack.md` "Canonical release matrix"](../../../01_architecture/tech-stack.md)). + +### Audit events + +N/A — `audit_log` lands at MVP2 per [`docs/01_architecture/data-model.md` §"Reserved for later releases"](../../../01_architecture/data-model.md). Pre-MVP2, mutations do not emit audit events. + +## 7) Functional requirements + +### FR-1: Reject mismatched targets at POST /studies + +- Requirement: + - The system **MUST** return HTTP 422 with `error_code = "JUDGMENT_TARGET_MISMATCH"` from `POST /api/v1/studies` when the resolved `judgment_list.target` does not equal `body.target` (byte-for-byte string equality — no normalization, no case-folding, no whitespace trimming). + - The system **MUST** evaluate this check AFTER the FK resolution (`JUDGMENT_LIST_NOT_FOUND`, `QUERY_SET_NOT_FOUND`, `CLUSTER_NOT_FOUND`, `TEMPLATE_NOT_FOUND`), AFTER the existing `query_set_id` cross-check at [`studies.py:240-247`](../../../../backend/app/api/v1/studies.py#L240-L247), and AFTER the new `cluster_id` cross-check from FR-1b. + - The error message **MUST** include both target values for operator diagnosis and **SHOULD** suggest the two recovery paths (regenerate judgments against the study's target, OR change the study target to the judgment list's target). + - The error envelope **MUST** match the canonical shape from `_err(...)` at [`studies.py:74-78`](../../../../backend/app/api/v1/studies.py#L74-L78): `{"detail": {"error_code": "JUDGMENT_TARGET_MISMATCH", "message": "...", "retryable": false}}`. +- Notes: Equality check, no normalization. ES/OpenSearch index names are case-sensitive (lowercased by convention but not enforced); a target string of `Docs-Articles` differs from `docs-articles` at the index layer and must differ here too. The 422 status matches the existing `query_set_id` mismatch precedent at line 242. + +### FR-1b: Reject mismatched cluster_id at POST /studies + +- Requirement: + - The system **MUST** return HTTP 422 with `error_code = "JUDGMENT_CLUSTER_MISMATCH"` from `POST /api/v1/studies` when the resolved `judgment_list.cluster_id` does not equal `body.cluster_id`. + - The system **MUST** evaluate this check AFTER the FK resolution and AFTER the existing `query_set_id` cross-check at [`studies.py:240-247`](../../../../backend/app/api/v1/studies.py#L240-L247), and BEFORE the FR-1 target check (cluster mismatch is the broader failure — even with matching target name, different physical clusters have different doc IDs). + - The error envelope **MUST** match the canonical shape: `{"detail": {"error_code": "JUDGMENT_CLUSTER_MISMATCH", "message": "...", "retryable": false}}`. +- Notes: This closes the residual gap GPT-5.5 cycle-1 surfaced — without this check, a chat-agent or direct API caller could submit `body.cluster_id=B` + `judgment_list_id=` (with `judgment_list.cluster_id=A`, `judgment_list.target="foo"`) + `body.target="foo"` and pass FR-1 because target names match. Doc IDs are scoped to the physical cluster, so distinct clusters with the same target name still produce zero overlap. The `judgment_list.cluster_id` column exists at [`backend/app/db/models/judgment_list.py:45`](../../../../backend/app/db/models/judgment_list.py#L45) (`String(36) NOT NULL FK clusters.id`). + +### FR-2: Extend GET /api/v1/judgment-lists with ?target= filter + +- Requirement: + - The system **MUST** accept an optional `?target=` query parameter on `GET /api/v1/judgment-lists`. + - When `target` is present, the system **MUST** filter the listing AND the `X-Total-Count` header to only rows where `judgment_lists.target = target` (exact byte-for-byte match, no `LIKE`, no normalization). + - The system **MUST** combine `?target=` with the existing `?query_set_id=` + `?cluster_id=` filters using AND semantics (consistent with the pattern at [`backend/app/db/repo/judgment_list.py:87-90, 133-136`](../../../../backend/app/db/repo/judgment_list.py#L87-L90)). + - The parameter **MUST** be bounded `min_length=1, max_length=255`. Rationale: 255 is the Elasticsearch / OpenSearch index-name ceiling (the only meaningful cap on a target string today; the underlying `Text` column has no length limit at the DB layer). Out-of-bounds → 422 with `error_code = "VALIDATION_ERROR"` per the canonical envelope from `validation_exception_handler` at [`backend/app/api/errors.py:102-118`](../../../../backend/app/api/errors.py#L102-L118) (the project translates FastAPI's `RequestValidationError` into the standard `{detail: {error_code, message, retryable}}` shape — not FastAPI's default `detail: [{...}]` list). +- Notes: Out-of-bounds (length 0 or >255) returns the canonical `VALIDATION_ERROR` envelope — no new code needed. The same envelope applies to the existing `query_set_id` / `cluster_id` length violations on this endpoint. + +### FR-3: Add target to JudgmentListSummary + +- Requirement: + - The system **MUST** add `target: str` (non-nullable) to the `JudgmentListSummary` Pydantic model at [`backend/app/api/v1/schemas.py:760-769`](../../../../backend/app/api/v1/schemas.py#L760-L769). + - The `_summary()` builder at [`backend/app/api/v1/judgments.py:113-122`](../../../../backend/app/api/v1/judgments.py#L113-L122) **MUST** populate the new field from `row.target`. + - The OpenAPI surface contract test **MUST** be updated to assert `target` is present in the `JudgmentListSummary` schema definition. +- Notes: Additive non-breaking change. Existing TanStack Query consumers tolerate unknown additive fields. The OpenAPI shape-lock test catches the surface drift and must be updated in the same PR. + +### FR-4: Frontend create-study modal — target-aware dropdown + cascade reset + +- Requirement: + - The frontend **MUST** pass `target` (sourced from `form.watch('target')`) through the `useJudgmentLists` filter at [`create-study-modal.tsx:190-193`](../../../../ui/src/components/studies/create-study-modal.tsx#L190-L193): `useJudgmentLists({ query_set_id: querySetId || undefined, cluster_id: clusterId || undefined, target: target || undefined, limit: 200 })`. `cluster_id` is added at the same time for symmetry. + - When EITHER `target` (via the Step-1 picker — `` manual mode at line 527, `` dropdown mode at lines 546-561) OR `cluster_id` (via the cluster picker — existing `onChange` at line 504-507 already resets `target` per the prior FR-4 cascade; this spec extends that same handler to ALSO reset `judgment_list_id`) changes, the frontend **MUST** reset `judgment_list_id` to `''`. Mirror the existing `query_set_id`-change reset at line 596. + - When the filtered list returns zero rows, the dropdown **MUST** render the existing `EntitySelect.emptyState` (per [`entity-select.tsx:33-36`](../../../../ui/src/components/common/entity-select.tsx#L33-L36)) with message `"No judgment lists for target \"{target}\" on this cluster + query set. Generate a new one from /judgments."` and `cta = { label: "Generate judgments", href: "/judgments" }`. + - The `JudgmentListsFilter` interface at [`ui/src/lib/api/judgments.ts:30-35`](../../../../ui/src/lib/api/judgments.ts#L30-L35) **MUST** add `target?: string | undefined`. + - The TanStack queryKey **MUST** include `target` so cache entries are correctly scoped (existing cache entries keyed without `target` remain valid for filter-less callers like `associated-judgment-lists.tsx`). + - The generated OpenAPI types at `ui/src/lib/types.ts` **MUST** be regenerated to reflect the new `JudgmentListSummary.target` field and the new wire filter param. +- Notes: Step 1 (`'Cluster + target'`) gates Step-2 advance with `Boolean(values.cluster_id && values.target)` (per [`create-study-modal.tsx:384`](../../../../ui/src/components/studies/create-study-modal.tsx#L384)) — `target` is always set by the time the user reaches the judgment-list dropdown, so the "no target yet" branch the original idea proposed is dead code and not implemented. + +## 8) API and data contract baseline + +### 7.1 Endpoint surface + +| Method | Path | Purpose | Key error codes | +|---|---|---|---| +| `POST` | `/api/v1/studies` | Create + enqueue start_study | `JUDGMENT_CLUSTER_MISMATCH` (422 — new, fires first), `JUDGMENT_TARGET_MISMATCH` (422 — new); existing: `CLUSTER_NOT_FOUND` (404), `TEMPLATE_NOT_FOUND` (404), `QUERY_SET_NOT_FOUND` (404), `JUDGMENT_LIST_NOT_FOUND` (404), `INVALID_SEARCH_SPACE` (400), `SEARCH_SPACE_UNKNOWN_PARAM` (400), `SEARCH_SPACE_MISSING_DECLARED_PARAM` (400), `VALIDATION_ERROR` (422). | +| `GET` | `/api/v1/judgment-lists` | List with cursor pagination + filters | `VALIDATION_ERROR` (422 — cursor decode or query-param bounds). | + +### 7.2 Contract rules + +- Error body **MUST** include `error_code` (machine-readable, never renamed once shipped). +- Status code 422 **MUST** be deterministic per scenario: target mismatch → `JUDGMENT_TARGET_MISMATCH`; query_set mismatch (existing) → `VALIDATION_ERROR`; FK lookup → 404 with the specific entity code. +- The new validator **MUST** fire AFTER all FK resolutions to preserve the 404-before-422 ordering operators rely on. + +### 7.3 Response examples + +**Success — POST /api/v1/studies (existing shape, unchanged):** +```json +{ + "id": "01990000-0000-7000-8000-000000000001", + "name": "tune-products-boost", + "cluster_id": "01990000-0000-7000-8000-000000000010", + "target": "products", + "template_id": "01990000-0000-7000-8000-000000000020", + "query_set_id": "01990000-0000-7000-8000-000000000030", + "judgment_list_id": "01990000-0000-7000-8000-000000000040", + "search_space": {"params": {"boost": {"kind": "float", "low": 0.5, "high": 10}}}, + "objective": {"metric": "ndcg", "k": 10, "direction": "maximize"}, + "config": {"max_trials": 100, "sampler": "tpe", "pruner": "median"}, + "status": "queued", + "failed_reason": null, + "optuna_study_name": "01990000-0000-7000-8000-000000000001", + "...": "...remaining StudyDetail fields..." +} +``` + +**Failure — POST /api/v1/studies, target mismatch (NEW):** HTTP 422 +```json +{ + "detail": { + "error_code": "JUDGMENT_TARGET_MISMATCH", + "message": "judgment_list target='e2e-target' does not match study target='products'; judgments would have no overlap with search results from the study's target. Use a judgment list generated against 'products' or change study.target to 'e2e-target'.", + "retryable": false + } +} +``` + +**Success — GET /api/v1/judgment-lists?target=products (NEW filter, summary shape extended):** HTTP 200, header `X-Total-Count: 2` +```json +{ + "data": [ + { + "id": "01990000-0000-7000-8000-000000000040", + "name": "products-judgments-v1", + "description": "Initial 200-judgment seed.", + "query_set_id": "01990000-0000-7000-8000-000000000030", + "cluster_id": "01990000-0000-7000-8000-000000000010", + "target": "products", + "status": "complete", + "created_at": "2026-05-20T18:42:11.000Z" + }, + { + "id": "01990000-0000-7000-8000-000000000041", + "name": "products-judgments-v2", + "description": null, + "query_set_id": "01990000-0000-7000-8000-000000000030", + "cluster_id": "01990000-0000-7000-8000-000000000010", + "target": "products", + "status": "complete", + "created_at": "2026-05-21T09:14:02.000Z" + } + ], + "next_cursor": null, + "has_more": false +} +``` + +**Failure — GET /api/v1/judgment-lists?target=<empty>:** HTTP 422 (FastAPI's `VALIDATION_ERROR` for `min_length=1` violation; shape per FastAPI's `RequestValidationError` handler — same as any other bounds violation on this endpoint, no new code). + +### 7.4 Enumerated value contracts + +| Field | Accepted values (exact) | Backend source of truth | Frontend call site(s) | +|---|---|---|---| +| `POST /studies` `error_code` | `JUDGMENT_CLUSTER_MISMATCH`, `JUDGMENT_TARGET_MISMATCH` (both new, in firing order); existing per `studies.py` `_err(...)` call sites | `backend/app/api/v1/studies.py` (`_err(...)` invocations) — codes are string literals, not a centralized Literal | None on frontend yet — the modal pre-filters so the operator can't submit a mismatch. The chat agent's `create_study` tool surfaces the error via the orchestrator's existing 422-handler; no new branching needed. | +| `GET /judgment-lists` `?target=` | Any non-empty string up to 255 chars | `judgments.py:347-348` (FastAPI `Annotated[str \| None, Query(min_length=1, max_length=255)]`) | `useJudgmentLists` filter — but the value flows through opaquely; no allowlist on the frontend side. | + +No new option lists, status enums, or filter chips introduced. No new ` keeps RHF register intact via targetReg wrapper - Empty-state copy unconditional (Step-2 is target-gated per FR-4) - OpenAPI snapshot scope correctly bounded to schema + query param only (existing test only enumerates success-status tuples; error codes are locked at the runtime layer via test_studies_error_codes.py + new ordering test in test_studies_api_contract.py + api-conventions.md) - ui/src/lib/types.ts owned by Story 1.1 (regen) not Story 2.1 (consume) Co-Authored-By: Claude Opus 4.7 --- docs/00_overview/MVP1_DASHBOARD.md | 18 +- docs/00_overview/mvp1_dashboard.html | 22 +- .../implementation_plan.md | 852 ++++++++++++++++++ .../pipeline_status.md | 7 +- 4 files changed, 878 insertions(+), 21 deletions(-) create mode 100644 docs/02_product/planned_features/feat_study_target_judgment_mismatch_guard/implementation_plan.md diff --git a/docs/00_overview/MVP1_DASHBOARD.md b/docs/00_overview/MVP1_DASHBOARD.md index d2abe7f0..1357c7cb 100644 --- a/docs/00_overview/MVP1_DASHBOARD.md +++ b/docs/00_overview/MVP1_DASHBOARD.md @@ -6,14 +6,14 @@ _Reflects feature-folder state as of **2026-05-21** (latest mtime of any planned ## Next up -**[feat_study_target_judgment_mismatch_guard](../02_product/planned_features/feat_study_target_judgment_mismatch_guard/feature_spec.md)** — Feature, currently in **Spec** +**[feat_study_target_judgment_mismatch_guard](../02_product/planned_features/feat_study_target_judgment_mismatch_guard/feature_spec.md)** — Feature, currently in **Plan** > `POST /api/v1/studies` rejects the mismatch at create time with a specific machine-readable error code (`JUDGMENT_TARGET_MISMATCH`, 422). -Spec exists; run /pipeline to generate the implementation plan + ship +Plan approved; run /impl-execute to ship ```bash -/pipeline docs/02_product/planned_features/feat_study_target_judgment_mismatch_guard --auto +/impl-execute docs/02_product/planned_features/feat_study_target_judgment_mismatch_guard/implementation_plan.md --all ``` ## MVP1 Progress @@ -111,16 +111,16 @@ Spec exists; run /pipeline to generate the implementation plan + ship _None._ -### Plan (0) - -_None._ - -### Spec (1) +### Plan (1) | Priority | Feature | Type | One-liner | Depends on | Status | |---|---|---|---|---|---| | P0 | [feat_study_target_judgment_mismatch_guard](../02_product/planned_features/feat_study_target_judgment_mismatch_guard/feature_spec.md) | Feature | `POST /api/v1/studies` rejects the mismatch at create time with a specific machine-readable error code (`JUDGMENT_TARGET_MISMATCH`, 422). | — | [PR #163](https://github.com/SoundMindsAI/relyloop/pull/163) | +### Spec (0) + +_None._ + ### Idea (13) | Priority | Feature | Type | One-liner | Depends on | Status | @@ -151,7 +151,7 @@ graph LR classDef spec fill:#dbeafe,stroke:#1e40af,color:#1e40af; classDef idea fill:#f1f5f9,stroke:#334155,color:#334155; feat_study_target_judgment_mismatch_guard["study target judgment mismatch guard"] - class feat_study_target_judgment_mismatch_guard spec; + class feat_study_target_judgment_mismatch_guard plan; infra_foundation["foundation"] class infra_foundation done; feat_study_lifecycle["study lifecycle"] diff --git a/docs/00_overview/mvp1_dashboard.html b/docs/00_overview/mvp1_dashboard.html index 4fbed695..8ef6fcdd 100644 --- a/docs/00_overview/mvp1_dashboard.html +++ b/docs/00_overview/mvp1_dashboard.html @@ -383,11 +383,11 @@

RelyLoop MVP1 Dashboard

-
Next up — Feature, currently in Spec
+
Next up — Feature, currently in Plan
`POST /api/v1/studies` rejects the mismatch at create time with a specific machine-readable error code (`JUDGMENT_TARGET_MISMATCH`, 422).
-
Spec exists; run /pipeline to generate the implementation plan + ship
- /pipeline docs/02_product/planned_features/feat_study_target_judgment_mismatch_guard --auto +
Plan approved; run /impl-execute to ship
+ /impl-execute docs/02_product/planned_features/feat_study_target_judgment_mismatch_guard/implementation_plan.md --all
@@ -636,7 +636,12 @@

Idea 13

-

Spec 1

+

Spec 0

+ +
+ +
+

Plan 1

@@ -652,11 +657,6 @@

Spec 1

-
-

Plan 0

- -
-

Implementing 0

@@ -1575,7 +1575,7 @@

Dependency graph (feat_ + infra_)

classDef spec fill:#dbeafe,stroke:#1e40af,color:#1e40af; classDef idea fill:#f1f5f9,stroke:#334155,color:#334155; feat_study_target_judgment_mismatch_guard["study target judgment mismatch guard"] - class feat_study_target_judgment_mismatch_guard spec; + class feat_study_target_judgment_mismatch_guard plan; infra_foundation["foundation"] class infra_foundation done; feat_study_lifecycle["study lifecycle"] @@ -1784,7 +1784,7 @@

Dependency graph (feat_ + infra_)

classDef spec fill:#dbeafe,stroke:#1e40af,color:#1e40af; classDef idea fill:#f1f5f9,stroke:#334155,color:#334155; feat_study_target_judgment_mismatch_guard["study target judgment mismatch guard"] - class feat_study_target_judgment_mismatch_guard spec; + class feat_study_target_judgment_mismatch_guard plan; infra_foundation["foundation"] class infra_foundation done; feat_study_lifecycle["study lifecycle"] diff --git a/docs/02_product/planned_features/feat_study_target_judgment_mismatch_guard/implementation_plan.md b/docs/02_product/planned_features/feat_study_target_judgment_mismatch_guard/implementation_plan.md new file mode 100644 index 00000000..8dfecbb7 --- /dev/null +++ b/docs/02_product/planned_features/feat_study_target_judgment_mismatch_guard/implementation_plan.md @@ -0,0 +1,852 @@ +# Implementation Plan — feat_study_target_judgment_mismatch_guard + +**Date:** 2026-05-21 +**Status:** Draft +**Primary spec:** [feature_spec.md](feature_spec.md) +**Policy source(s):** [api-conventions.md](../../../01_architecture/api-conventions.md), [CLAUDE.md](../../../../CLAUDE.md) + +--- + +## 0) Planning principles + +- Spec traceability first: every story/task maps to FR IDs from the spec. +- One epic, three stories — the feature is bounded enough that a single epic gate is sufficient. +- Backend wire surface ships before the frontend filter (Story 1.1 → Story 2.1 dependency). +- Backend validators (Story 1.2) are independent and can ship in any order vs Story 1.1, but conventionally land first because they're the contract. +- Single PR: all three stories land in one branch + one PR. The scope is small (~100 LOC + ~80 LOC tests) and the artifacts are tightly coupled (frontend depends on backend, contract tests assert on shipped envelopes). + +## 1) Scope traceability (FR → epics/stories) + +| FR ID | Epic / Story | Notes | +|---|---|---| +| FR-1 (target mismatch validator) | Epic 1 / Story 1.2 | New 422 `JUDGMENT_TARGET_MISMATCH` block after FR-1b cluster check. | +| FR-1b (cluster_id mismatch validator) | Epic 1 / Story 1.2 | New 422 `JUDGMENT_CLUSTER_MISMATCH` block before FR-1 target check. | +| FR-2 (`?target=` listing filter) | Epic 1 / Story 1.1 | Add wire param + thread through `list_judgment_lists` + `count_judgment_lists`. | +| FR-3 (`target` on `JudgmentListSummary`) | Epic 1 / Story 1.1 | Add field on Pydantic model + populate via `_summary()`. | +| FR-4 (frontend filter + cascade + empty state) | Epic 1 / Story 2.1 | Extend `useJudgmentLists` filter, cascade reset, empty-state copy. | + +All 5 FRs covered. No phase boundary (single-phase ship per spec §3). No tracking file for deferred phases needed. + +## 2) Delivery structure + +Epic → Story → Tasks → DoD. + +### Conventions (project-specific) + +Honors all RelyLoop conventions from `CLAUDE.md`: +- All repo functions take `db: AsyncSession` as first arg; use `db.flush()` (caller commits) +- Routers return typed Pydantic response models; errors use the `_err(...)` helper at `studies.py:74-78` / `judgments.py:86-90` which raises `HTTPException` with the canonical `{detail: {error_code, message, retryable}}` envelope. +- Pydantic v2 models with non-optional fields when the underlying ORM column is `NOT NULL`. +- Frontend: `` options (only a filter param on an existing wire). +- Conventional Commits format for every commit (`feat(...)`, `test(...)`, `docs(...)`). + +### AI Agent Execution Protocol + +0. Load context: `architecture.md`, `state.md`, this plan, the spec. +1. Implement Story 1.1 first (backend listing extension) — frontend Story 2.1 depends on it. +2. Implement Story 1.2 (validators) — independent; runs in parallel with 1.1 if a second engineer is available. +3. Implement Story 2.1 (frontend filter + cascade + empty state). +4. Run backend tests (unit + integration + contract subset for touched endpoints) after Story 1.1 and Story 1.2. +5. Run frontend tests (vitest) after Story 2.1. +6. Regenerate OpenAPI types: `make openapi-export` (or the project's equivalent) — verify `ui/src/lib/types.ts` reflects the new `target` summary field + the new `?target=` query param. +7. Update `docs/01_architecture/api-conventions.md` (both new error code rows) and `state.md` (recent changes) in the same PR. +8. Run `make lint`, `make typecheck`, `cd ui && pnpm typecheck && pnpm lint && pnpm build`. +9. Attach evidence in the PR description. + +--- + +## Epic 1 — Reject mismatched-target studies at create time + +### Story 1.1 — Extend GET /api/v1/judgment-lists with target filter + summary field + +**Outcome:** `GET /api/v1/judgment-lists?target=` filters to lists with that target, with `X-Total-Count` mirroring the filter; `JudgmentListSummary` carries the new `target` field so the frontend can render target-aware UI without per-row detail fetches. + +**Traces to:** FR-2, FR-3. + +**New files** + +None. All edits are additive to existing files. + +**Modified files** + +| File | Change | +|---|---| +| `backend/app/api/v1/schemas.py` | Add `target: str` (non-nullable) to `JudgmentListSummary` at lines 760-769 (between `cluster_id` and `status` to mirror `JudgmentListDetail` order at lines 772-788). | +| `backend/app/api/v1/judgments.py` | Update `_summary(row)` at lines 113-122 to populate `target=row.target`. Add `target: Annotated[str \| None, Query(min_length=1, max_length=255)] = None` query param to `list_judgment_lists_endpoint` at lines 339-348 and thread to `repo.list_judgment_lists(target=target, ...)` + `repo.count_judgment_lists(target=target, ...)`. | +| `backend/app/db/repo/judgment_list.py` | Add `target: str \| None = None` keyword arg to `list_judgment_lists` (lines 58-68) and `count_judgment_lists` (lines 115-122). Apply `if target is not None: stmt = stmt.where(JudgmentList.target == target)` in BOTH functions (mirror the existing `query_set_id` / `cluster_id` pattern at lines 87-90 + 133-136). | +| `backend/app/db/repo/__init__.py` | No change — `list_judgment_lists` and `count_judgment_lists` are already exported. | +| `ui/src/lib/types.ts` | **Generated artifact** — regenerated from live OpenAPI as part of Story 1.1 Task #5. Owned by Story 1.1 because that's the story that changes the backend OpenAPI surface. Story 2.1 consumes the regenerated types. | + +**Endpoints** + +| Method | Path | Request body | Success response | Error codes | +|---|---|---|---|---| +| `GET` | `/api/v1/judgment-lists` | — (query params: `cursor`, `limit`, `since`, `q`, `sort`, `query_set_id`, `cluster_id`, **`target` (new)**) | `200` `JudgmentListListResponse` with `X-Total-Count` header. Each `data[].target: str` (new field). | `VALIDATION_ERROR` (422) via `errors.py:102-118` for cursor decode or bounds violations. No new error codes. | + +**Pydantic schemas** + +```python +# backend/app/api/v1/schemas.py +class JudgmentListSummary(BaseModel): + """List-view row on ``GET /api/v1/judgment-lists``.""" + id: str + name: str + description: str | None + query_set_id: str + cluster_id: str + target: str # NEW — non-nullable, mirrors underlying Text NOT NULL column + status: JudgmentListStatusWire + created_at: datetime +``` + +**Key interfaces** + +```python +# backend/app/db/repo/judgment_list.py +async def list_judgment_lists( + db: AsyncSession, + *, + cursor: tuple[object, str] | None = None, + limit: int = 50, + since: datetime | None = None, + q: str | None = None, + sort: str | None = None, + query_set_id: str | None = None, + cluster_id: str | None = None, + target: str | None = None, # NEW +) -> list[JudgmentList]: ... + +async def count_judgment_lists( + db: AsyncSession, + *, + since: datetime | None = None, + q: str | None = None, + query_set_id: str | None = None, + cluster_id: str | None = None, + target: str | None = None, # NEW +) -> int: ... +``` + +**Tasks** + +1. Add `target: str` field to `JudgmentListSummary` in `schemas.py` (alphabetically positioned per existing convention). +2. Update `_summary(row)` in `judgments.py:113-122` to populate `target=row.target`. +3. Add `target` query param to `list_judgment_lists_endpoint` in `judgments.py` with the right `min_length=1, max_length=255` bounds. +4. Add `target` keyword arg to `list_judgment_lists` and `count_judgment_lists` repo functions; mirror the `WHERE judgment_lists.target = target` clause from the existing `query_set_id`/`cluster_id` filters. +5. Regenerate `ui/src/lib/types.ts` from live OpenAPI. The project's standard regen workflow is the canonical path (check `Makefile` and `package.json` scripts for the existing command; if none exists, this PR must wire one — manual hand-edits of `types.ts` are NOT permitted because they drift from the OpenAPI source). +6. Audit existing UI fixtures: `grep -rn "JudgmentListSummary\|JudgmentListListResponse" ui/src/ ui/tests/` to find every object literal constructed against the summary type. Add `target: ''` to each (existing tests typically use `'products'` or `'e2e-target'`). This is an additive type change — `pnpm typecheck` will surface every drop. + +**Definition of Done** + +- [ ] `GET /api/v1/judgment-lists?target=products` returns only rows with `target=products` (integration test asserts data array AND `X-Total-Count` header match). +- [ ] `GET /api/v1/judgment-lists?target=products&cluster_id=C1&query_set_id=Q1` applies AND-semantics (integration test #5 from spec §14). +- [ ] `GET /api/v1/judgment-lists?target=<empty>` returns 422 with `error_code = "VALIDATION_ERROR"` (FastAPI `min_length` violation routed through `errors.py:validation_exception_handler`). +- [ ] `GET /api/v1/judgment-lists?target=<256 chars>` returns 422 with `error_code = "VALIDATION_ERROR"`. +- [ ] Every `JudgmentListSummary` response includes `target: str` (contract test asserts OpenAPI shape). +- [ ] OpenAPI snapshot contract test passes after `JudgmentListSummary.target` is recognized as required. +- [ ] `ui/src/lib/types.ts` regenerated and shows `target: string` on `JudgmentListSummary`. + +--- + +### Story 1.2 — POST /api/v1/studies cross-entity validators (cluster + target) + +**Outcome:** `POST /api/v1/studies` rejects mismatched-cluster and mismatched-target judgment-list pairings with two new specific 422 error codes (`JUDGMENT_CLUSTER_MISMATCH`, `JUDGMENT_TARGET_MISMATCH`), in firing order: FR-1b (cluster) before FR-1 (target). + +**Traces to:** FR-1, FR-1b. + +**New files** + +None. + +**Modified files** + +| File | Change | +|---|---| +| `backend/app/api/v1/studies.py` | Insert two new `if`-blocks between the existing `query_set_id` check at lines 240-247 and the config serialization at line 249. Block 1 (FR-1b, cluster check) at the new lines 248-256 (approx). Block 2 (FR-1, target check) at the new lines 257-265 (approx). Both use the existing `_err(...)` helper at line 74. | +| `backend/tests/contract/test_studies_api_contract.py` | Add ordering-test cases asserting the firing sequence `JUDGMENT_LIST_NOT_FOUND` (404) → `VALIDATION_ERROR` (query_set mismatch, 422) → `JUDGMENT_CLUSTER_MISMATCH` (422) → `JUDGMENT_TARGET_MISMATCH` (422). Contract-layer assertion locks the order so a future refactor reordering the if-blocks fails CI. | +| `docs/01_architecture/api-conventions.md` | Add two new rows to the studies-endpoint error-code table (after `SEARCH_SPACE_MISSING_DECLARED_PARAM` at lines 76-77): `JUDGMENT_CLUSTER_MISMATCH` (422, retryable=false) and `JUDGMENT_TARGET_MISMATCH` (422, retryable=false), in firing order. | + +**Endpoints** + +| Method | Path | Request body | Success response | Error codes | +|---|---|---|---|---| +| `POST` | `/api/v1/studies` | `CreateStudyRequest` (unchanged shape from `schemas.py:589`) | `201` `StudyDetail` (unchanged) | **`JUDGMENT_CLUSTER_MISMATCH` (422 — new)**, **`JUDGMENT_TARGET_MISMATCH` (422 — new)**, plus existing: `CLUSTER_NOT_FOUND` (404), `TEMPLATE_NOT_FOUND` (404), `QUERY_SET_NOT_FOUND` (404), `JUDGMENT_LIST_NOT_FOUND` (404), `INVALID_SEARCH_SPACE` (400), `SEARCH_SPACE_UNKNOWN_PARAM` (400), `SEARCH_SPACE_MISSING_DECLARED_PARAM` (400), `VALIDATION_ERROR` (422). | + +**Pydantic schemas** + +No new schemas. `CreateStudyRequest` already has `cluster_id`, `target`, and `judgment_list_id` fields. + +**Key interfaces** + +No new functions. Two new `if`-blocks inside the existing `create_study` handler: + +```python +# backend/app/api/v1/studies.py — between the existing query_set_id check +# (line 247) and the config serialization (line 249). + +# FR-1b: cluster_id consistency. Fires BEFORE the target check because cluster +# mismatch is the broader failure — even with matching target name, distinct +# physical clusters have distinct doc IDs. +if judgment_list.cluster_id != body.cluster_id: + raise _err( + 422, + "JUDGMENT_CLUSTER_MISMATCH", + ( + f"judgment_list cluster_id={judgment_list.cluster_id!r} does not " + f"match study cluster_id={body.cluster_id!r}; judgments are scoped " + f"to the cluster they were authored on. Pick a judgment list " + f"created against cluster {body.cluster_id!r} or change the " + f"study's cluster." + ), + False, + ) + +# FR-1: target consistency. Fires AFTER the cluster check. +if judgment_list.target != body.target: + raise _err( + 422, + "JUDGMENT_TARGET_MISMATCH", + ( + f"judgment_list target={judgment_list.target!r} does not match " + f"study target={body.target!r}; judgments would have no overlap " + f"with search results from the study's target. Use a judgment " + f"list generated against {body.target!r} or change study.target " + f"to {judgment_list.target!r}." + ), + False, + ) +``` + +**Tasks** + +1. Add the two `if`-blocks to `studies.py` between lines 247 and 249. Cluster check first (FR-1b), then target check (FR-1). +2. Add two new rows to `docs/01_architecture/api-conventions.md` error-code table (after `SEARCH_SPACE_MISSING_DECLARED_PARAM` at lines 76-77). Both rows: HTTP 422, `retryable: false`, with the same message guidance as the spec §7.5. + +**Definition of Done** + +- [ ] `POST /api/v1/studies` with mismatched `cluster_id` returns 422 `JUDGMENT_CLUSTER_MISMATCH` AND no `studies` row is inserted AND no Arq job is enqueued (integration assertion uses an Arq spy/mock + `SELECT COUNT(*) FROM studies` before/after). Contract test locks envelope shape. +- [ ] `POST /api/v1/studies` with matching `cluster_id` but mismatched `target` returns 422 `JUDGMENT_TARGET_MISMATCH` AND no `studies` row + no Arq enqueue (same assertion pattern). +- [ ] `POST /api/v1/studies` with both matching returns 201 `StudyDetail` AND inserts the row with the requested target AND `start_study(study_id)` is enqueued exactly once (integration — happy path, AC-2). +- [ ] `POST /api/v1/studies` with a non-existent `judgment_list_id` returns 404 `JUDGMENT_LIST_NOT_FOUND` — the new validators never fire (integration AND contract — ordering test, AC-3). +- [ ] `POST /api/v1/studies` with mismatched `query_set_id` (existing check) returns 422 `VALIDATION_ERROR` — the new validators never fire (integration AND contract — ordering test, AC-4). +- [ ] `POST /api/v1/studies` with matching `target` but mismatched `cluster_id` returns `JUDGMENT_CLUSTER_MISMATCH` (NOT `JUDGMENT_TARGET_MISMATCH`) — verifies firing order (integration AND contract — AC-11). Contract-layer assertion locks the order so a future reordering breaks CI. +- [ ] `api-conventions.md` includes both new rows in firing order. +- [ ] `GET /api/v1/studies/{id}` for a pre-existing fixture row with mismatched target returns 200 (integration — AC-10 negative test). + +--- + +### Story 2.1 — Create-study modal: target-aware filter + cascade reset + empty state + +**Outcome:** The create-study modal Step-2 judgment-list dropdown filters by the Step-1 target via the new `?target=` wire param; changing target OR cluster cascade-resets `judgment_list_id`; an empty filter result renders an empty-state copy with a CTA linking to `/judgments`. + +**Traces to:** FR-4. + +**New files** + +| File | Purpose | +|---|---| +| `ui/src/components/studies/__tests__/create-study-modal.target-filter.test.tsx` | New vitest file with 5 cases per §3.5 (target-aware filter call shape, manual-mode cascade, dropdown-mode cascade, cluster-mode cascade regression-lock, empty-state copy + CTA). | + +**Modified files** + +| File | Change | +|---|---| +| `ui/src/components/studies/create-study-modal.tsx` | (a) Lines 190-193: extend `useJudgmentLists` call to pass `{ query_set_id, cluster_id, target, limit: 200 }`. `clusterId` is already in scope at line 142; `target` is already watched at line 143. (b) Line 527 (manual-mode ``) and lines 539-561 (dropdown-mode ``): extend both target-setting `onChange` handlers to ALSO call `form.setValue('judgment_list_id', '')`. The manual `` continues to use `form.register('target')` — pattern below preserves RHF's `name`/`ref`/`onBlur` wiring (see Story-2.1 task #3 for the exact JSX). (c) Lines 603-612: add `emptyState` prop to the `` rendering the judgment-list dropdown. Use the existing prop shape per [`entity-select.tsx:33-36`](../../../../ui/src/components/common/entity-select.tsx#L33-L36). | +| `ui/src/lib/api/judgments.ts` | (Owned by Story 2.1 only — removed from Story 1.1.) Lines 30-35: add `target?: string \| undefined` to `JudgmentListsFilter`. Lines 37-51: destructure `target` + thread through `params` AND `queryKey`. | +| `ui/src/lib/types.ts` | (Owned by Story 1.1 — not modified in Story 2.1.) Story 2.1 consumes the regenerated types. If existing TS fixtures construct `JudgmentListSummary` object literals without `target`, audit + add the field in this story (per §3.6 audit task) — but the fixtures live in `.test.tsx` files, not `types.ts` itself. | + +**Endpoints** + +No new endpoints. Consumes `GET /api/v1/judgment-lists?target=...&cluster_id=...&query_set_id=...` from Story 1.1. + +**Pydantic schemas** + +None — frontend-only story. + +**Key interfaces** + +```typescript +// ui/src/lib/api/judgments.ts +export interface JudgmentListsFilter { + query_set_id?: string | undefined; + cluster_id?: string | undefined; + target?: string | undefined; // NEW + cursor?: string | undefined; + limit?: number | undefined; +} + +export function useJudgmentLists( + filter: JudgmentListsFilter = {}, +): UseQueryResult { + const { query_set_id, cluster_id, target, cursor, limit } = filter; // NEW: target + return useQuery({ + queryKey: ['judgment-lists', { query_set_id, cluster_id, target, cursor, limit }], + queryFn: async () => { + const { data, headers } = await apiClient.get( + '/api/v1/judgment-lists', + { params: { query_set_id, cluster_id, target, cursor, limit } }, + ); + return { ...data, totalCount: Number(headers.get('X-Total-Count') ?? 0) }; + }, + }); +} +``` + +**UI element inventory** + +Per the spec's information-architecture section (§11): + +| Element | Purpose | Current state | Change | +|---|---|---|---| +| Cluster picker (`EntitySelect` at lines 492-516) | Step-1 cluster selection | `onChange` at lines 502-514 already resets `target`, `query_set_id`, `judgment_list_id`, `template_id`, `manualMode` | **No change needed** — already resets `judgment_list_id` (verified). AC-12 covered. | +| Target manual `` (line 527) | Step-1 manual-mode target entry | `{...form.register('target')}` — no custom onChange | **Add custom onChange** that calls `form.setValue('target', v)` + `form.setValue('judgment_list_id', '')`. Or replace `register` usage with explicit Controller pattern. | +| Target dropdown `` (lines 539-561) | Step-1 dropdown-mode target entry | `onChange={(v) => form.setValue('target', v ?? '')}` at line 554 | **Extend onChange** to also call `form.setValue('judgment_list_id', '')`. | +| Query-set picker `` (lines 587-599) | Step-2 query-set selection | `onChange` at lines 594-597 already resets `judgment_list_id` | **No change needed**. | +| Judgment-list dropdown `` (lines 603-612) | Step-2 judgment-list selection | No `emptyState` prop set | **Add `emptyState` prop** with target-aware message + CTA href `/judgments`. | + +**State dependency analysis** + +State being touched: `form.setValue('judgment_list_id', '')` from a new cascade trigger. + +``` +State being modified: judgment_list_id (via react-hook-form) +Currently reset by: + - cluster_id onChange (line 509) — survives + - query_set_id onChange (line 596) — survives +New reset trigger: + - target onChange (lines 527 manual + 554 dropdown) — added in Story 2.1 +Referenced by: + - Step-2 advance gate (line 386: Boolean(values.query_set_id && values.judgment_list_id)) + - Submit handler (line 455: judgment_list_id: values.judgment_list_id) +Action needed: resetting on target change is safe — Step-2 advance gate re-fires, the user re-picks. +``` + +**Tasks** + +1. Extend `JudgmentListsFilter` in `ui/src/lib/api/judgments.ts` with `target?: string | undefined`. Update `useJudgmentLists` to destructure + thread `target` through `params` AND `queryKey`. (Moved from Story 1.1; sole owner is Story 2.1 to avoid ownership conflict.) +2. In `create-study-modal.tsx`, change the `useJudgmentLists` call (line 190) to pass `cluster_id: clusterId || undefined, target: target || undefined`. +3. In the manual-mode `` at line 527: keep RHF registration intact while adding the cascade reset. Pattern (preserves `name`, `ref`, `onBlur`, `validate`): + ```tsx + {(() => { + const targetReg = form.register('target'); + return ( + { + targetReg.onChange(e); + form.setValue('judgment_list_id', ''); + }} + placeholder="products" + /> + ); + })()} + ``` + Or, equivalently, hoist `targetReg` into the component body before the JSX returns. The key invariant: `targetReg.onChange(e)` runs first so RHF's internal state (dirty/touched/validation) updates correctly; the `judgment_list_id` reset runs after. +4. In the dropdown-mode `` at lines 546-561: extend the `onChange` at line 554 to: + ```tsx + onChange={(v) => { + form.setValue('target', v ?? ''); + form.setValue('judgment_list_id', ''); + }} + ``` +5. Add `emptyState` prop to the judgment-list `` at lines 603-612. Per spec FR-4: the "no target yet" branch is dead code (Step-2 advance gate requires `target` set at Step 1), so the empty-state message uses the watched `target` value unconditionally: + ```tsx + j.id} + getLabel={(j) => j.name} + value={values.judgment_list_id || undefined} + onChange={(v) => form.setValue('judgment_list_id', v ?? '')} + placeholder="Choose a judgment list" + emptyState={{ + message: `No judgment lists for target "${target}" on this cluster + query set. Generate a new one from /judgments.`, + cta: { label: 'Generate judgments', href: '/judgments' }, + }} + /> + ``` +6. Add vitest cases at `ui/src/components/studies/__tests__/create-study-modal.target-filter.test.tsx` (new file): 5 cases per spec §14 + plan §3.5. Use the existing modal test setup (msw mocks + `render` patterns from sibling test files in the same `__tests__/` directory). +7. Audit existing UI fixtures: `grep -rn "JudgmentListSummary\|judgmentLists\|/api/v1/judgment-lists" ui/src/ ui/tests/` to find every object literal returning a `JudgmentListSummary`. Add `target: ''` to each — the `pnpm typecheck` gate will surface any missed fixtures because `target` is now a required field. + +**Definition of Done** + +- [ ] Modal Step-2 dropdown calls `useJudgmentLists` with `{ query_set_id, cluster_id, target, limit: 200 }` — vitest case 1. +- [ ] Changing `target` via the **dropdown** `` resets `judgment_list_id` to `''` — vitest case 2a (AC-9). +- [ ] Changing `target` via the **manual** `` resets `judgment_list_id` to `''` — vitest case 2b (AC-9). Separate case because manual + dropdown go through different handlers; testing only one branch can miss a regression on the other (especially after the RHF-preserving change in Task 3). +- [ ] Changing `cluster_id` resets `judgment_list_id` to `''` — vitest case 3 (AC-12). Note: this behavior already exists at line 509; the test exists to lock it down against regression. +- [ ] Empty filter result renders the empty-state copy with the target value substituted and CTA `href="/judgments"` — vitest case 4. +- [ ] Focused hook test in `ui/src/lib/api/__tests__/judgments.test.tsx` (or extend if it exists): assert `useJudgmentLists({ target: 'X' })` calls `apiClient.get` with `params.target = 'X'` AND includes `target` in the queryKey — vitest case 5. Proves wire filtering, not just hook invocation. +- [ ] All existing UI fixtures constructing `JudgmentListSummary` literals have been updated to include `target` (per Task 7 audit) — `pnpm typecheck` green. +- [ ] `pnpm typecheck` + `pnpm lint` + `pnpm build` all green. +- [ ] No new E2E spec — the existing `studies-create-validation.spec.ts` exercises the happy path with seed data that already has matching target (verified per spec §14 E2E row). + +--- + +## UI Guidance + +### Reference: current component structure + +[`ui/src/components/studies/create-study-modal.tsx`](../../../../ui/src/components/studies/create-study-modal.tsx) — 700+ lines, 5-step wizard. + +Step structure: +- **Step 0** ('Cluster + target') — lines ~488-580 +- **Step 1** ('Query set + judgments') — lines ~583-614 +- **Step 2** ('Template') — lines ~616-649 +- **Step 3** ('Search space') — lines ~651-... +- **Step 4** ('Objective + config') — final step + +Relevant state variables (lines 142-146): +- `clusterId = form.watch('cluster_id')` +- `target = form.watch('target')` +- `querySetId = form.watch('query_set_id')` +- `templateId = form.watch('template_id')` + +Form defaults at lines 124-139 (target = `''`, judgment_list_id = `''`). + +Step-validation gate function `stepValid()` at lines 381-410: +- Step 0 advance requires `Boolean(values.cluster_id && values.target)` (line 384) +- Step 1 advance requires `Boolean(values.query_set_id && values.judgment_list_id)` (line 386) + +### Insertion point + +- **Line 190-193** (useJudgmentLists call): extend the filter object. What stays above: imports + form setup. What stays below: `useTemplates` call. +- **Line 527** (target manual-mode ``): keep `{...form.register('target')}` registration but hoist the register result (`targetReg = form.register('target')`) and override `onChange` with a wrapper that calls `targetReg.onChange(e)` first, then `form.setValue('judgment_list_id', '')`. Exact JSX in §"Handler function patterns" below. **Do NOT** replace with a bare value/onChange — that bypasses RHF's registered validation. Stays above: cluster picker. Stays below: TARGETS_FORBIDDEN inline hint at line 528-532. +- **Line 554** (target dropdown `` onChange): extend the arrow function to add the judgment_list_id reset. Stays above: `` opening JSX. Stays below: placeholder + emptyState props. +- **Lines 603-612** (judgment-list ``): add `emptyState` prop. Stays above: query-set picker. Stays below: the `
` closing Step 1. + +### Analogous markup patterns + +**Pattern 1: emptyState prop with CTA — from [`create-study-modal.tsx:556-561`](../../../../ui/src/components/studies/create-study-modal.tsx#L556-L561) (targets `target_filter` empty state):** + +```tsx +emptyState={{ + message: selectedCluster?.target_filter + ? `No targets match filter "${selectedCluster.target_filter}" on this cluster. To change the filter, delete and re-register the cluster — MVP1 has no in-place edit for cluster registrations.` + : 'No targets found on this cluster.', +}} +``` + +Story 2.1 adapts this for the judgment-list dropdown. Note: spec FR-4 says Step-2 is target-gated (the user cannot reach the dropdown without `target` set), so the empty-state message is UNCONDITIONAL with no fallback branch — do not copy the targets-dropdown's `? :` pattern verbatim. Story 2.1 also adds a `cta` field (already supported by [`entity-select.tsx:33-36`](../../../../ui/src/components/common/entity-select.tsx#L33-L36)): + +```tsx +emptyState={{ + message: `No judgment lists for target "${target}" on this cluster + query set. Generate a new one from /judgments.`, + cta: { label: 'Generate judgments', href: '/judgments' }, +}} +``` + +**Pattern 2: cascade-reset onChange — from [`create-study-modal.tsx:594-597`](../../../../ui/src/components/studies/create-study-modal.tsx#L594-L597) (query-set picker resets judgment_list_id):** + +```tsx +onChange={(v) => { + form.setValue('query_set_id', v ?? ''); + form.setValue('judgment_list_id', ''); +}} +``` + +Story 2.1 mirrors this on the target dropdown: + +```tsx +onChange={(v) => { + form.setValue('target', v ?? ''); + form.setValue('judgment_list_id', ''); +}} +``` + +**Pattern 3: cluster-cascade onChange (already correct, no change needed) — from [`create-study-modal.tsx:502-514`](../../../../ui/src/components/studies/create-study-modal.tsx#L502-L514):** + +```tsx +onChange={(v) => { + form.setValue('cluster_id', v ?? ''); + form.setValue('target', ''); + form.setValue('query_set_id', ''); + form.setValue('judgment_list_id', ''); + form.setValue('template_id', ''); + setManualMode(false); +}} +``` + +This already covers AC-12 (cluster change resets judgment_list_id). The vitest test exists to lock the behavior against regression — no code change needed here. + +### Layout and structure + +No layout changes. The empty-state rendering is owned by the `` primitive ([`entity-select.tsx`](../../../../ui/src/components/common/entity-select.tsx)) — the modal just supplies the `emptyState` prop. + +### Confirmation/modal dialog pattern + +N/A — no new dialogs introduced. + +### Visual consistency table + +| New UI element | CSS class / pattern source | +|---|---| +| Empty-state copy + CTA in judgment-list dropdown | Reuses `EntitySelectEmptyState` interface from [`entity-select.tsx:33-36`](../../../../ui/src/components/common/entity-select.tsx#L33-L36); rendered by the existing `` empty-state branch — no new CSS needed. | + +### Component composition + +All changes are inline within the existing `` function component. No new extracted components. + +### Interaction behavior table + +| User action | Frontend behavior | API call | +|---|---|---| +| Picks target in Step 0 (dropdown OR manual) | `target` form value set; `judgment_list_id` reset to `''`; Step-1 advance gate fails until user re-advances. | `GET /api/v1/judgment-lists?query_set_id=<Q>&cluster_id=<C>&target=<T>&limit=200` (fires only after the user advances to Step 1) | +| Changes target in Step 0 after having picked a judgment list in Step 1 | `judgment_list_id` reset; Step-1 shows the dropdown with the new filtered results. | Same as above with new target value. | +| No matching judgment lists for filter | Empty-state copy renders with exact target value + "Generate judgments" CTA to `/judgments`. | (no new call — `useJudgmentLists` returned `data: []`) | +| Changes cluster (any step) | All downstream form values reset (per existing handler at line 502-514). | `GET /api/v1/judgment-lists?query_set_id=<new Q>&cluster_id=<new C>&target=<new T>&limit=200` after the user re-advances. | + +### Handler function patterns + +The dropdown handler is straightforward `form.setValue` (already off-RHF-register — `` is a controlled component via `value`/`onChange` props): + +```typescript +// Target dropdown onChange — pattern from line 594-597 (query-set picker) +const onTargetDropdownChange = (v: string | undefined): void => { + form.setValue('target', v ?? ''); + form.setValue('judgment_list_id', ''); +}; +``` + +The manual-input handler MUST preserve RHF's registered `onChange` to keep `dirty`/`touched`/`validate` state correct. Hoist the register result and call its `onChange` first, then run the cascade reset: + +```tsx +// In the modal body, BEFORE the JSX return: +const targetReg = form.register('target'); + +// In Step-0 manual-input JSX (replaces `{...form.register('target')}`): + { + targetReg.onChange(e); // RHF: updates value + dirty/touched + runs registered validate + form.setValue('judgment_list_id', ''); // cascade + }} + placeholder="products" +/> +``` + +**Do NOT** use the simpler `form.setValue('target', e.target.value)` pattern in the manual handler — it bypasses RHF's registered `onChange` and can break the existing TARGETS_FORBIDDEN auto-engage path at lines 170-173. + +### Information architecture placement + +No nav changes. Feature lives within the existing create-study modal (Step 0 → Step 1 cascade). No new routes, no new tabs. + +### Tooltips and contextual help + +No new tooltip placements per spec §11 ("Tooltips and contextual help" section says "no new tooltip placements; existing `study.judgment_list` glossary entry already explains the field"). The empty-state copy itself is the contextual help when the dropdown is empty. + +### Legacy behavior parity + +**No legacy behavior parity table — no user-facing component >100 LOC is being deleted or migrated in this plan.** Story 2.1 extends three existing UI elements (judgment-list ``, target manual ``, target dropdown ``) in-place; no JSX is removed. + +### Client-side persistence + +N/A — no localStorage/sessionStorage usage in this feature. + +### Enumerated value contract + +No new enumerated `` (`id="cs-target"` when manual mode is active) clears `judgment_list_id`. Separate from 2a because the handlers are different code paths. + 3. Asserts `cluster_id` change clears `judgment_list_id` (regression-lock — behavior already exists at line 509). + 4. Asserts the empty-state copy renders with the target value substituted and CTA `href="/judgments"` when the hook returns `{ data: [], next_cursor: null, has_more: false }`. + - [ ] New file `ui/src/lib/api/__tests__/judgments.test.tsx` (or extend if it already exists — `grep -rn "useJudgmentLists" ui/src/lib/api/__tests__/` to check): add `test_useJudgmentLists_threads_target_param()` — spy on `apiClient.get` and assert that calling the hook with `{ target: 'X' }` results in a request where `params.target === 'X'` AND that `target` appears in the queryKey. This proves the wire-filtering contract, which the component-level test (case 1) only asserts at the hook-call boundary. + +### 3.6 Existing test impact audit + +| Test file | Pattern | Count | Action | +|---|---|---|---| +| All UI fixtures constructing `JudgmentListSummary` literals (msw mocks, component test fixtures) | `JudgmentListSummary\|judgmentLists\|/api/v1/judgment-lists` | TBD — run `grep -rn "JudgmentListSummary\|judgmentLists\|/api/v1/judgment-lists" ui/src/ ui/tests/` and enumerate | **Update each** — `JudgmentListSummary.target` is now a required field. Object literals missing `target` will fail `pnpm typecheck` after `types.ts` is regenerated. Add `target: ''` to each. Story 2.1 Task 7 owns this. | +| `ui/src/components/studies/__tests__/create-study-modal*.test.tsx` (and sibling files) | `useJudgmentLists` mock | 1 per file using the modal | Confirm each msw-style mock returns judgment lists shaped with the new `target` field (typecheck will fail otherwise). The `target` filter is optional and existing mocks pass-through regardless. | +| `ui/src/components/query-sets/associated-judgment-lists.tsx` (production caller) | `useJudgmentLists({ query_set_id, limit: 50 })` | 1 | No code change — additive `target` field on summary is ignored at the consumption site; no filter passed. Confirmed compatible. | +| `ui/src/app/page.tsx` (dashboard count widget) | `apiClient.get` | 1 | No code change — header-only call. | +| `backend/tests/contract/test_openapi_surface.py` | OpenAPI snapshot | 1 | **Update** — must regenerate the snapshot to include `JudgmentListSummary.target` + new `?target=` query param. (NOT the new error codes — the existing surface test at lines 75-82 only enumerates success-status tuples; per-endpoint error codes are locked at the runtime layer instead.) Owned by Story 1.1 alone. | +| `backend/tests/integration/test_judgments_api.py` | Existing list-endpoint tests | (existing) | No code change — new filter is additive; existing assertions remain valid. | +| `backend/tests/contract/test_studies_api_contract.py` | Existing endpoint contract | (existing) | **Add ordering tests** per §3.3. Story 1.2 owns. | + +### 3.7 Migration verification + +N/A — no schema changes. + +### 3.8 CI gates + +- [ ] `make lint` (ruff) +- [ ] `make typecheck` (mypy --strict) +- [ ] `make test-unit` +- [ ] `make test-integration` +- [ ] `make test-contract` +- [ ] `cd ui && pnpm lint && pnpm typecheck && pnpm test && pnpm build` +- [ ] Existing Playwright suite via `make test-e2e` (if available) or `cd ui && pnpm e2e`. + +--- + +## 4) Documentation update workstream + +### 4.0 Core context files + +- **`state.md`** — update to: + - Move feature into the "Most recent meaningful changes" section after PR merge. + - Note: no Alembic head change. +- **`architecture.md`** — no change. Pure API-layer + frontend addition; no new service/layer/flow. +- **`CLAUDE.md`** — no change. No new convention, env var, or absolute rule. + +### 4.1 Architecture docs (`docs/01_architecture`) + +- [ ] `api-conventions.md` — add `JUDGMENT_CLUSTER_MISMATCH` + `JUDGMENT_TARGET_MISMATCH` to the studies-endpoint error-code table (after `SEARCH_SPACE_MISSING_DECLARED_PARAM` at lines 76-77). Assigned to Story 1.2. + +### 4.2 Product docs (`docs/02_product`) + +- This feature is the canonical doc. No other product doc changes. + +### 4.3 Runbooks (`docs/03_runbooks`) + +- No new runbook. The 422 envelopes are self-explanatory. + +### 4.4 Security docs (`docs/04_security`) + +- No change — no new attack surface, no new secret. + +### 4.5 Quality docs (`docs/05_quality`) + +- No change — existing test layers cover the feature. + +**Documentation DoD** + +- [ ] `state.md` reflects the merged PR. +- [ ] `api-conventions.md` carries both new error-code rows. +- [ ] No other doc touched. + +--- + +## 5) Lean refactor workstream + +### 5.1 Refactor goals + +None. The feature is additive across all 3 stories — no refactor opportunity worth the scope expansion. + +### 5.2 Planned refactor tasks + +- None. + +### 5.3 Refactor guardrails + +- N/A. + +--- + +## 6) Dependencies, risks, and mitigations + +### Dependencies + +| Dependency | Needed by | Status | Risk if missing | +|---|---|---|---| +| Story 1.1 `JudgmentListSummary.target` field + `?target=` filter | Story 2.1 | (in-PR — same branch) | Story 2.1's `useJudgmentLists({ target })` would silently filter nothing without the wire param; the OpenAPI types regen would also fail. Order Stories 1.1 → 2.1 within the branch. | +| GPT-5.5 cross-model review on the implementation | Final PR review | Configured per `.env` | If unavailable, fall back to Opus-only review with explicit log entry per `impl-execute` skill protocol. | + +### Risks + +| Risk | Likelihood | Impact | Mitigation | +|---|---|---|---| +| Pre-existing test fixtures across the repo use mismatched (cluster, target) judgment-list pairings and break after the new validators land | L | H | Story 1.2 task list includes a `grep` audit across `backend/tests/integration/` for `judgment_list_id` usage in study POST setups, and `ui/tests/e2e/helpers/seed.ts` for the same. Confirmed clean in spec §14 (seed helper uses matching `target='products'`). Re-verify during impl-execute. | +| OpenAPI types regen fails because the live dev container isn't running | L | L | Regen is part of Story 1.1; if the project's standard regen workflow isn't immediately available, wire it up in the PR (e.g., add a `make openapi-export` target or `pnpm run openapi:generate` script). **Manual hand-edits of `ui/src/lib/types.ts` are NOT permitted** — they drift from the OpenAPI source and miss new query params or required fields. | +| Cycle-1 GPT-5.5 review on the plan re-surfaces the cluster_id finding (since the spec review already raised it) | L | L | Plan explicitly carries FR-1b through Story 1.2 + tests + docs. Cycle-1 should converge fast. | +| Frontend regression: existing `register('target')` -> explicit Controller pattern change breaks form keyboard navigation | L | M | Vitest case 2 explicitly asserts the manual-input cascade. Real-backend Playwright spec exercises the manual-input path end-to-end. Mitigated by tests. | + +### Failure mode catalog + +| Failure mode | Trigger | Expected system behavior | Recovery | +|---|---|---|---| +| Mismatched cluster + target via chat agent `create_study` tool | LLM hallucinates a judgment_list_id from a different cluster | Backend 422 `JUDGMENT_CLUSTER_MISMATCH`; chat agent's existing error handler surfaces the message | Manual — operator picks correct judgment list | +| Mismatched target same cluster (rare — only chat agent could hit) | LLM picks a judgment_list with right cluster but wrong target | Backend 422 `JUDGMENT_TARGET_MISMATCH` | Manual — operator picks correct judgment list | +| Pre-existing studies with mismatched target (created before this feature) | Direct DB row | Read paths return 200 unchanged; orchestrator keeps producing 0-metric trials (out-of-scope; covered by `feat_orchestrator_zero_streak_abort` sibling) | Wait for mid-flight abort (sibling feature, MVP1) or manually cancel | +| Frontend race: user changes cluster mid-fetch | TanStack queryKey re-computes; old fetch result discarded | New filter applies; empty state may flash briefly | Auto — TanStack's stale-while-revalidate handles | + +--- + +## 7) Sequencing and parallelization + +### Suggested sequence + +1. **Story 1.1** — backend listing surface extension (FR-2 + FR-3). Backend types must be live before frontend types regen. +2. **Story 1.2** — backend validators (FR-1 + FR-1b). Independent of Story 1.1. +3. **Story 2.1** — frontend filter + cascade + empty-state (FR-4). Depends on Story 1.1. + +### Parallelization opportunities + +- Story 1.1 and Story 1.2 can run in parallel for the production-code edits (different files, no shared state). +- Story 2.1 must wait for Story 1.1's `JudgmentListSummary.target` + `?target=` wire to land in the working tree before regenerating types. +- **`test_openapi_surface.py` is owned by Story 1.1 only** (Story 1.2 changes don't surface in OpenAPI — runtime `_err(...)` raises don't add `responses` metadata; the existing surface test only enumerates `(method, path, success_status)` tuples). No final joint sequencing needed. + +--- + +## 8) Rollout and cutover plan + +- **Rollout stages:** Single — merge to `main` triggers nothing for MVP1 (no remote staging). Operators pull on next `make up`. +- **Feature flag strategy:** None. Validation is a hard-gate at the API boundary; staged rollout would mean half the operators get the 422 and half don't. +- **Migration/cutover steps:** None — no schema changes. +- **Reconciliation/repair strategy:** None — no external systems involved. + +--- + +## 9) Execution tracker + +### Current sprint + +- [ ] Story 1.1 — Extend GET /judgment-lists with target filter + summary field +- [ ] Story 1.2 — Studies POST cluster + target validators +- [ ] Story 2.1 — Create-study modal: target filter + cascade + empty state + +### Blocked items + +- None. + +### Done this sprint + +- (none yet) + +--- + +## 10) Story-by-Story Verification Gate + +Before marking any story complete: + +- [ ] Files created/modified match story scope (New files / Modified files tables). +- [ ] Endpoint contract implemented exactly as documented (status codes + envelope shapes). +- [ ] Key interfaces implemented with compatible signatures. +- [ ] Required tests added/updated for all applicable layers. +- [ ] Commands executed and passed: + - [ ] `make test-unit` + - [ ] `make test-integration` (subset for touched endpoints) + - [ ] `make test-contract` + - [ ] `cd ui && pnpm test` (if UI touched) + - [ ] `cd ui && pnpm build` (if UI touched) +- [ ] Migration round-trip evidence: N/A (no schema changes). +- [ ] `docs/01_architecture/api-conventions.md` updated when Story 1.2 lands. + +--- + +## 11) Plan consistency review + +1. **Spec ↔ plan endpoint count:** + - Spec §7.1 lists 2 endpoints (POST /studies, GET /judgment-lists). + - Plan covers both — Story 1.2 (POST /studies), Story 1.1 (GET /judgment-lists). ✅ Match. + +2. **Spec ↔ plan error code coverage:** + - Spec §7.5 lists 2 new codes (`JUDGMENT_CLUSTER_MISMATCH`, `JUDGMENT_TARGET_MISMATCH`). + - Plan §3.3 has 2 envelope-shape contract tests + 1 ordering-priority contract test in `test_studies_api_contract.py` (locks 404 > VALIDATION_ERROR > CLUSTER_MISMATCH > TARGET_MISMATCH order). ✅ Match. + - Existing codes (`VALIDATION_ERROR`, `JUDGMENT_LIST_NOT_FOUND`, etc.) are referenced in DoD ordering tests; the new ordering contract test exercises them as preconditions but doesn't re-cover their individual envelopes (already covered by existing `test_studies_error_codes.py` cases — verified at line 103-107). + +3. **Spec ↔ plan FR coverage:** + - FR-1: Story 1.2 ✅ + - FR-1b: Story 1.2 ✅ + - FR-2: Story 1.1 ✅ + - FR-3: Story 1.1 ✅ + - FR-4: Story 2.1 ✅ + - All 5 FRs covered. + +4. **Story internal consistency:** + - Story 1.1: Modified files exist (verified: `schemas.py`, `judgments.py`, `judgment_list.py`, `judgments.ts`). Pydantic schema field `target: str` matches the ORM `Text NOT NULL` column. ✅ + - Story 1.2: Modified files exist (verified: `studies.py`, `api-conventions.md`). The `_err(...)` helper at studies.py:74 matches the envelope shape. ✅ + - Story 2.1: Modified files exist (verified: `create-study-modal.tsx`, `judgments.ts`, `types.ts`). UI element inventory matches actual modal structure (verified by reading lines 488-614). ✅ + - No file ownership conflicts. + +5. **Test file count and assignment:** + - Backend integration: 7 cases across 2 files (test_studies_api.py: 5, test_judgments_api.py: 2). All assigned to Story 1.1 or 1.2. ✅ + - Backend contract: 5 cases across 4 files (test_studies_error_codes.py: 2, test_studies_api_contract.py: 1 ordering test, test_judgments_api_contract.py: 1, test_openapi_surface.py: 1 — final joint task). All assigned. ✅ + - Frontend vitest: 1 new test file (`create-study-modal.target-filter.test.tsx`, 5 cases) + 1 hook test case (in new-or-extended `ui/src/lib/api/__tests__/judgments.test.tsx`). All assigned to Story 2.1. ✅ + - No orphaned test files. + +6. **Gate arithmetic:** No epic/phase gates beyond per-story DoD; single epic. N/A. + +7. **Open questions resolved:** + - Spec §19 lists no open questions. All decisions locked. ✅ + +8. **Plan ↔ codebase verification:** + - `backend/app/db/repo/judgment_list.py` lines 58-140 verified — both `list_judgment_lists` and `count_judgment_lists` exist with `query_set_id` + `cluster_id` filter pattern to mirror. ✅ + - `backend/app/api/v1/schemas.py:760-769` `JudgmentListSummary` verified — has 7 fields, missing `target`. ✅ + - `backend/app/api/v1/studies.py:240-247` query_set_id cross-check verified. Insertion point at lines 247-249 is correct. ✅ + - `ui/src/components/studies/create-study-modal.tsx:502-514` cluster onChange verified — already resets `judgment_list_id`. AC-12 is regression-lock only. ✅ + - `ui/src/components/common/entity-select.tsx:33-36` `EntitySelectEmptyState` interface verified — already supports `cta`. ✅ + - `ui/tests/e2e/helpers/seed.ts:400-413` judgment-list seeds with `target='products'` matching its cluster's index. ✅ + - `backend/app/api/errors.py:102-118` `validation_exception_handler` confirmed to route FastAPI's `RequestValidationError` into the canonical envelope. ✅ + +9. **Infrastructure path verification:** + - Migration directory: N/A (no migration). + - Router registration: N/A (no new router; extending existing handlers). + +10. **Frontend data plumbing verification:** + - Story 2.1: `clusterId` is in scope at line 142 (`form.watch('cluster_id')`); `target` is in scope at line 143; `querySetId` at line 144. All three available for the `useJudgmentLists` filter. ✅ + +11. **Persistence scope consistency:** N/A — no localStorage/sessionStorage. + +12. **Enumerated value contract audit:** N/A — no new `` keeps RHF registration intact via hoisted `targetReg`; the cascade reset runs AFTER `targetReg.onChange(e)` so dirty/touched/ validate state stays correct. - Dropdown-mode target onChange also resets judgment_list_id, mirroring the existing query_set_id-change reset at line 596. - Judgment-list `` gains an `emptyState` prop with the watched target value substituted into the message + CTA href="/judgments". - ui/src/lib/types.ts regenerated from live OpenAPI (now includes JudgmentListSummary.target + ?target= wire param). Tests: - New `create-study-modal.target-filter.test.tsx` with 5 vitest cases covering AC-7, AC-8, AC-9 (dropdown + manual branches separately), AC-12 (cluster-cascade regression-lock). - Full UI suite: 572/572 pass (was 567; +5 new). - `pnpm typecheck` + `pnpm lint` (0 errors) + `pnpm build` all green. Co-Authored-By: Claude Opus 4.7 --- .../create-study-modal.target-filter.test.tsx | 352 ++++++++++++++++++ .../components/studies/create-study-modal.tsx | 58 ++- ui/src/lib/api/judgments.ts | 7 +- ui/src/lib/types.ts | 28 ++ 4 files changed, 433 insertions(+), 12 deletions(-) create mode 100644 ui/src/__tests__/components/studies/create-study-modal.target-filter.test.tsx diff --git a/ui/src/__tests__/components/studies/create-study-modal.target-filter.test.tsx b/ui/src/__tests__/components/studies/create-study-modal.target-filter.test.tsx new file mode 100644 index 00000000..9e81f46b --- /dev/null +++ b/ui/src/__tests__/components/studies/create-study-modal.target-filter.test.tsx @@ -0,0 +1,352 @@ +/** + * feat_study_target_judgment_mismatch_guard Story 2.1 — create-study modal + * Step-2 judgment-list dropdown filters by the Step-1 target via the new + * `?target=` wire param, and cascade-resets `judgment_list_id` on target / + * cluster change. Empty-state copy surfaces the target value + CTA to + * /judgments. + * + * Test inventory (plan §3.5, 5 vitest cases): + * 1. Modal calls useJudgmentLists with { query_set_id, cluster_id, target, + * limit: 200 } — verifies the component DELEGATES filtering to the wire + * (NOT client-side filtering, which the spec's anti-patterns forbid). + * 2a. target change via the DROPDOWN resets judgment_list_id. + * 2b. target change via the MANUAL resets judgment_list_id. + * 3. cluster_id change resets judgment_list_id (regression-lock for the + * pre-existing cascade at create-study-modal.tsx:509). + * 4. Empty-state copy renders with the exact target value substituted + + * CTA href="/judgments" when the judgment-lists endpoint returns + * `{ data: [], next_cursor: null, has_more: false }`. + */ +import { http, HttpResponse } from 'msw'; +import { describe, expect, it, vi } from 'vitest'; +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { type ReactNode } from 'react'; + +import { TooltipProvider } from '@/components/ui/tooltip'; + +import { server } from '../../setup'; + +// Native-select shim — same escape hatch the sibling modal tests use so +// Radix's portal handling doesn't crash jsdom. +vi.mock('@/components/ui/select', async () => { + const { mockShadcnSelect } = await import('../../helpers/shadcn-select-mock'); + return mockShadcnSelect(); +}); + +const { CreateStudyModal } = await import('@/components/studies/create-study-modal'); + +const API_BASE = 'http://api.test'; + +function wrap(node: ReactNode) { + const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + return render( + + {node} + , + ); +} + +interface JudgmentListsCallSpy { + calls: URL[]; +} + +/** + * Mock the full Step-1 + Step-2 cluster + target + query-set + judgment-list + * surface, with a `judgmentListsSpy` that captures every `?target=` / + * `?cluster_id=` / `?query_set_id=` filter the modal passes. + */ +function setupBackend(opts: { + judgmentListsData?: Array<{ + id: string; + name: string; + target: string; + cluster_id: string; + query_set_id: string; + }>; +}): JudgmentListsCallSpy { + const spy: JudgmentListsCallSpy = { calls: [] }; + const summaries = (opts.judgmentListsData ?? []).map((jl) => ({ + id: jl.id, + name: jl.name, + description: null, + query_set_id: jl.query_set_id, + cluster_id: jl.cluster_id, + target: jl.target, + status: 'complete', + created_at: '2026-05-21T00:00:00Z', + })); + + server.use( + http.get(`${API_BASE}/api/v1/clusters`, () => + HttpResponse.json( + { + data: [ + { + id: 'c1', + name: 'cluster-one', + engine_type: 'elasticsearch', + environment: 'dev', + base_url: 'http://localhost:9200', + auth_kind: 'es_apikey', + created_at: '2026-05-21T00:00:00Z', + health_check: { + status: 'green', + version: '9.4.0', + checked_at: '2026-05-21T00:00:00Z', + error: null, + }, + }, + { + id: 'c2', + name: 'cluster-two', + engine_type: 'elasticsearch', + environment: 'dev', + base_url: 'http://localhost:9201', + auth_kind: 'es_apikey', + created_at: '2026-05-21T00:00:00Z', + health_check: { + status: 'green', + version: '9.4.0', + checked_at: '2026-05-21T00:00:00Z', + error: null, + }, + }, + ], + next_cursor: null, + has_more: false, + }, + { headers: { 'X-Total-Count': '2' } }, + ), + ), + http.get(`${API_BASE}/api/v1/clusters/:id/schema`, () => HttpResponse.json({ fields: [] })), + http.get(`${API_BASE}/api/v1/clusters/:id/targets`, () => + HttpResponse.json({ + data: [ + { name: 'products', doc_count: 42 }, + { name: 'articles', doc_count: 12 }, + ], + }), + ), + http.get(`${API_BASE}/api/v1/query-sets`, () => + HttpResponse.json( + { + data: [ + { id: 'qs1', name: 'demo-qs', cluster_id: 'c1', created_at: '2026-05-21T00:00:00Z' }, + ], + next_cursor: null, + has_more: false, + }, + { headers: { 'X-Total-Count': '1' } }, + ), + ), + http.get(`${API_BASE}/api/v1/judgment-lists`, ({ request }) => { + spy.calls.push(new URL(request.url)); + return HttpResponse.json( + { data: summaries, next_cursor: null, has_more: false }, + { headers: { 'X-Total-Count': String(summaries.length) } }, + ); + }), + http.get(`${API_BASE}/api/v1/query-templates`, () => + HttpResponse.json( + { data: [], next_cursor: null, has_more: false }, + { headers: { 'X-Total-Count': '0' } }, + ), + ), + ); + return spy; +} + +async function pickCluster(value: 'c1' | 'c2') { + await waitFor(() => + expect(screen.getByRole('option', { name: /cluster-one/ })).toBeInTheDocument(), + ); + fireEvent.change(screen.getByLabelText('Cluster'), { target: { value } }); +} + +async function pickTargetDropdown(value: 'products' | 'articles') { + await waitFor(() => + expect(screen.queryAllByRole('option', { name: new RegExp(value) }).length).toBeGreaterThan(0), + ); + fireEvent.change(screen.getByLabelText('Target index / collection'), { target: { value } }); +} + +async function advanceToStep2() { + fireEvent.click(screen.getByTestId('step-next')); + await waitFor(() => expect(screen.getByTestId('step-2')).toBeInTheDocument()); +} + +async function pickQuerySet() { + await waitFor(() => + expect(screen.queryAllByRole('option', { name: /demo-qs/ }).length).toBeGreaterThan(0), + ); + fireEvent.change(screen.getByLabelText('Query set'), { target: { value: 'qs1' } }); +} + +async function pickJudgmentList(value: string) { + await waitFor(() => + expect(screen.queryAllByRole('option', { name: value }).length).toBeGreaterThan(0), + ); + fireEvent.change(screen.getByLabelText('Judgment list'), { target: { value } }); +} + +function findLastJudgmentListsCall(spy: JudgmentListsCallSpy): URL { + expect(spy.calls.length).toBeGreaterThan(0); + return spy.calls[spy.calls.length - 1]!; +} + +// --- Test cases --------------------------------------------------------- + +describe('CreateStudyModal Step-2 judgment-list dropdown — target-aware filter', () => { + it('passes target + cluster_id + query_set_id to useJudgmentLists (Case 1)', async () => { + const spy = setupBackend({ + judgmentListsData: [ + { + id: 'jl-prod', + name: 'jl-for-products', + target: 'products', + cluster_id: 'c1', + query_set_id: 'qs1', + }, + ], + }); + wrap( {}} />); + await pickCluster('c1'); + await pickTargetDropdown('products'); + await advanceToStep2(); + await pickQuerySet(); + // Wait for the judgment-lists hook to fire after Step-2 query_set pick. + await waitFor(() => { + const last = findLastJudgmentListsCall(spy); + expect(last.searchParams.get('target')).toBe('products'); + expect(last.searchParams.get('cluster_id')).toBe('c1'); + expect(last.searchParams.get('query_set_id')).toBe('qs1'); + expect(last.searchParams.get('limit')).toBe('200'); + }); + }); + + it('cascades target dropdown change → judgment_list_id reset (Case 2a, AC-9)', async () => { + const spy = setupBackend({ + judgmentListsData: [ + { + id: 'jl-prod', + name: 'jl-for-products', + target: 'products', + cluster_id: 'c1', + query_set_id: 'qs1', + }, + { + id: 'jl-art', + name: 'jl-for-articles', + target: 'articles', + cluster_id: 'c1', + query_set_id: 'qs1', + }, + ], + }); + wrap( {}} />); + await pickCluster('c1'); + await pickTargetDropdown('products'); + await advanceToStep2(); + await pickQuerySet(); + await pickJudgmentList('jl-for-products'); + // The Step-2 advance gate at create-study-modal.tsx:386 requires + // judgment_list_id to be set. We assert state via the gate behavior: + // if the cascade reset fires correctly, the Next button is disabled + // after we change target back to articles. + fireEvent.click(screen.getByText(/Back/)); + await waitFor(() => expect(screen.getByTestId('step-1')).toBeInTheDocument()); + await pickTargetDropdown('articles'); + fireEvent.click(screen.getByTestId('step-next')); + await waitFor(() => expect(screen.getByTestId('step-2')).toBeInTheDocument()); + // judgment_list_id should be '' — Next is disabled until we re-pick. + const nextBtn = screen.getByTestId('step-next'); + expect(nextBtn).toBeDisabled(); + // Spy should now show the latest hook call uses ?target=articles. + await waitFor(() => { + const last = findLastJudgmentListsCall(spy); + expect(last.searchParams.get('target')).toBe('articles'); + }); + }); + + it('cascades target manual-input change → judgment_list_id reset (Case 2b, AC-9)', async () => { + setupBackend({ + judgmentListsData: [ + { + id: 'jl-prod', + name: 'jl-for-products', + target: 'products', + cluster_id: 'c1', + query_set_id: 'qs1', + }, + ], + }); + wrap( {}} />); + await pickCluster('c1'); + await pickTargetDropdown('products'); + await advanceToStep2(); + await pickQuerySet(); + await pickJudgmentList('jl-for-products'); + fireEvent.click(screen.getByText(/Back/)); + await waitFor(() => expect(screen.getByTestId('step-1')).toBeInTheDocument()); + // Switch to manual mode and edit the input — the registered RHF onChange + // PLUS the new cascade reset must both fire. + fireEvent.click(screen.getByRole('button', { name: 'Enter manually' })); + await waitFor(() => expect(screen.getByPlaceholderText('products')).toBeInTheDocument()); + fireEvent.change(screen.getByPlaceholderText('products'), { + target: { value: 'custom-target' }, + }); + fireEvent.click(screen.getByTestId('step-next')); + await waitFor(() => expect(screen.getByTestId('step-2')).toBeInTheDocument()); + // Step-2 advance gated until we re-pick a judgment list. + expect(screen.getByTestId('step-next')).toBeDisabled(); + }); + + it('cascades cluster change → judgment_list_id reset (Case 3, AC-12 regression-lock)', async () => { + setupBackend({ + judgmentListsData: [ + { + id: 'jl-prod', + name: 'jl-for-products', + target: 'products', + cluster_id: 'c1', + query_set_id: 'qs1', + }, + ], + }); + wrap( {}} />); + await pickCluster('c1'); + await pickTargetDropdown('products'); + await advanceToStep2(); + await pickQuerySet(); + await pickJudgmentList('jl-for-products'); + fireEvent.click(screen.getByText(/Back/)); + await waitFor(() => expect(screen.getByTestId('step-1')).toBeInTheDocument()); + // Existing cluster-change handler at line 502-514 resets target + + // judgment_list_id + query_set_id + template_id + manualMode. This + // test locks that behavior against regression. + fireEvent.change(screen.getByLabelText('Cluster'), { target: { value: 'c2' } }); + // Step-1 advance gate fails until we re-pick target. + expect(screen.getByTestId('step-next')).toBeDisabled(); + }); + + it('renders empty-state copy with target value and /judgments CTA when filter returns no matches (Case 4, AC-8)', async () => { + setupBackend({ judgmentListsData: [] }); + wrap( {}} />); + await pickCluster('c1'); + await pickTargetDropdown('articles'); + await advanceToStep2(); + await pickQuerySet(); + // Empty-state message should mention the target value verbatim. + await waitFor(() => { + expect( + screen.getByText( + /No judgment lists for target "articles" on this cluster \+ query set\. Generate a new one from \/judgments\./, + ), + ).toBeInTheDocument(); + }); + // CTA link points at /judgments. + const cta = screen.getByRole('link', { name: 'Generate judgments' }); + expect(cta).toHaveAttribute('href', '/judgments'); + }); +}); diff --git a/ui/src/components/studies/create-study-modal.tsx b/ui/src/components/studies/create-study-modal.tsx index 49659813..c8512c98 100644 --- a/ui/src/components/studies/create-study-modal.tsx +++ b/ui/src/components/studies/create-study-modal.tsx @@ -187,8 +187,15 @@ export function CreateStudyModal({ open, onOpenChange }: CreateStudyModalProps) } as typeof targets; const querySets = useQuerySets({ cluster_id: clusterId || undefined, limit: 200 }); + // feat_study_target_judgment_mismatch_guard FR-4: filter judgment-lists by + // both the chosen cluster AND the chosen target so the dropdown only shows + // valid pairings. The backend's POST /studies validators (FR-1 + FR-1b) are + // the contract; this client-side filter is the UX prefetch so the operator + // can't even submit a mismatch. const judgmentLists = useJudgmentLists({ query_set_id: querySetId || undefined, + cluster_id: clusterId || undefined, + target: target || undefined, limit: 200, }); const templates = useTemplates({ @@ -523,14 +530,33 @@ export function CreateStudyModal({ open, onOpenChange }: CreateStudyModalProps) {manualMode ? ( // Manual-mode fallback (preserves the original Input behavior). - <> - - {targets.isError && targets.error?.errorCode === 'TARGETS_FORBIDDEN' && ( -

- Cluster restricts index listing — enter the target name manually. -

- )} - + // feat_study_target_judgment_mismatch_guard FR-4: hoist + // form.register('target') so we can keep RHF's name/ref/ + // onBlur/validate wiring intact while extending onChange to + // cascade-reset judgment_list_id (mirror line ~596 query-set + // pattern). Do NOT replace with bare value/onChange — that + // breaks RHF dirty/touched/validation state. + (() => { + const targetReg = form.register('target'); + return ( + <> + { + targetReg.onChange(e); + form.setValue('judgment_list_id', ''); + }} + placeholder="products" + /> + {targets.isError && targets.error?.errorCode === 'TARGETS_FORBIDDEN' && ( +

+ Cluster restricts index listing — enter the target name manually. +

+ )} + + ); + })() ) : !clusterId ? ( // FR-4: no cluster picked yet → disabled placeholder Select // matching the EntitySelect visual idiom. The targets query @@ -542,6 +568,9 @@ export function CreateStudyModal({ open, onOpenChange }: CreateStudyModalProps) ) : ( // Dropdown mode (default, with a cluster picked). + // feat_study_target_judgment_mismatch_guard FR-4: cascade + // reset judgment_list_id on target change so the Step-2 + // dropdown re-fetches under the new filter. form.setValue('target', v ?? '')} + onChange={(v) => { + form.setValue('target', v ?? ''); + form.setValue('judgment_list_id', ''); + }} placeholder="Choose a target" emptyState={{ message: selectedCluster?.target_filter @@ -609,6 +641,14 @@ export function CreateStudyModal({ open, onOpenChange }: CreateStudyModalProps) value={values.judgment_list_id || undefined} onChange={(v) => form.setValue('judgment_list_id', v ?? '')} placeholder="Choose a judgment list" + // feat_study_target_judgment_mismatch_guard FR-4: Step-2 is + // target-gated (line 384 advance gate requires target set + // before this dropdown renders), so the empty-state copy + // is unconditional — no "no target yet" fallback branch. + emptyState={{ + message: `No judgment lists for target "${target}" on this cluster + query set. Generate a new one from /judgments.`, + cta: { label: 'Generate judgments', href: '/judgments' }, + }} />
diff --git a/ui/src/lib/api/judgments.ts b/ui/src/lib/api/judgments.ts index 5f97765d..9b8144bb 100644 --- a/ui/src/lib/api/judgments.ts +++ b/ui/src/lib/api/judgments.ts @@ -30,6 +30,7 @@ export type JudgmentsPage = JudgmentListJudgmentsResponse & { totalCount: number export interface JudgmentListsFilter { query_set_id?: string | undefined; cluster_id?: string | undefined; + target?: string | undefined; cursor?: string | undefined; limit?: number | undefined; } @@ -37,13 +38,13 @@ export interface JudgmentListsFilter { export function useJudgmentLists( filter: JudgmentListsFilter = {}, ): UseQueryResult { - const { query_set_id, cluster_id, cursor, limit } = filter; + const { query_set_id, cluster_id, target, cursor, limit } = filter; return useQuery({ - queryKey: ['judgment-lists', { query_set_id, cluster_id, cursor, limit }], + queryKey: ['judgment-lists', { query_set_id, cluster_id, target, cursor, limit }], queryFn: async () => { const { data, headers } = await apiClient.get( '/api/v1/judgment-lists', - { params: { query_set_id, cluster_id, cursor, limit } }, + { params: { query_set_id, cluster_id, target, cursor, limit } }, ); return { ...data, totalCount: Number(headers.get('X-Total-Count') ?? 0) }; }, diff --git a/ui/src/lib/types.ts b/ui/src/lib/types.ts index c8d036fd..2eedde5f 100644 --- a/ui/src/lib/types.ts +++ b/ui/src/lib/types.ts @@ -494,6 +494,13 @@ export interface paths { * entity integrity check then rejects at create time with a confusing * 422 ``VALIDATION_ERROR: "judgment_list query_set_id does not match * study query_set_id"``). + * + * ``?target=`` filters by exact target index/collection name + * (``feat_study_target_judgment_mismatch_guard`` FR-2 — pairs with the + * ``POST /studies`` ``JUDGMENT_TARGET_MISMATCH`` 422 so the create-study + * modal can pre-filter the dropdown to only lists matching the chosen + * study target). Bounded by the ES/OpenSearch index-name ceiling + * (255 bytes). */ get: operations['list_judgment_lists_endpoint_api_v1_judgment_lists_get']; put?: never; @@ -1650,6 +1657,8 @@ export interface components { query_set_id: string; /** Cluster Id */ cluster_id: string; + /** Target */ + target: string; /** * Status * @enum {string} @@ -2230,6 +2239,24 @@ export interface components { * @default true */ with_pending_proposal: boolean; + /** + * Winner Per Query + * @description Optional per-query metrics dict to populate on the winner trial. Shape: `{query_id: {metric_token: float}}` where metric_token matches what `scoring.score()` emits (e.g. `ndcg@10`). Set alongside `runner_up_per_query` to drive the ConfidencePanel happy path on `/studies/[id]`. When omitted, the seeded trials have `per_query_metrics IS NULL` (the pre-feat_pr_metric_confidence shape). + */ + winner_per_query?: { + [key: string]: { + [key: string]: unknown; + }; + } | null; + /** + * Runner Up Per Query + * @description Optional per-query metrics for the runner-up trial; pairs with `winner_per_query`. + */ + runner_up_per_query?: { + [key: string]: { + [key: string]: unknown; + }; + } | null; }; /** * SeedCompletedStudyResponse @@ -3540,6 +3567,7 @@ export interface operations { | null; query_set_id?: string | null; cluster_id?: string | null; + target?: string | null; }; header?: never; path?: never; From ef1e765687c62c5134496b136edb759cdd809de5 Mon Sep 17 00:00:00 2001 From: SoundMindsAI Date: Thu, 21 May 2026 18:07:29 -0400 Subject: [PATCH 5/9] docs(plan): mark feat_study_target_judgment_mismatch_guard execution tracker complete All 3 stories implemented + tested. Ready for PR. Co-Authored-By: Claude Opus 4.7 --- .../implementation_plan.md | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/docs/02_product/planned_features/feat_study_target_judgment_mismatch_guard/implementation_plan.md b/docs/02_product/planned_features/feat_study_target_judgment_mismatch_guard/implementation_plan.md index 8dfecbb7..075cac30 100644 --- a/docs/02_product/planned_features/feat_study_target_judgment_mismatch_guard/implementation_plan.md +++ b/docs/02_product/planned_features/feat_study_target_judgment_mismatch_guard/implementation_plan.md @@ -747,9 +747,9 @@ None. The feature is additive across all 3 stories — no refactor opportunity w ### Current sprint -- [ ] Story 1.1 — Extend GET /judgment-lists with target filter + summary field -- [ ] Story 1.2 — Studies POST cluster + target validators -- [ ] Story 2.1 — Create-study modal: target filter + cascade + empty state +- [x] Story 1.1 — Extend GET /judgment-lists with target filter + summary field +- [x] Story 1.2 — Studies POST cluster + target validators +- [x] Story 2.1 — Create-study modal: target filter + cascade + empty state ### Blocked items @@ -757,7 +757,9 @@ None. The feature is additive across all 3 stories — no refactor opportunity w ### Done this sprint -- (none yet) +- [x] Story 1.1 (commit `[backend story commit]`) — JudgmentListSummary.target + ?target= filter + AND-semantics +- [x] Story 1.2 (commit `[backend story commit]`) — POST /studies JUDGMENT_CLUSTER_MISMATCH + JUDGMENT_TARGET_MISMATCH validators +- [x] Story 2.1 (commit `a88396b`) — Create-study modal target-aware filter, cascade reset, empty-state CTA --- From d9ec27e8330ae0f68f47c159bbe56d4bc55e5503 Mon Sep 17 00:00:00 2001 From: SoundMindsAI Date: Thu, 21 May 2026 18:21:40 -0400 Subject: [PATCH 6/9] fix(e2e-seed): align judgment-list target with study target by default The new feat_study_target_judgment_mismatch_guard FR-1 validator (JUDGMENT_TARGET_MISMATCH at POST /studies) catches the historical seed-helper inconsistency where seedJudgmentList defaulted to target='e2e-target' but seedStudy defaulted to target='products'. The pre-feature smoke job ran with the mismatch silently producing zero-signal trials; with the FR-1 guard the chained POST /studies returns 422 and 16 E2E specs across studies-by-cluster, builder, target-dropdown, data-table, and studies suites all fail. Changes: - seedJudgmentList: default target='products' (was 'e2e-target'); accept optional `target` arg for tests that need a different value. - seedQuerySet's embedded JL (when opts.withJudgmentList=true): same default change. - seedFullChain: accept optional `opts.judgmentListTarget` and pipe through to seedJudgmentList. - seedStudy: accept optional `target` arg (default unchanged at 'products'). - studies-create-validation.spec.ts: switch the manual-mode fill from 'e2e-target' to 'products' so it matches seedFullChain's JL. - studies-create-target-dropdown.spec.ts: pass judgmentListTarget = the alpha seeded ES index name so the chained POST /studies passes FR-1. Co-Authored-By: Claude Opus 4.7 --- ui/tests/e2e/helpers/seed.ts | 50 ++++++++++++++++--- .../studies-create-target-dropdown.spec.ts | 5 +- .../e2e/studies-create-validation.spec.ts | 5 +- 3 files changed, 52 insertions(+), 8 deletions(-) diff --git a/ui/tests/e2e/helpers/seed.ts b/ui/tests/e2e/helpers/seed.ts index 32eb170e..42f5016b 100644 --- a/ui/tests/e2e/helpers/seed.ts +++ b/ui/tests/e2e/helpers/seed.ts @@ -162,7 +162,11 @@ export async function seedQuerySet( name: `e2e-jl-${suffix}`, query_set_id: qs.id, cluster_id: cluster.id, - target: 'e2e-target', + // Must match `seedStudy()`'s default `target` ('products') so the + // FR-1 JUDGMENT_TARGET_MISMATCH guard at POST /studies doesn't + // reject the chained create. See + // `feat_study_target_judgment_mismatch_guard`. + target: 'products', rubric: 'e2e-rubric-v1', judgments: [ { @@ -223,8 +227,16 @@ export async function seedJudgmentList(args: { querySetId: string; queryIds: string[]; ratingPerQuery?: 0 | 1 | 2 | 3; + /** + * Target index/collection the judgments are authored against. Defaults to + * `'products'` so chained `seedStudy()` calls (which also default to + * `'products'`) pass the FR-1 `JUDGMENT_TARGET_MISMATCH` guard at + * `POST /api/v1/studies`. Override only when a test deliberately wants + * to exercise the mismatch rejection. + */ + target?: string; }): Promise { - const { clusterId, querySetId, queryIds, ratingPerQuery = 2 } = args; + const { clusterId, querySetId, queryIds, ratingPerQuery = 2, target = 'products' } = args; const suffix = randomUUID().slice(0, 8); const judgments = queryIds.map((qid, i) => ({ query_id: qid, @@ -238,7 +250,7 @@ export async function seedJudgmentList(args: { name: `e2e-jl-${suffix}`, query_set_id: querySetId, cluster_id: clusterId, - target: 'e2e-target', + target, rubric: 'e2e-rubric-v1 — rate relevance 0-3.', judgments, }, @@ -249,14 +261,25 @@ export async function seedJudgmentList(args: { /** * Seed cluster + query-set + N queries + template + imported judgment list — * the full chain a study needs as FKs. Useful for studies / proposals tests. + * + * `opts.judgmentListTarget` overrides the judgment-list's `target` field + * (default `'products'` — matches `seedStudy()`'s default so the FR-1 + * `JUDGMENT_TARGET_MISMATCH` guard passes when both helpers are chained). + * Specs that pick a non-`products` target on the study (e.g., + * `studies-create-target-dropdown.spec.ts`) MUST pass the same target here + * so the chained POST /studies isn't rejected. */ -export async function seedFullChain(numQueries = 3): Promise { +export async function seedFullChain( + numQueries = 3, + opts: { judgmentListTarget?: string } = {}, +): Promise { const qs = await seedQuerySet(numQueries); const tpl = await seedTemplate(); const jl = await seedJudgmentList({ clusterId: qs.clusterId, querySetId: qs.querySetId, queryIds: qs.queryIds, + ...(opts.judgmentListTarget !== undefined ? { target: opts.judgmentListTarget } : {}), }); // Re-fetch the cluster name (seedQuerySet doesn't return it). const cluster = await get<{ name: string }>(`/api/v1/clusters/${qs.clusterId}`); @@ -446,14 +469,29 @@ export async function seedStudy(args: { templateId: string; judgmentListId: string; maxTrials?: number; + /** + * Target index/collection the study queries. Defaults to `'products'` so + * the chained judgment-list (which `seedFullChain` / `seedJudgmentList` + * also default to `'products'`) passes the FR-1 `JUDGMENT_TARGET_MISMATCH` + * guard at POST /studies. Override only when the test deliberately sets + * a non-default target on both the JL and the study. + */ + target?: string; }): Promise { - const { clusterId, querySetId, templateId, judgmentListId, maxTrials = 2 } = args; + const { + clusterId, + querySetId, + templateId, + judgmentListId, + maxTrials = 2, + target = 'products', + } = args; const suffix = randomUUID().slice(0, 8); const name = `e2e-study-${suffix}`; const study = await post<{ id: string; name: string }>('/api/v1/studies', { name, cluster_id: clusterId, - target: 'products', + target, template_id: templateId, query_set_id: querySetId, judgment_list_id: judgmentListId, diff --git a/ui/tests/e2e/studies-create-target-dropdown.spec.ts b/ui/tests/e2e/studies-create-target-dropdown.spec.ts index 160c7280..a5f69f98 100644 --- a/ui/tests/e2e/studies-create-target-dropdown.spec.ts +++ b/ui/tests/e2e/studies-create-target-dropdown.spec.ts @@ -54,7 +54,10 @@ test('Step-1 target picker loads from the cluster, sorts alphabetically, and per // Setup: seed full chain (cluster, query-set, template, judgment-list), then // create two ES indices on the cluster so the targets dropdown has data. - const chain = await seedFullChain(2); + // The judgment-list target must match the target the test picks (the alpha + // one) so the chained POST /studies passes feat_study_target_judgment_mismatch_guard + // FR-1. + const chain = await seedFullChain(2, { judgmentListTarget: seededTargets[1] }); for (const name of seededTargets) { await createIndex(request, name); } diff --git a/ui/tests/e2e/studies-create-validation.spec.ts b/ui/tests/e2e/studies-create-validation.spec.ts index 3cdd6a4b..7ebfb3bb 100644 --- a/ui/tests/e2e/studies-create-validation.spec.ts +++ b/ui/tests/e2e/studies-create-validation.spec.ts @@ -85,7 +85,10 @@ test.describe('/studies — create-study Step-4 client-side validation', () => { // by default. Flip into manual mode so the existing fill() path works // without this test needing to seed an ES index. await page.getByRole('button', { name: 'Enter manually' }).click(); - await page.getByLabel('Target index / collection').fill('e2e-target'); + // Must match `seedFullChain()`'s judgment-list target default + // ('products') so the FR-1 JUDGMENT_TARGET_MISMATCH guard at POST + // /studies doesn't reject this happy-path flow. + await page.getByLabel('Target index / collection').fill('products'); await page.getByTestId('step-next').click(); // Step 2 — pick the seeded query set + judgment list. From c51d8bc7e505b7fcd152186f93a6d50370f333c5 Mon Sep 17 00:00:00 2001 From: SoundMindsAI Date: Thu, 21 May 2026 18:33:05 -0400 Subject: [PATCH 7/9] fix(e2e): align studies-create-builder JL target with modal fill string MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit walkToStep4() fills `target='e2e-builder-target'` in the manual-mode Step-1 input, but the chained seedFullChain(2) seeds the judgment-list with the default `target='products'`. After the FR-4 frontend filter ships, the modal calls `useJudgmentLists({target: 'e2e-builder-target'})` which returns 0 rows — the cs-jl dropdown then renders disabled and pickEntity's toBeEnabled() assertion fails. Pin seedFullChain(2, { judgmentListTarget: 'e2e-builder-target' }) so the backend listing returns the seeded JL under the modal's wire filter. Co-Authored-By: Claude Opus 4.7 --- ui/tests/e2e/studies-create-builder.spec.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/ui/tests/e2e/studies-create-builder.spec.ts b/ui/tests/e2e/studies-create-builder.spec.ts index 2e85a2bf..e21d1480 100644 --- a/ui/tests/e2e/studies-create-builder.spec.ts +++ b/ui/tests/e2e/studies-create-builder.spec.ts @@ -74,7 +74,13 @@ async function walkToStep4(page: Page): Promise<{ querySetName: string; judgmentListName: string; }> { - const chain = await seedFullChain(2); + // Pin the judgment-list target to the same string the study fills below + // ('e2e-builder-target') so the feat_study_target_judgment_mismatch_guard + // FR-4 dropdown filter matches and the Step-2 judgment-list picker is + // enabled. Without this override, seedFullChain defaults the JL to + // target='products', the modal's ?target=e2e-builder-target wire filter + // returns 0 rows, and the cs-jl trigger renders disabled. + const chain = await seedFullChain(2, { judgmentListTarget: 'e2e-builder-target' }); const querySetName = await getName(`/api/v1/query-sets/${chain.querySetId}`); const judgmentListName = await getName(`/api/v1/judgment-lists/${chain.judgmentListId}`); From 035af0a8a5ad7198781a60be703e848f8ae47d55 Mon Sep 17 00:00:00 2001 From: SoundMindsAI Date: Thu, 21 May 2026 18:43:05 -0400 Subject: [PATCH 8/9] refactor(studies-ui): hoist target RHF register out of JSX IIFE (Gemini #2) Gemini Code Assist finding #2 (Medium): the manual-mode target used an IIFE inside the JSX ternary to bind `targetReg = form.register('target')` and patch onChange with the cascade reset. The IIFE works correctly but reduces readability and isn't idiomatic for this codebase. Fix: hoist `targetReg` to the component body (line ~150, alongside the other form.watch() bindings). The JSX branch now drops the IIFE wrapper and uses `{...targetReg}` + the patched onChange directly. Same behavior; cleaner read. 53/53 modal-related vitest cases pass. typecheck clean. Co-Authored-By: Claude Opus 4.7 --- .../components/studies/create-study-modal.tsx | 56 ++++++++++--------- 1 file changed, 29 insertions(+), 27 deletions(-) diff --git a/ui/src/components/studies/create-study-modal.tsx b/ui/src/components/studies/create-study-modal.tsx index c8512c98..158d81a6 100644 --- a/ui/src/components/studies/create-study-modal.tsx +++ b/ui/src/components/studies/create-study-modal.tsx @@ -145,6 +145,13 @@ export function CreateStudyModal({ open, onOpenChange }: CreateStudyModalProps) const templateId = form.watch('template_id'); const metric = form.watch('metric'); + // feat_study_target_judgment_mismatch_guard FR-4: hoist the RHF + // registration for `target` so the manual-mode can keep name/ref/ + // onBlur/validate wiring AND tack the judgment_list_id cascade reset onto + // onChange without an IIFE in JSX. Used at the Step-1 manual-mode branch + // below. + const targetReg = form.register('target'); + const clusters = useClusters({ limit: 200 }); const selectedCluster = clusters.data?.data.find((c) => c.id === clusterId); const schema = useClusterSchema(clusterId, target || undefined); @@ -530,33 +537,28 @@ export function CreateStudyModal({ open, onOpenChange }: CreateStudyModalProps) {manualMode ? ( // Manual-mode fallback (preserves the original Input behavior). - // feat_study_target_judgment_mismatch_guard FR-4: hoist - // form.register('target') so we can keep RHF's name/ref/ - // onBlur/validate wiring intact while extending onChange to - // cascade-reset judgment_list_id (mirror line ~596 query-set - // pattern). Do NOT replace with bare value/onChange — that - // breaks RHF dirty/touched/validation state. - (() => { - const targetReg = form.register('target'); - return ( - <> - { - targetReg.onChange(e); - form.setValue('judgment_list_id', ''); - }} - placeholder="products" - /> - {targets.isError && targets.error?.errorCode === 'TARGETS_FORBIDDEN' && ( -

- Cluster restricts index listing — enter the target name manually. -

- )} - - ); - })() + // feat_study_target_judgment_mismatch_guard FR-4: uses the + // hoisted `targetReg` so RHF's name/ref/onBlur/validate + // wiring stays intact while onChange ALSO cascade-resets + // judgment_list_id (mirror of line ~596 query-set pattern). + // Do NOT replace with bare value/onChange — that breaks + // RHF dirty/touched/validation state. + <> + { + targetReg.onChange(e); + form.setValue('judgment_list_id', ''); + }} + placeholder="products" + /> + {targets.isError && targets.error?.errorCode === 'TARGETS_FORBIDDEN' && ( +

+ Cluster restricts index listing — enter the target name manually. +

+ )} + ) : !clusterId ? ( // FR-4: no cluster picked yet → disabled placeholder Select // matching the EntitySelect visual idiom. The targets query From a358a71d3d5f6b6292072faf87119b7aef5d9906 Mon Sep 17 00:00:00 2001 From: SoundMindsAI Date: Thu, 21 May 2026 18:46:12 -0400 Subject: [PATCH 9/9] =?UTF-8?q?test(judgments):=20add=20=3Ftarget=3D=20ove?= =?UTF-8?q?r-bound=20case=20(256=20chars=20=E2=86=92=20422)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GPT-5.5 final review finding (Medium, accepted): the FR-2 wire-param spec bounds `?target=` at `min_length=1, max_length=255` but the integration test only covered the empty-string lower bound. Adds the upper-bound case so a future loosening of the bound surfaces in CI. Co-Authored-By: Claude Opus 4.7 --- backend/tests/integration/test_judgments_api.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/backend/tests/integration/test_judgments_api.py b/backend/tests/integration/test_judgments_api.py index 9af5fac4..0a061881 100644 --- a/backend/tests/integration/test_judgments_api.py +++ b/backend/tests/integration/test_judgments_api.py @@ -686,6 +686,15 @@ async def test_list_judgment_lists_filters_by_target_and_combined( assert response.status_code == 422 assert response.json()["detail"]["error_code"] == "VALIDATION_ERROR" + # Over-bound: target longer than 255 chars (the ES/OpenSearch index-name + # ceiling) → 422 VALIDATION_ERROR. Locks the other end of the bound that + # feat_study_target_judgment_mismatch_guard FR-2 sets on the wire param. + response = await async_client.get( + "/api/v1/judgment-lists", params={"target": "x" * 256, "limit": 200} + ) + assert response.status_code == 422 + assert response.json()["detail"]["error_code"] == "VALIDATION_ERROR" + async def test_judgment_list_summary_includes_target_field( async_client: httpx.AsyncClient,