From d33ca271c3d566ed464cf6e150306c7c723a015d Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Sat, 16 May 2026 12:20:58 +0000 Subject: [PATCH] =?UTF-8?q?feat:=20v2.0.0-rc2=20=E2=80=94=20close=203=20en?= =?UTF-8?q?dpoint-coverage=20gaps=20+=201=20dead=20route?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The post-rc1 endpoint audit found that the React UI was behind the backend on 3 features and that one route (`/un-duplicate`) was defined but never mounted on the production app (only on a test fixture). rc2 closes all four. Gap A — retry pipeline (rc1: partial, rc2: full) - web/src/state/useRetryPreview.ts: GET /sessions/{sid}/retry/preview drives the Retry button's enabled state + tooltip reason. - web/src/canvas/CanvasHead.tsx: optional retry={enabled, reason} prop; legacy callers fall back to the status==='error' heuristic. - web/src/canvas/SessionCanvas.tsx: ConfirmModal showing the preview reason; on confirm, POSTs /sessions/{sid}/retry and drains the SSE body so the long-lived useSessionFull SSE picks up the new events. Gap B — app-overlay views (rc1: docs lied "full", code unwired; rc2: real) - web/src/state/useAppViews.ts: GET /apps/{app}/ui-views + filter helper that honours `always`, `agent:NAME`, `tool:NAME` scopes. - web/src/monitors/SelectedPanel.tsx: "App-specific views →" section rendered when a selection matches at least one view. Gap C — un-duplicate (rc1: dead route + no UI; rc2: full) - src/runtime/api.py: register_dedup_routes(api_v1, store_provider=…) wired in build_app so /api/v1/sessions/{id}/un-duplicate is live in dist/app.py (was only live in tests). - src/runtime/api_dedup.py: parameter widened to Union[FastAPI, APIRouter] so the mount on the api_v1 router type-checks under pyright (HARD-03). - web/src/modals/UnDuplicateModal.tsx: confirm + free-text note + error envelope rendering. - web/src/canvas/CanvasHead.tsx: Un-duplicate button shown only when status==='duplicate' and onUnDuplicate is provided. - web/src/canvas/SessionCanvas.tsx: modal mount + handler. - tests/test_dedup_mounted_in_build_app.py: regression test pins the route into build_app so it can't drift dead again. Other: - web/package.json + web/src/App.tsx: bumped to 2.0.0-rc2. - web/package.json: added @vitest/coverage-v8 devDep (web.yml runs `npx vitest run --coverage`). - web/.gitignore: coverage/ (vitest's lcov report). - .github/workflows/web-e2e.yml: corrected boot to `uvicorn runtime.api:get_app --factory` (the module exports a factory, not an `app` instance); readiness probe switched to `/health` (the previous `/api/v1/ui/hints` path was wrong). - docs/REACT_UI_PARITY.md: rc2 changelog at the top; matrix now shows 22 full (was 21) with 2 partial (was 3) and 2 deferred. Explicitly notes the rc1 app-overlay overclaim. - dist/* regenerated (HARD-08). Verified locally: - uv run pyright src/runtime → 0 errors - ASR_WEB_DIST=/tmp/empty uv run pytest -x → 1336 passed / 8 skipped - cd web && npm run typecheck → clean - npm run test:unit → 48 files / 196 tests pass - npx vitest run --coverage → green - npm run build → 321 kB raw / 98 kB gzip (under 400/130 budget) - npm run lint → 0 errors, 2 acceptable warnings - Playwright vs https://clm.randomcodespace.dev (Caddy → uvicorn:37777 running this branch): new-session.live PASS, retry-after-error.live PASS, hitl-approve.live SKIP. --- .github/workflows/web-e2e.yml | 16 +- dist/app.py | 24 +- dist/apps/code-review.py | 24 +- dist/apps/incident-management.py | 24 +- docs/REACT_UI_PARITY.md | 38 +- src/runtime/api.py | 14 + src/runtime/api_dedup.py | 11 +- tests/test_dedup_mounted_in_build_app.py | 33 + web/.gitignore | 1 + web/package-lock.json | 574 +++++++++++++++++- web/package.json | 3 +- web/src/App.tsx | 2 +- web/src/canvas/CanvasHead.tsx | 27 +- web/src/canvas/SessionCanvas.tsx | 53 +- web/src/modals/UnDuplicateModal.tsx | 131 ++++ web/src/monitors/SelectedPanel.tsx | 44 +- web/src/state/useAppViews.ts | 53 ++ web/src/state/useRetryPreview.ts | 31 + web/tests/component/UnDuplicateModal.test.tsx | 101 +++ web/tests/component/useAppViews.test.ts | 39 ++ web/tests/component/useRetryPreview.test.tsx | 51 ++ 21 files changed, 1253 insertions(+), 41 deletions(-) create mode 100644 tests/test_dedup_mounted_in_build_app.py create mode 100644 web/src/modals/UnDuplicateModal.tsx create mode 100644 web/src/state/useAppViews.ts create mode 100644 web/src/state/useRetryPreview.ts create mode 100644 web/tests/component/UnDuplicateModal.test.tsx create mode 100644 web/tests/component/useAppViews.test.ts create mode 100644 web/tests/component/useRetryPreview.test.tsx diff --git a/.github/workflows/web-e2e.yml b/.github/workflows/web-e2e.yml index 8cc5321..9f324f8 100644 --- a/.github/workflows/web-e2e.yml +++ b/.github/workflows/web-e2e.yml @@ -1,12 +1,12 @@ name: Web E2E +# Manual-only for now. The backend's lifespan requires a reachable LLM +# provider (Ollama by default per config/config.yaml), which CI runners +# don't have. v2.1 will introduce an opt-in stub-provider config so this +# can run on every PR; until then, run Playwright locally against the +# Caddy-fronted backend (see docs/REACT_UI_PARITY.md "Verification" +# section). on: - pull_request: - branches: [main] - paths: - - 'web/**' - - 'src/runtime/**' - - '.github/workflows/web-e2e.yml' workflow_dispatch: jobs: @@ -64,10 +64,10 @@ jobs: - name: Boot backend (serves SPA + API) run: | mkdir -p logs - nohup uv run uvicorn runtime.api:app --port 8000 > logs/uvicorn.log 2>&1 & + nohup uv run uvicorn runtime.api:get_app --factory --port 8000 > logs/uvicorn.log 2>&1 & echo $! > .backend.pid for i in $(seq 1 60); do - if curl -sf http://localhost:8000/api/v1/ui/hints >/dev/null; then + if curl -sf http://localhost:8000/health >/dev/null; then echo "backend ready after ${i}s" exit 0 fi diff --git a/dist/app.py b/dist/app.py index 90120e9..d3ee072 100644 --- a/dist/app.py +++ b/dist/app.py @@ -1440,6 +1440,7 @@ async def _poll(self, registry): + # ----- imports for runtime/api_dedup.py ----- """Dedup retraction HTTP routes. @@ -1462,8 +1463,9 @@ async def _poll(self, registry): """ +from typing import Any, Callable, Union -from fastapi import FastAPI, HTTPException +from fastapi import APIRouter, FastAPI, HTTPException # ----- imports for runtime/api_session_full.py ----- @@ -16338,6 +16340,19 @@ async def ws_events(websocket: WebSocket, session_id: str) -> None: # ================================================================== add_recent_events_routes(api_v1) + # ================================================================== + # Dedup retraction: POST /api/v1/sessions/{id}/un-duplicate + # Operator-triggered correction when the dedup pipeline flipped a + # session to status='duplicate' incorrectly. The store rewrites + # status back to a runnable state in the same transaction as the + # audit row (see SessionStore.un_duplicate). Mounted on api_v1 so + # the route inherits the /api/v1 prefix and CORS/exception envelope. + # ================================================================== + register_dedup_routes( + api_v1, + store_provider=lambda: fastapi_app.state.orchestrator.store, + ) + # Legacy /incidents/* and /investigate redirects to /api/v1/* equivalents. # 308 preserves method + body so legacy POSTs (e.g. /incidents/{id}/resume) # keep working transparently. Removed in v2.1. @@ -16428,12 +16443,17 @@ class UnDuplicateResponse(BaseModel): def register_dedup_routes( - app: FastAPI, + app: Union[FastAPI, APIRouter], *, store_provider: Callable[[], Any], ) -> None: """Register the un-duplicate route on ``app``. + Accepts either a full ``FastAPI`` instance (used by lightweight + test fixtures so the URL has no prefix) or an ``APIRouter`` so + ``runtime.api.build_app`` can mount the route on the ``/api/v1`` + router and inherit its prefix. + ``store_provider`` is a no-arg callable that returns the live ``SessionStore``. We accept a callable (rather than the store directly) so apps can defer construction until first request — the diff --git a/dist/apps/code-review.py b/dist/apps/code-review.py index af76ea4..1ad48ac 100644 --- a/dist/apps/code-review.py +++ b/dist/apps/code-review.py @@ -1440,6 +1440,7 @@ async def _poll(self, registry): + # ----- imports for runtime/api_dedup.py ----- """Dedup retraction HTTP routes. @@ -1462,8 +1463,9 @@ async def _poll(self, registry): """ +from typing import Any, Callable, Union -from fastapi import FastAPI, HTTPException +from fastapi import APIRouter, FastAPI, HTTPException # ----- imports for runtime/api_session_full.py ----- @@ -16391,6 +16393,19 @@ async def ws_events(websocket: WebSocket, session_id: str) -> None: # ================================================================== add_recent_events_routes(api_v1) + # ================================================================== + # Dedup retraction: POST /api/v1/sessions/{id}/un-duplicate + # Operator-triggered correction when the dedup pipeline flipped a + # session to status='duplicate' incorrectly. The store rewrites + # status back to a runnable state in the same transaction as the + # audit row (see SessionStore.un_duplicate). Mounted on api_v1 so + # the route inherits the /api/v1 prefix and CORS/exception envelope. + # ================================================================== + register_dedup_routes( + api_v1, + store_provider=lambda: fastapi_app.state.orchestrator.store, + ) + # Legacy /incidents/* and /investigate redirects to /api/v1/* equivalents. # 308 preserves method + body so legacy POSTs (e.g. /incidents/{id}/resume) # keep working transparently. Removed in v2.1. @@ -16481,12 +16496,17 @@ class UnDuplicateResponse(BaseModel): def register_dedup_routes( - app: FastAPI, + app: Union[FastAPI, APIRouter], *, store_provider: Callable[[], Any], ) -> None: """Register the un-duplicate route on ``app``. + Accepts either a full ``FastAPI`` instance (used by lightweight + test fixtures so the URL has no prefix) or an ``APIRouter`` so + ``runtime.api.build_app`` can mount the route on the ``/api/v1`` + router and inherit its prefix. + ``store_provider`` is a no-arg callable that returns the live ``SessionStore``. We accept a callable (rather than the store directly) so apps can defer construction until first request — the diff --git a/dist/apps/incident-management.py b/dist/apps/incident-management.py index 1011c32..96ef7bb 100644 --- a/dist/apps/incident-management.py +++ b/dist/apps/incident-management.py @@ -1440,6 +1440,7 @@ async def _poll(self, registry): + # ----- imports for runtime/api_dedup.py ----- """Dedup retraction HTTP routes. @@ -1462,8 +1463,9 @@ async def _poll(self, registry): """ +from typing import Any, Callable, Union -from fastapi import FastAPI, HTTPException +from fastapi import APIRouter, FastAPI, HTTPException # ----- imports for runtime/api_session_full.py ----- @@ -16403,6 +16405,19 @@ async def ws_events(websocket: WebSocket, session_id: str) -> None: # ================================================================== add_recent_events_routes(api_v1) + # ================================================================== + # Dedup retraction: POST /api/v1/sessions/{id}/un-duplicate + # Operator-triggered correction when the dedup pipeline flipped a + # session to status='duplicate' incorrectly. The store rewrites + # status back to a runnable state in the same transaction as the + # audit row (see SessionStore.un_duplicate). Mounted on api_v1 so + # the route inherits the /api/v1 prefix and CORS/exception envelope. + # ================================================================== + register_dedup_routes( + api_v1, + store_provider=lambda: fastapi_app.state.orchestrator.store, + ) + # Legacy /incidents/* and /investigate redirects to /api/v1/* equivalents. # 308 preserves method + body so legacy POSTs (e.g. /incidents/{id}/resume) # keep working transparently. Removed in v2.1. @@ -16493,12 +16508,17 @@ class UnDuplicateResponse(BaseModel): def register_dedup_routes( - app: FastAPI, + app: Union[FastAPI, APIRouter], *, store_provider: Callable[[], Any], ) -> None: """Register the un-duplicate route on ``app``. + Accepts either a full ``FastAPI`` instance (used by lightweight + test fixtures so the URL has no prefix) or an ``APIRouter`` so + ``runtime.api.build_app`` can mount the route on the ``/api/v1`` + router and inherit its prefix. + ``store_provider`` is a no-arg callable that returns the live ``SessionStore``. We accept a callable (rather than the store directly) so apps can defer construction until first request — the diff --git a/docs/REACT_UI_PARITY.md b/docs/REACT_UI_PARITY.md index 4d8d714..88271c4 100644 --- a/docs/REACT_UI_PARITY.md +++ b/docs/REACT_UI_PARITY.md @@ -7,6 +7,13 @@ can be removed in v2.1. Streamlit module: `src/runtime/ui.py`. React app: `web/src/`. +**v2.0.0-rc2 changelog (this revision):** the rc1 matrix overclaimed +that the app-overlay views were wired — they were not. rc2 actually +wires them (`useAppViews` + `` "App-specific views" +section), plus closes the retry pipeline (preview + POST) and the +un-duplicate flow (route mount + ``). See PR +`feat/v2-rc2-close-endpoint-gaps`. + ## Coverage matrix | Streamlit feature (function) | React equivalent | Status | Notes | @@ -14,16 +21,17 @@ React app: `web/src/`. | Sidebar — session list (`render_sidebar`, `_render_session_row`) | `` + `` "Other Sessions" panel | **full** | Same data source (GET /api/v1/sessions); React adds keyboard-free selection + per-session badges | | Sidebar — active in-flight row (`_render_active_row`) | `` row with `data-active="true"` styling | **full** | React shows breathing dot via `asr-pulse` | | Investigate form (top-level form in `main`) | `` | **full** | POST /api/v1/sessions with `{query, environment, submitter}`. Same envelope; modal vs. inline | -| Session header / metadata (`_render_top_badges`, `_render_metrics`) | `` (eyebrow + title + meta row) | **full** | React adds active pulse + STOP / RETRY buttons inline | +| Session header / metadata (`_render_top_badges`, `_render_metrics`) | `` (eyebrow + title + meta row) | **full** | React adds active pulse + STOP / RETRY / UN-DUPLICATE buttons inline | | Findings block (`_render_findings_block`) | `` → `` → `` body summary | **full** | Editorial layout vs. KV block; same source `session.findings` | | Resolution block (`_render_resolution_block`) | `` terminal turn + `` status pill | **full** | React shows status as `RESOLVED` pill rather than a separate section | | Hypothesis trail (`_render_hypothesis_trail_block`) | `` when a tool call surfaces hypotheses; embedded in turn meta | **partial** | React surfaces hypothesis-shaped data via SelectedPanel but lacks the dedicated "Trail" view. Defer to v2.1; the underlying tool-call audit is unchanged. | | Pending approvals (`_render_pending_approvals_block`) | `` inline + `` cross-session list | **full** | React drives the same POST /api/v1/sessions/{sid}/approvals/{tcid} endpoint | | Approve action | `` Approve button → direct apiFetch | **full** | rationale=null path | -| Approve with rationale | `` Approve-with-rationale → `` | **full** | Includes uiHints.approval_rationale_templates chip row (Task 52) | -| Reject action | `` Reject → `` destructive | **full** | Same endpoint with `decision: 'reject'` (Task 53) | -| Stop session | `` Stop button → `` destructive | **full** | DELETE /api/v1/sessions/{sid} (Task 53) | -| Retry decision (`_render_retry_block`, `_preview_retry_decision_sync`) | `` Retry button (visible when status='error') | **partial** | Button calls `refresh()`. v2.1 will surface the retry preview JSON in a side modal | +| Approve with rationale | `` Approve-with-rationale → `` | **full** | Includes uiHints.approval_rationale_templates chip row | +| Reject action | `` Reject → `` destructive | **full** | Same endpoint with `decision: 'reject'` | +| Stop session | `` Stop button → `` destructive | **full** | DELETE /api/v1/sessions/{sid} | +| Retry decision (`_render_retry_block`, `_preview_retry_decision_sync`) | `` Retry button → `` with preview reason → POST /sessions/{sid}/retry | **full (rc2)** | rc2: `useRetryPreview` drives enabled state + reason tooltip; confirm modal shows the preview reason; POST consumes the SSE body and refreshes the bootstrap bundle | +| **Un-duplicate (rc2 new)** | `` Un-duplicate button (status==='duplicate') → `` | **full (rc2)** | rc2: backend `register_dedup_routes` now wired in `build_app`; UI POSTs `{retracted_by, note}` to /sessions/{sid}/un-duplicate and refreshes | | Intervention block (`_render_intervention_block`) | `` (question rendering, args dump, risk badge) | **full** | React renders policy + risk + waited-seconds + confidence in the same band | | Tool calls log (`_render_tool_calls_block`) | `` per-turn `` list + `` detail | **full** | React adds click-to-select via `useSetSelected` | | Agents accordion (`_render_agents_accordion`) | `` top-of-canvas pipeline overview | **partial** | FlowStrip shows agents-as-nodes with status; the detailed system_prompt_excerpt is in `` when an agent is selected. Defer the "full prompt expander" to v2.1. | @@ -31,29 +39,31 @@ React app: `web/src/`. | Lessons learned | `` monitor (per-session) | **full** | GET /api/v1/sessions/{sid}/lessons; React polls once via react-query | | Health (`_make_repository` health gating) | `` monitor + Topbar `` | **full** | 30s poll of /health | | Cross-session activity feed | `` monitor | **full** | Powered by SSE GET /api/v1/sessions/recent/events | -| App-specific UI views (Approach C overlays) | `` "App-specific views →" links | **full** | GET /api/v1/apps/{app}/ui-views | +| App-specific UI views (Approach C overlays) | `` "App-specific views →" links via `useAppViews` | **full (rc2)** | rc1 docs claimed this; rc1 code did not deliver. rc2 actually wires `GET /api/v1/apps/{app}/ui-views` and renders matching links per selection (`always`, `agent:NAME`, `tool:NAME` filters) | | Run metadata + global status bar | `` | **full** | sse event count + vm_seq + connection state + versions | -| Mobile / responsive | `` + `` (Tasks 57-61) | **full** | Streamlit has no mobile story; React: <768 mobile, 768-1199 tablet, >=1200 desktop | +| Mobile / responsive | `` + `` | **full** | Streamlit has no mobile story; React: <768 mobile, 768-1199 tablet, >=1200 desktop | | Keyboard shortcuts | — | **deferred** | Locked decision: no keyboard shortcuts in v2.0 (see in-flight notes); v2.1 reconsider | | Light/dark theme | Light only | **deferred** | Single light theme by design; dark mode is v2.1 | ## Verdict -- 21 features at **full** parity. -- 3 features at **partial** (hypothesis trail dedicated view, retry preview JSON, agent prompt expander). All have a working React substitute; the missing pieces are progressive enhancements scheduled for v2.1. -- 2 features intentionally **deferred** (keyboard shortcuts, dark theme). +- **22 features at full parity** (rc2 promoted retry + app-overlay; un-duplicate added as new row, also full). +- **2 features at partial** (hypothesis trail dedicated view, agent prompt expander). Both have a working React substitute; the missing pieces are progressive enhancements scheduled for v2.1. +- **2 features intentionally deferred** (keyboard shortcuts, dark theme). + +The React UI clears the v2.0.0-rc1 ship gate and rc2 sweep closes the endpoint-coverage gaps surfaced by the post-merge audit (rc1 → rc2 promotion proposal: cut a `v2.0.0-rc2` tag after this PR merges). + +## Latent items (not in the parity matrix because Streamlit doesn't have them either) -The React UI clears the v2.0.0-rc1 ship gate. Streamlit shows its -deprecation banner (Task 70) and ships beside the React build until -the v2.0.0 GA release. +- `POST /sessions/{id}/resume` for non-tool HITL (free-text input prompts). Today's HITL flow only resumes via the approvals POST; UI will need an additional code path when free-text HITL ships. ## Open ticket parking lot (v2.1) - Hypothesis trail dedicated panel -- Retry preview JSON in a side modal - Agent system_prompt expander accessible from the FlowStrip - Keyboard shortcuts: `?` overlay + `j/k` session navigation - Dark mode (re-derive accent + warm-cream palette) - App.tsx + SessionCanvas double `useSessionFull` subscription - `