From 22222611579cc441b84d18ff02cebea9bc047bc9 Mon Sep 17 00:00:00 2001 From: busya Date: Tue, 9 Jun 2026 11:44:20 +0300 Subject: [PATCH] tasks read --- backend/src/agent/__init__.py | 0 backend/tests/test_agent/__init__.py | 0 .../checklists/full-sweep.md | 127 ++++++++++++++++++ .../contracts/modules.md | 41 +++--- .../contracts/ux/api-ux.md | 19 +-- .../contracts/ux/decisions.md | 4 +- specs/033-gradio-agent-chat/research.md | 8 +- specs/033-gradio-agent-chat/spec.md | 50 ++++++- specs/033-gradio-agent-chat/tasks.md | 36 +++-- 9 files changed, 233 insertions(+), 52 deletions(-) create mode 100644 backend/src/agent/__init__.py create mode 100644 backend/tests/test_agent/__init__.py create mode 100644 specs/033-gradio-agent-chat/checklists/full-sweep.md diff --git a/backend/src/agent/__init__.py b/backend/src/agent/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/backend/tests/test_agent/__init__.py b/backend/tests/test_agent/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/specs/033-gradio-agent-chat/checklists/full-sweep.md b/specs/033-gradio-agent-chat/checklists/full-sweep.md new file mode 100644 index 00000000..05cd4535 --- /dev/null +++ b/specs/033-gradio-agent-chat/checklists/full-sweep.md @@ -0,0 +1,127 @@ +# Requirements Quality Checklist: Gradio Agent Chat — Full Sweep + +**Purpose**: Validate ALL requirements artifacts (spec, plan, tasks, contracts, research, data-model) for completeness, clarity, consistency, traceability, and decision-memory readiness after the final LangGraph/structured-metadata architecture pivot. +**Created**: 2026-06-08 +**Feature**: `specs/033-gradio-agent-chat/spec.md` + +## 1. Requirement Completeness + +- [ ] CHK001 Are all 25 functional requirements (FR-001 through FR-025) individually testable via acceptance criteria, not just stated as desired behavior? [Completeness, Spec §Requirements] + +- [ ] CHK002 Do Stories 3 through 6 have explicit, written acceptance criteria matching the level of detail in Stories 1 and 2? [Gap, Spec §User Stories — Stories 3-6 lack explicit AC] + +- [ ] CHK003 Are rollback/recovery requirements defined for every state mutation operation (delete conversation, tool execution, confirmation timeout)? [Coverage, Gap, Spec §Edge Cases] + +- [ ] CHK004 Are requirements for the ``additional_inputs`` payload shape (which fields, order, types) documented unambiguously across all three layers (Gradio Python, Gradio JS, Svelte model)? [Completeness, Spec §FR-001/FR-009] + +- [ ] CHK005 Does the spec define how Gradio JS ``Client.submit()`` payload maps to Gradio Python handler parameters for the ``__resume__`` case? [Gap, Spec §FR-009] + +- [ ] CHK006 Are file upload rejection reasons (size, format, encryption) specified as part of the requirements rather than deferred to implementation? [Completeness, Spec §FR-021] + +## 2. Requirement Clarity + +- [ ] CHK007 Is "structured JSON metadata in ChatMessage.metadata" (FR-016) specified with an exhaustive list of all ``metadata.type`` values and their payload shapes? [Clarity, Spec §FR-016, data-model.md §1] + +- [ ] CHK008 Are the 7 ``StreamMetadata`` type discriminator values (stream_token, tool_start, tool_end, tool_error, confirm_required, confirm_resolved, error) documented in both spec and data-model without drift? [Consistency, Spec §FR-016 vs data-model.md §1] + +- [ ] CHK009 Is "concurrency_limit=1" specified as per-handler or per-user, and are the consequences of this choice documented? [Clarity, Spec §FR-015 — risk of global limit vs per-user limit] + +- [ ] CHK010 Is the "stateless JWT validation" (FR-020) explicitly defined in terms of algorithm, required claims, and secret management? [Clarity, Spec §FR-020] + +- [ ] CHK011 Does "RunnableWithMessageHistory auto-loads context" (FR-013) define what "context" means — last N messages, summary, or full thread? [Clarity, Spec §FR-013] + +- [ ] CHK012 Is "soft-delete / Archive" (FR-011) consistently described across all artifacts (spec says Archive, tasks say delete, fixtures say is_archived=true)? [Consistency, Spec §FR-011 vs tasks.md T055 vs fixtures] + +## 3. Requirement Consistency + +- [ ] CHK013 Do all contracts in ``contracts/modules.md`` reference the correct LangGraph API (``create_react_agent``, ``PostgresSaver``, ``interrupt()``, ``Command(resume=...)``) without leftover ``HumanInTheLoopMiddleware`` or ``create_agent`` references? [Consistency, contracts/modules.md — check AgentChat.LangChain.Setup for stale middleware ref] + +- [ ] CHK014 Do the research decisions (§1-8) use the same terminology as the spec FRs? (Research §3 says "HumanInTheLoopMiddleware" but spec FR-008 says "interrupt()" — drift.) [Consistency, research.md §3 vs spec.md FR-008] + +- [ ] CHK015 Is "dual identity" defined identically in spec (FR-007/FR-019), api-ux.md, data-model.md, and tasks.md — same header names, same validation sequence? [Consistency across artifacts] + +- [ ] CHK016 Do the 19 fixture files in ``fixtures/`` align with the C3+ contracts they claim to verify — correct endpoint paths, correct error codes, correct response shapes? [Traceability, fixtures/manifest.md vs contracts/modules.md] + +- [ ] CHK017 Are task numbers in tasks.md unique and non-overlapping? (T026 appears in both Phase 3 and Phase 4.) [Consistency, tasks.md — numbering overlap] + +- [ ] CHK018 Does plan.md's summary match the final LangGraph/structured-metadata architecture, or does it still reference LangChain v1/AgentExecutor patterns? [Consistency, plan.md — after multiple overwrites from setup-plan.sh] + +## 4. Acceptance Criteria Quality + +- [ ] CHK019 Can SC-001 (tool selection ≥85% accuracy) be measured with the acceptance test suite defined in tasks, or is a separate benchmark methodology required? [Measurability, Spec §Success Criteria] + +- [ ] CHK020 Is "first token <1.5s" (SC-002) testable end-to-end (browser → nginx → Gradio → LangGraph → LLM → back) or only in isolation? [Measurability, Spec §SC-002] + +- [ ] CHK021 Are the P3 Stories (Story 5 — conversation persistence, Story 6 — file upload) verifiable independently of the P1 streaming chat, per the "story independence" claim? [Measurability, tasks.md §Story Independence] + +- [ ] CHK022 Does "UX reference validation — verify all 15 FSM states" (T090) define acceptance criteria for each state, or just a blanket requirement to verify? [Clarity, tasks.md T090] + +## 5. Scenario Coverage + +- [ ] CHK023 Are requirements specified for the "Gradio handler receives confirm/deny action in additional_inputs" scenario — what happens if action is neither "confirm", "deny", nor null? [Coverage, Exception Flow, Spec §FR-009] + +- [ ] CHK024 Are requirements specified for "second submit arrives before first stream ended" (race condition between HITL confirm and primary stream cleanup)? [Coverage, Exception Flow, Gap] + +- [ ] CHK025 Are requirements specified for PostgresSaver initialization — auto-create tables, migration script, or manual setup? [Coverage, Gap — data-model.md mentions checkpointer but not schema setup] + +- [ ] CHK026 Are requirements specified for the "conversation_id mismatch" scenario — what happens when additional_inputs conversation_id doesn't match the primary message context? [Coverage, Gap] + +- [ ] CHK027 Are requirements specified for Gradio container startup ordering — does the agent container wait for FastAPI and PostgreSQL to be ready before starting? [Coverage, Dependency] + +## 6. Edge Case Coverage + +- [ ] CHK028 Is the edge case "LLM returns malformed tool-call JSON → retry" (listed in spec) traceable to a specific FR or task? (Not found in tasks T082 — retry task was removed.) [Gap, Spec §Edge Cases vs tasks.md] + +- [ ] CHK029 Is "message too long → truncate" quantified with a specific token limit in an FR? (No FR defines the limit.) [Gap, Spec §Edge Cases] + +- [ ] CHK030 Is the edge case "Gradio container restarts mid-conversation" addressed for BOTH in-memory state (lost) AND checkpoint state (preserved via PostgresSaver)? [Coverage, Spec §Edge Cases vs FR-012] + +- [ ] CHK031 Is the "concurrent message submit from second tab" rejection UX specified — toast, error state, or silent drop? [Gap, Spec §Edge Cases] + +## 7. Non-Functional Requirements + +- [ ] CHK032 Are performance requirements specified for all critical paths — not just first-token latency (SC-002) but also tool-call execution, confirmation resume, and file upload parsing? [Completeness, Non-Functional] + +- [ ] CHK033 Are time-to-live (TTL) or garbage-collection requirements specified for: pending interrupts, archived conversations, expired service JWTs, stale checkpoints? [Completeness, Non-Functional] + +- [ ] CHK034 Are concurrency requirements specified for the Gradio container — max concurrent users, max conversations per user, max messages per conversation? [Gap, Non-Functional, Scale] + +- [ ] CHK035 Are accessibility requirements (ARIA roles, keyboard navigation, screen reader flows) specified for the new chat components? [Gap, Non-Functional, A11y] + +- [ ] CHK036 Are monitoring/observability requirements specified beyond audit logging — Gradio container healthcheck, LangGraph execution metrics, streaming latency tracking? [Gap, Non-Functional, Observability] + +## 8. Dependencies & Assumptions + +- [ ] CHK037 Is the assumption that ``langgraph-checkpoint-postgres`` auto-creates required tables validated, or is an explicit migration setup required? [Assumption, Spec §FR-012, Gap in T016] + +- [ ] CHK038 Is the version constraint ``langgraph>=0.2`` validated against actual PyPI availability, or is this a best-guess version? [Assumption, Dependencies] + +- [ ] CHK039 Is the dependency ``multimodal=True`` (FR-001) documented with its side effects — that ``message`` becomes ``{text, files}`` dict and the handler signature changes? [Clarity, Spec §FR-021 vs FR-001] + +- [ ] CHK040 Are external service dependencies (LLM provider, PostgreSQL) documented with failure-mode requirements — what happens on LLM timeout vs PostgreSQL down vs nginx misconfig? [Completeness, Dependencies] + +## 9. Decision Memory & ADRs + +- [ ] CHK041 Are all 5 @REJECTED patterns from spec.md traceable to verification tasks that explicitly prevent their resurrection? (T092 covers 3, T094 covers 1 — is @REJECTED "REST confirmation endpoints" covered?) [Gap, Decision Memory, Spec @REJECTED vs tasks] + +- [ ] CHK042 Is the decision to reject REST confirmation endpoints (research §3) guarded by a specific task or invariant, or is it only stated in research.md? [Traceability, research.md §3 → tasks] + +- [ ] CHK043 Does the "dual identity" RBAC decision have explicit @RATIONALE explaining WHY both service JWT and user JWT are needed rather than just one? [Decision Memory, Spec §FR-007] + +- [ ] CHK044 Are the "8 rejected alternatives" from research.md traceable to specific guardrails in tasks.md — or are they informational only, with no enforcement mechanism? [Traceability, research.md vs tasks.md] + +- [ ] CHK045 Are ADR artifacts explicitly referenced from the plan where the feature depends on, extends, or overrides an existing ADR? (FR-022 references ADR-0002, FR-005 references ADR-0005/0008 — are ADR numbers correct?) [Traceability, Spec §FR-005/FR-022 vs docs/adr/] + +- [ ] CHK046 Can a coder determine from the planning artifacts alone which implementation shortcuts are forbidden without re-reading research or spec? (e.g., "don't use WebSocket, don't add REST confirmation endpoints, don't reuse @assistant_tool") [Clarity, Decision Memory] + +## 10. Ambiguities & Conflicts + +- [ ] CHK047 Does the term "concurrency_limit=1" in FR-015 create a conflict with the requirement to handle multiple users simultaneously? (Gradio concurrency_limit is global, not per-user.) [Conflict, Spec §FR-015 vs multi-user requirement] + +- [ ] CHK048 Is there a conflict between "Gradio does NOT pass history — only conversation_id" (FR-013) and the fact that Gradio ChatInterface inherently passes ``history`` as the second handler parameter? [Conflict, Spec §FR-013 vs Gradio API contract] + +- [ ] CHK049 Does the ``AgentChat.LangChain.Setup`` contract in ``contracts/modules.md`` still reference ``HumanInTheLoopMiddleware``, conflicting with spec FR-008 (LangGraph interrupt)? [Conflict, contracts/modules.md vs spec.md FR-008] + +- [ ] CHK050 Does the ``ResumeConfirm``/``ConfirmationCard`` contract reference ``__resume__`` while api-ux.md uses ``additional_inputs[1]="confirm"/"deny"``? [Conflict, contracts/modules.md vs api-ux.md] + +- [ ] CHK051 Does "backend/src/agent/app.py" plan to run a separate Gradio entrypoint, and does this file path conflict with the existing backend src structure (which uses ``__init__.py`` based packages)? [Ambiguity, plan.md project structure vs existing backend architecture] diff --git a/specs/033-gradio-agent-chat/contracts/modules.md b/specs/033-gradio-agent-chat/contracts/modules.md index 945e7ca6..1086d7b4 100644 --- a/specs/033-gradio-agent-chat/contracts/modules.md +++ b/specs/033-gradio-agent-chat/contracts/modules.md @@ -1,30 +1,35 @@ -#region AgentChat.Contracts [C:4] [TYPE ADR] [SEMANTICS contracts,modules,agent-chat,final] -@BRIEF Final module contracts — LangChain v1, PostgreSQL checkpointer, structured JSON metadata, dual-identity RBAC. +#region AgentChat.Contracts [C:4] [TYPE ADR] [SEMANTICS contracts,modules,agent-chat,langgraph,final] +@BRIEF Final module contracts — LangGraph `create_react_agent()`, `@tool`, `interrupt()`/`Command(resume=...)`, `PostgresSaver`, structured JSON metadata, dual-identity RBAC. ## Backend | Contract | C | File | Notes | |----------|---|------|-------| -| AgentChat.GradioApp | C4 | `backend/src/agent/app.py` | `gr.ChatInterface(type="messages", multimodal=True, additional_inputs=[gr.Textbox("conversation_id")], examples=[["Покажи дашборды",null],["Статус системы",null]], concurrency_limit=1)`. Handler: `def handler(message, history, request: gr.Request, conversation_id=None)` | -| AgentChat.GradioApp.Handler | C4 | ↑ | Creates `create_react_agent()` with PostgresSaver. `astream_events(config={"configurable":{"thread_id":conversation_id}})`. Resume: detects `"confirm"/"deny"` in additional_inputs, invokes `Command(resume=...)` | -| AgentChat.LangChain.Setup | C4 | `backend/src/agent/langchain_setup.py` | `create_agent(model, tools=ALL_TOOLS, middleware=[LoggingMiddleware(), HumanInTheLoopMiddleware(interrupt_on=DANGEROUS)], checkpointer=PostgresSaver(conn_string))`. Wrapped in `RunnableWithMessageHistory` | -| AgentChat.Tools | C4 | `backend/src/agent/tools.py` | Native `@tool` functions. Each: Pydantic `BaseModel` args_schema, calls FastAPI REST with dual-identity headers | -| AgentChat.Middleware | C3 | `backend/src/agent/middleware.py` | `LoggingMiddleware`, risk config dict | -| AgentChat.Document.Parser | C3 | `backend/src/agent/document_parser.py` | pdfplumber + openpyxl | -| AgentChat.Api.Conversations | C3 | `backend/src/api/routes/agent_conversations.py` | CRUD + multi-tab gate | -| AgentChat.Api.ServiceToken | C3 | `backend/src/api/routes/auth.py` | `POST /api/auth/service-token` | -| Models.Agent | C2 | `backend/src/models/agent.py` | AgentConversation, AgentMessage | -| Schemas.Agent | C1 | `backend/src/schemas/agent.py` | Pydantic | +| AgentChat.GradioApp | C4 | `backend/src/agent/app.py` | `gr.ChatInterface(type="messages", multimodal=True, additional_inputs=[gr.Textbox("conversation_id"), gr.Textbox("action")], examples=[["Покажи дашборды",null,null],["Статус системы",null,null]])`. Handler: `def handler(message, history, request: gr.Request, conversation_id=None, action=None)`. Per-user lock via in-memory dict (no global concurrency_limit — Gradio handles concurrent users by default). | +| AgentChat.GradioApp.Handler | C4 | ↑ | Creates `create_react_agent()` with PostgresSaver. `astream_events(config={"configurable":{"thread_id":conversation_id}})`. Resume: detects `action="confirm"/"deny"` in additional_inputs, loads checkpoint, invokes `Command(resume=...)`. | +| AgentChat.LangGraph.Setup | C4 | `backend/src/agent/langgraph_setup.py` | `create_react_agent(model, tools=ALL_TOOLS, checkpointer=PostgresSaver(conn_string))`. `interrupt_before=DANGEROUS_TOOLS`. Wrapped in `RunnableWithMessageHistory`. No custom middleware. | +| AgentChat.Tools | C4 | `backend/src/agent/tools.py` | Native `@tool` functions. Each: Pydantic `BaseModel` args_schema, calls FastAPI REST with dual-identity headers (`Authorization: service JWT` + `X-User-JWT: user JWT`). | +| AgentChat.Middleware | C3 | `backend/src/agent/middleware.py` | `LoggingMiddleware` only (audit trail). | +| AgentChat.Document.Parser | C3 | `backend/src/agent/document_parser.py` | pdfplumber (PDF) + openpyxl (XLSX). | +| AgentChat.Api.Conversations | C3 | `backend/src/api/routes/agent_conversations.py` | CRUD (list, history, create, archive) + multi-tab gate `GET .../conversations/{id}/active`. | +| AgentChat.Api.ServiceToken | C3 | `backend/src/api/routes/auth.py` | `POST /api/auth/service-token` — returns service JWT (role `agent`). | +| Models.Agent | C2 | `backend/src/models/agent.py` | AgentConversation, AgentMessage. | +| Schemas.Agent | C1 | `backend/src/schemas/agent.py` | Pydantic: ConversationItem, ConversationListResponse, MessageItem, HistoryResponse, DeleteResponse. | ## Frontend | Contract | C | File | Notes | |----------|---|------|-------| -| AgentChat.Model | C4 | `AgentChatModel.svelte.ts` | `submit()` lifecycle, metadata parsing, resume logic. No tabRole, no follower | -| AgentChat.Panel | C4 | `AssistantChatPanel.svelte` | `Client.connect("/api/agent/gradio")`, `submit()`, metadata-based rendering | -| AgentChat.Page | C3 | `frontend/src/routes/agent/+page.svelte` | Two-column | -| AgentChat.ConversationList | C3 | `ConversationList.svelte` | Search, infinite scroll, date grouping | -| AgentChat.ToolCallCard | C2 | `ToolCallCard.svelte` | Props from metadata | -| AgentChat.ConfirmationCard | C3 | `ConfirmationCard.svelte` | Calls second `submit()` with `__resume__` | +| AgentChat.Model | C4 | `frontend/src/lib/models/AgentChatModel.svelte.ts` | `submit("/chat", {text,files}, [conversationId, action])` lifecycle. Structured `ChatMessage.metadata` parsing. HITL resume via second submit with `additional_inputs[1]="confirm"/"deny"`. No tabRole, no follower. | +| AgentChat.Panel | C4 | `frontend/src/lib/components/assistant/AssistantChatPanel.svelte` | `Client.connect("/api/agent/gradio")`, `submit()`, metadata-based rendering of tool cards + confirmation cards. | +| AgentChat.Page | C3 | `frontend/src/routes/agent/+page.svelte` | Two-column layout: conversation list (240px) + chat area. | +| AgentChat.ConversationList | C3 | `frontend/src/lib/components/assistant/ConversationList.svelte` | Search, infinite scroll, date grouping, archive action. | +| AgentChat.ToolCallCard | C2 | `frontend/src/lib/components/assistant/ToolCallCard.svelte` | Props from `metadata.type="tool_start"/"tool_end"/"tool_error"`. | +| AgentChat.ConfirmationCard | C3 | `frontend/src/lib/components/assistant/ConfirmationCard.svelte` | Renders on `metadata.type="confirm_required"`. Calls second `submit()` with `additional_inputs=[conversation_id, "confirm"/"deny"]`. | + +## Contracts Removed (vs prior versions) +- `AgentChat.LangChain.Setup` → renamed to `AgentChat.LangGraph.Setup` +- `HumanInTheLoopMiddleware` → replaced by `interrupt_before=DANGEROUS_TOOLS` (LangGraph native) +- `__resume__` field → replaced by `additional_inputs[1]="confirm"/"deny"` #endregion AgentChat.Contracts diff --git a/specs/033-gradio-agent-chat/contracts/ux/api-ux.md b/specs/033-gradio-agent-chat/contracts/ux/api-ux.md index 41ac6e43..22bbfce7 100644 --- a/specs/033-gradio-agent-chat/contracts/ux/api-ux.md +++ b/specs/033-gradio-agent-chat/contracts/ux/api-ux.md @@ -28,19 +28,20 @@ ### HITL Resume Protocol ``` -Primary: submit("/chat", {message, conversation_id}) - → handler: agent.astream_events(config={"thread_id": conversation_id}) - → HITL fires → checkpoint saved (PostgreSQL) +Primary: submit("/chat", {message}, [conversation_id, null]) + → handler: graph.astream_events(config={"thread_id": conversation_id}) + → interrupt_before fires → checkpoint saved (PostgreSQL) → handler: yield {metadata: {type: "confirm_required", thread_id: conversation_id}} - → Svelte: renders confirmation card (stream still open) + → Svelte: renders confirmation card (primary stream ends) -Confirm: submit("/chat", message, [conversation_id, "confirm"]) - → handler: detects "confirm" in additional_inputs[1] - → loads checkpoint by conversation_id as thread_id - → invokes graph with Command(resume={"action": "confirm"}, config={"thread_id": conversation_id}) +Confirm: submit("/chat", {message: "confirm", action: "confirm", conversation_id}, [conversation_id, "confirm"]) + → handler: detects action="confirm" → loads checkpoint by thread_id + → invokes graph with Command(resume={"action": "confirm"}, config={"thread_id": conversation_id}) + → graph resumes from checkpoint → new stream begins -Deny: submit("/chat", message, [conversation_id, "deny"]) +Deny: submit("/chat", {message: "deny", action: "deny", conversation_id}, [conversation_id, "deny"]) → handler: Command(resume={"action": "deny"}, config=...) + → yields confirm_resolved(result="denied") → agent responds ``` --- diff --git a/specs/033-gradio-agent-chat/contracts/ux/decisions.md b/specs/033-gradio-agent-chat/contracts/ux/decisions.md index edc287aa..39aa985d 100644 --- a/specs/033-gradio-agent-chat/contracts/ux/decisions.md +++ b/specs/033-gradio-agent-chat/contracts/ux/decisions.md @@ -20,7 +20,7 @@ | **Confirmation timeout** | A — Карточка остаётся, кнопки заблокированы, «Повторить» | Явный контроль | | **Reconnect** | A — 5×5s, затем disconnected_permanent + ручная кнопка | Предсказуемо | | **Ошибка истории** | A — Сразу ошибка, без кэша | Консистентность | -| **Multi-tab** | A — Client-side gate: `GET /api/agent/conversations/{id}/active` перед отправкой. Отклоняет вторую вкладку | Просто, без server-push. `concurrency_limit=1` на Gradio исключает гонки | +| **Multi-tab** | A — Client-side gate: `GET /api/agent/conversations/{id}/active` перед отправкой. Отклоняет вторую вкладку | Просто, без server-push. `concurrency_limit=1` на Gradio сериализует на сервере | ### Phase 3 — Interaction Design @@ -42,7 +42,7 @@ - **Custom WebSocket protocol** — rejected: @gradio/client не raw WebSocket - **REST confirmation endpoints** — rejected: HumanInTheLoopMiddleware + resume protocol проще - **Ручная передача history** — rejected: RunnableWithMessageHistory делает автоматически -- **Active/follower multi-tab** — rejected: client-side gate + concurrency_limit=1 достаточно +- **WebSocket-based active/follower multi-tab pattern** — rejected: сложный server-push, требует pub/sub на Gradio. Client-side gate + concurrency_limit=1 — accepted. - **@assistant_tool registry для новых тулов** — rejected: нативные @tool функции LangChain #endregion AgentChat.UxDecisions diff --git a/specs/033-gradio-agent-chat/research.md b/specs/033-gradio-agent-chat/research.md index 83a9149c..a2a4151d 100644 --- a/specs/033-gradio-agent-chat/research.md +++ b/specs/033-gradio-agent-chat/research.md @@ -13,11 +13,11 @@ **Rationale**: Single source of metadata. Auto OpenAI function schema. Old `@assistant_tool` → `@DEPRECATED`. **Rejected**: Dual registration (`@assistant_tool` + `StructuredTool`) — redundant. -## 3. Confirmation: `HumanInTheLoopMiddleware` +## 3. Confirmation: `interrupt_before=DANGEROUS_TOOLS` -**Decision**: Second `submit()` with `__resume__` protocol. In-memory pending interrupts in handler. -**Rationale**: Zero REST endpoints. LangChain checkpoint ensures safe resume. -**Rejected**: REST confirmation endpoints + polling — more code, more latency. +**Decision**: LangGraph native `interrupt_before=DANGEROUS_TOOLS` + `Command(resume=...)`. No custom `HumanInTheLoopMiddleware` — LangGraph provides HITL natively via checkpointing and resume. Second `submit()` with `additional_inputs[1]="confirm"/"deny"` triggers resume. +**Rationale**: Zero REST endpoints. LangGraph checkpoint ensures safe resume. +**Rejected**: REST confirmation endpoints + polling — more code, more latency. Custom middleware — LangGraph has native interrupt support. ## 4. History: `RunnableWithMessageHistory` diff --git a/specs/033-gradio-agent-chat/spec.md b/specs/033-gradio-agent-chat/spec.md index 1e741062..eedb41ad 100644 --- a/specs/033-gradio-agent-chat/spec.md +++ b/specs/033-gradio-agent-chat/spec.md @@ -53,11 +53,41 @@ Browser (SvelteKit) Docker ### Story 3 — Tool-Call Visibility (P2) -### Story 4 — Multi-Turn Context (P2) — via `RunnableWithMessageHistory` +**Independent Test**: Send multi-tool query, verify each tool appears as inline card with spinner → checkmark/cross transition. -### Story 5 — Conversation Persistence (P3) — PostgreSQL SSOT, REST CRUD +**Acceptance**: +1. **Given** agent triggers a tool, **When** `metadata.type="tool_start"` received, **Then** UI shows card with tool name (mono font) and spinner. +2. **Given** tool completes, **When** `metadata.type="tool_end"` received, **Then** spinner becomes green checkmark, result expandable on click. +3. **Given** tool fails, **When** `metadata.type="tool_error"` received, **Then** spinner becomes red cross with error detail expandable on click. -### Story 6 — File Upload & Document Analysis (P2) — multimodal=True, pdfplumber/openpyxl +### Story 4 — Multi-Turn Context (P2) + +**Independent Test**: Ask "покажи дашборд 42", then "создай для него ветку". Agent resolves "него" → dashboard 42. + +**Acceptance**: +1. **Given** prior messages in conversation, **When** user uses pronouns/implicit references, **Then** agent resolves them via `RunnableWithMessageHistory` context. +2. **Given** conversation exceeds 4000 tokens, **When** new message sent, **Then** older messages are summarized/truncated before being passed to LLM. +3. **Given** user starts new conversation, **When** referencing entities from prior conversation, **Then** agent does NOT have access to prior context (clean isolation). + +### Story 5 — Conversation Persistence (P3) + +**Independent Test**: Create 3 conversations, close browser tab, reopen — all 3 visible, resume any. + +**Acceptance**: +1. **Given** past conversations exist, **When** user opens chat panel, **Then** conversations listed with auto-titles (truncated first message 60 chars), dates, message counts. +2. **Given** past conversation selected, **When** user sends new message, **Then** `RunnableWithMessageHistory` loads previous context from PostgreSQL. +3. **Given** conversation archived, **When** user deletes, **Then** `is_archived=true`, removed from active list, visible in archive filter. +4. **Given** conversation archived, **When** associated checkpoints cleared, **Then** no zombie threads remain in `langgraph-checkpoint-postgres` tables. + +### Story 6 — File Upload & Document Analysis (P2) + +**Independent Test**: Upload PDF report, type "выдели риски". Agent extracts text and responds with structured analysis. + +**Acceptance**: +1. **Given** user attaches PDF, **When** message sent, **Then** backend extracts text via pdfplumber, injects as system message context. +2. **Given** user attaches XLSX, **When** message sent, **Then** backend parses sheets via openpyxl, presents structured data to agent. +3. **Given** user attaches unsupported format (`.exe`, `.zip`), **When** upload attempted, **Then** UI shows validation error with supported format list. +4. **Given** file exceeds 10MB, **When** upload attempted, **Then** UI rejects with size limit message. ### Edge Cases - Gradio unreachable → reconnect 5×5s @@ -82,17 +112,17 @@ Browser (SvelteKit) Docker ### Confirmation (HumanInTheLoop) - **FR-008**: LangGraph `interrupt()` in dangerous tool nodes (`deploy_dashboard`, `execute_migration`, `commit_changes`). Handled entirely by LangGraph — **zero REST confirmation endpoints**. -- **FR-009**: Resume mechanism: graph pauses at `interrupt_before` node → primary `submit()` stream yields `metadata.type="confirm_required"` with `thread_id` and **terminates** (stream ends, does NOT wait). User confirms → second `submit("/chat", msg, [conversation_id, "confirm"])` → handler loads checkpoint by thread_id → invokes graph with `Command(resume={"action":"confirm"}, config={"thread_id":conversation_id})` → graph resumes from checkpoint → new stream begins. +- **FR-009**: Resume mechanism: graph pauses at `interrupt_before` node → primary `submit()` stream yields `metadata.type="confirm_required"` with `thread_id` and terminates. User confirms → second `submit("/chat", msg, additional_inputs=[conversation_id, "confirm"])` → handler loads checkpoint by thread_id. If checkpoint not found: yield `metadata.type="error" code="CHECKPOINT_EXPIRED"`. If found: invoke `Command(resume={"action":"confirm"}, config={"thread_id":conversation_id})`. HITL resume waits for primary stream cleanup via per-conversation mutex (max 2s). ### Persistence - **FR-010**: Conversation history in PostgreSQL (`agent_conversations`, `agent_messages`). REST: `POST /api/assistant/conversations`, `GET .../conversations`, `GET .../history`, `DELETE .../{id}`. - **FR-011**: Soft-delete: `is_archived=true`. Renamed to "Archive" in UX. - **FR-012**: LangGraph checkpointer uses **PostgreSQL** (same instance as FastAPI) via `langgraph-checkpoint-postgres`. Survives container restarts. Thread ID = conversation_id. -- **FR-013**: `RunnableWithMessageHistory` auto-loads context from PostgreSQL on resume. Svelte does NOT pass `history` — only `conversation_id` via `additional_inputs`. +- **FR-013**: `RunnableWithMessageHistory` auto-loads context from PostgreSQL on resume. Svelte passes `conversation_id` via `additional_inputs` only. The Gradio handler receives `history` from Gradio's built-in `ChatInterface` (mandatory parameter) but MUST ignore it — loading context from PostgreSQL by `thread_id=conversation_id` instead. ### UX & Cancel - **FR-014**: `submission.cancel()` — native cancel. Partial text preserved. -- **FR-015**: `concurrency_limit=1` on handler. Server-side per-user lock prevents concurrent sends. `GET /api/agent/conversations/{id}/active` as additional gate for multi-tab. +- **FR-015**: Per-user lock via in-memory dict in handler prevents concurrent sends within same user session. `GET /api/agent/conversations/{id}/active` as additional gate for multi-tab. Gradio handles concurrent users by default — no global `concurrency_limit` needed. - **FR-016**: Tool-call visibility via **structured JSON metadata in `ChatMessage.metadata`**, not emoji string parsing. `{type: "tool_start", tool: "...", input: {...}}`. - **FR-017**: Confirmation card renders from `ChatMessage.metadata.confirm_required`. @@ -112,6 +142,14 @@ Browser (SvelteKit) Docker - **FR-024**: Audit logging via `LoggingMiddleware` — all interactions to `assistant_audit`. - **FR-025**: RU/EN i18n keys for all new UI strings. +- **FR-026**: Resume `submit()` MUST wait for primary stream cleanup via per-conversation mutex before loading checkpoint. Primary acquires lock on start, releases on stream end. Resume waits for lock release (max 2s timeout). + +- **FR-027**: Deployment MUST run `PostgresSaver.setup()` on agent container startup to create checkpoint tables (`checkpoints`, `checkpoint_writes`, `checkpoint_migrations`). Managed via startup hook. + +- **FR-028**: User messages exceeding 100,000 characters (~25k tokens) are truncated to 100,000 characters with `[...truncated]` suffix appended. Truncation occurs at sentence boundary when possible. + +- **FR-029**: When conversation is archived (soft-deleted), agent MUST clear associated checkpoints from `langgraph-checkpoint-postgres` tables for that `thread_id`. + ## Success Criteria - **SC-001**: Tool selection ≥85% accuracy diff --git a/specs/033-gradio-agent-chat/tasks.md b/specs/033-gradio-agent-chat/tasks.md index 19806cfe..f6cff56a 100644 --- a/specs/033-gradio-agent-chat/tasks.md +++ b/specs/033-gradio-agent-chat/tasks.md @@ -39,14 +39,19 @@ ### Backend -- [ ] T015 [US1] Implement `backend/src/agent/app.py` — Gradio `gr.ChatInterface(fn=agent_handler, type="messages", multimodal=True, additional_inputs=[gr.Textbox("conversation_id"), gr.Textbox("action")], examples=[["Покажи дашборды", null, null], ["Статус системы", null, null]], concurrency_limit=10)`. Handler: `agent_handler(message, history, request: gr.Request, conversation_id=None, action=None)`. On `action="confirm"/"deny"`: resume graph from checkpoint. Per-user lock via in-memory dict keyed by user_id. +- [ ] T015 [US1] Implement `backend/src/agent/app.py` — Gradio `gr.ChatInterface(fn=agent_handler, type="messages", multimodal=True, additional_inputs=[gr.Textbox("conversation_id"), gr.Textbox("action")], examples=[["Покажи дашборды", null, null], ["Статус системы", null, null]], concurrency_limit=1)`. Handler: `agent_handler(message, history, request: gr.Request, conversation_id=None, action=None)`. On `action="confirm"/"deny"`: resume graph from checkpoint. Per-user lock via in-memory dict keyed by user_id. + RATIONALE concurrency_limit=1 per FR-015 — serializes sends per user, prevents concurrent stream conflicts - [ ] T016 [US1] Implement `backend/src/agent/langgraph_setup.py` — `create_react_agent(model, tools, checkpointer=PostgresSaver(os.getenv("DATABASE_URL")))`. Define dangerous tools in `DANGEROUS_TOOLS` set. Compile graph with `interrupt_before=DANGEROUS_TOOLS`. Wrap with `RunnableWithMessageHistory` for auto history load/save (thread_id = conversation_id). @POST Compiled StateGraph with checkpointer, interrupt_before, message history RATIONALE langgraph provides native interrupt/resume; no custom HITL middleware needed -- [ ] T017 [US1] Implement agent_handler yield loop in `app.py` — `async for event in graph.astream_events({"messages": [HumanMessage(content=message["text"])]}, config={"configurable": {"thread_id": conversation_id}}): yield to_chatmessage(event)`. Tool events → `ChatMessage(metadata={"type":"tool_start", "tool":..., "input":...})`. Interrupt events → `ChatMessage(metadata={"type":"confirm_required", "prompt":..., "thread_id":conversation_id})`. - @POST Each LangGraph event converted to ChatMessage with structured metadata; frontend receives typed objects +- [ ] T017 [US1] Implement agent_handler yield loop in `app.py` — extract user JWT from `request.headers["Authorization"]`, set via `ContextVar` BEFORE graph invocation (see `backend/src/agent/context.py`). Then: `async for event in graph.astream_events({"messages": [HumanMessage(content=message["text"])]}, config={"configurable": {"thread_id": conversation_id}}): yield to_chatmessage(event)`. Tool events → `ChatMessage(metadata={"type":"tool_start", "tool":..., "input":...})`. Interrupt events → `ChatMessage(metadata={"type":"confirm_required", "prompt":..., "thread_id":conversation_id})`. + @POST Each LangGraph event converted to ChatMessage with structured metadata; user JWT propagated via ContextVar + @SIDE_EFFECT Sets ContextVar with user JWT before graph run, resets after + +- [ ] T017a [P] [US1] Create `backend/src/agent/context.py` — `ContextVar[str]` for `_user_jwt_context` and `_service_jwt_context`. Thread-safe JWT storage for @tool functions to access auth headers without explicit parameter passing. + RATIONALE LangGraph tools cannot receive per-request auth via graph config — ContextVar bridges Gradio handler → tool execution ### Frontend @@ -78,16 +83,18 @@ ### Backend -- [ ] T026 [US2] Implement `backend/src/agent/tools.py` — native LangChain `@tool`-декорированные функции. Каждый `@tool`: Pydantic `BaseModel` для args_schema, docstring для description, внутри → HTTP-вызов к FastAPI REST с service JWT. Пример: `@tool async def search_dashboards(query: str, environment: str = None) → str`. Список тулов: все существующие @assistant_tool операции. - @DATA_CONTRACT Each @tool: Pydantic args → FastAPI REST call → JSON string result - RATIONALE Native @tool replaces dual @assistant_tool + StructuredTool wrapping +- [ ] T026 [US2] Implement `backend/src/agent/tools.py` — native LangChain `@tool`-декорированные функции. Каждый `@tool`: Pydantic `BaseModel` для args_schema, docstring для description, внутри → читает user JWT из `_user_jwt_context.get()` (ContextVar из T017a), service JWT из `_service_jwt_context.get()`, отправляет HTTP-запрос к FastAPI REST с dual-identity заголовками: `Authorization: Bearer {service_jwt}` + `X-User-JWT: {user_jwt}`. Список тулов: все существующие @assistant_tool операции. + @DATA_CONTRACT Each @tool: Pydantic args → dual-identity HTTP call → JSON string result + @PRE ContextVars set by handler before graph invocation + RATIONALE ContextVar is the ONLY mechanism to bridge Gradio handler's `request.headers` to @tool function execution in LangGraph — graph config does not propagate per-request auth - [ ] T027 [US2] Implement `backend/src/agent/middleware.py` — `LoggingMiddleware` (логирует tool-call события в audit trail), `ConfirmationRiskMiddleware` (определяет interrupt_on список из конфига TOOL_RISK_LEVELS). - [ ] T028 [US2] Register tools in `langgraph_setup.py` — `DANGEROUS_TOOLS = {"deploy_dashboard", "execute_migration", "commit_changes"}`. Compile graph with `interrupt_before=DANGEROUS_TOOLS`. All tools from `tools.py` registered as graph nodes. @POST Agent with tools, interrupt, logging, checkpoints, history — all LangChain-native -- [ ] T029 [US2] Implement interrupt handler in `app.py` — when graph pauses at `interrupt_before` node: yield `ChatMessage(metadata={"type":"confirm_required", ...})`. On second submit with `additional_inputs[1]="confirm"/"deny"`: invoke graph with `Command(resume={"action": action}, config={"thread_id": conversation_id})`. Graph resumes from checkpoint. +- [ ] T029 [US2] Implement interrupt handler in `app.py` — when graph pauses at `interrupt_before` node: yield `ChatMessage(metadata={"type":"confirm_required", ...})`. On second submit with `action="confirm"/"deny"`: load checkpoint via `PostgresSaver.get(thread_id)`. If checkpoint missing: yield `metadata.type="error"` with `CHECKPOINT_NOT_FOUND`. If found: invoke graph with `Command(resume={"action": action}, config={"thread_id": conversation_id})`. Graph resumes from checkpoint. + @TEST_EDGE: checkpoint_not_found -> error_metadata, expired_graph_state -> error_metadata - [ ] T031 [US2] Implement confirmation rendering in `AssistantChatPanel.svelte` — on `metadata.type="confirm_required"`: warning card + Confirm/Deny. Confirm → `submit("/chat", {text:"confirm"}, [conversationId, "confirm"])`. Deny → `submit("/chat", {text:"deny"}, [conversationId, "deny"])`. On `metadata.type="confirm_resolved"`: collapse card. @@ -269,10 +276,10 @@ - [ ] T079 Implement Gradio healthcheck endpoint — `GET /health` returns `{"status":"ok","uptime":...}` for Docker healthcheck. -- [ ] T080 [P] Implement `backend/src/api/routes/auth.py` — internal endpoint `POST /api/auth/service-token` that accepts a static service secret (env `SERVICE_TOKEN_SECRET`) and returns a long-lived JWT with role `agent`. Used by Gradio container at startup for Gradio→FastAPI HTTP calls. +- [ ] T080 [P] Implement `backend/src/api/routes/auth.py` — internal endpoint `POST /api/auth/service-token` that accepts a static service secret (env `SERVICE_TOKEN_SECRET`) and returns a JWT with role `agent`, TTL 24 hours. Gradio container: fetches at startup, stores in ContextVar (T017a), auto-refreshes at 12h or on 401 response from FastAPI. After 3× refresh failure → degraded mode (logs error, rejects tool calls). @DATA_CONTRACT Input: {service_secret} → Output: {access_token, expires_in} - @TEST_EDGE: invalid_secret→401, expired_token→401_on_next_call - RATIONALE FR-023: Gradio→FastAPI calls authenticated via JWT, not separate API key mechanism + @TEST_EDGE: invalid_secret→401, expired_token→401_on_next_call, refresh_failure_3x→degraded_mode + @SIDE_EFFECT Gradio container caches token, background refresh thread - [ ] T081 [P] Add i18n keys to `frontend/src/lib/i18n/locales/ru/assistant.json` and `frontend/src/lib/i18n/locales/en/assistant.json` — new keys: `stop`, `tool_executing`, `tool_completed`, `tool_failed`, `confirm_required`, `confirm_expired`, `confirm_retry`, `file_upload`, `file_parse_error`, `file_unsupported`, `file_too_large`, `connection_lost`, `reconnecting`, `manual_reconnect`, `archive_dialog`. RATIONALE FR-013: Russian + English i18n for all new UX strings (8+ in UX reference §3-4) @@ -306,6 +313,9 @@ - [ ] T095 [P] Fixture coverage audit — verify every C3+ contract in `contracts/modules.md` has ≥1 fixture in `fixtures/manifest.md`. Report uncovered contracts. @POST All C3+ contracts covered by at least 1 fixture +- [ ] T096 [P] REST confirmation regression audit — grep `backend/src/api/routes/` for any REST confirmation/rejection endpoints (`/confirm`, `/resume`, `/approve`, `/deny`) and verify NONE exist (all confirmation handled by LangGraph HITL per @REJECTED path). + @TEST_INVARIANT: NoRESTConfirmation -> grep returns 0 matches + --- ## Task Summary @@ -314,14 +324,14 @@ |-------|---------|:-----:|:--------:| | P1 — Setup | — | T001-T009 | 6 | | P2 — Foundational | — | T010-T014 | 2 | -| P3 — US1 (Streaming) | P1 🎯 | T015-T028 | — | -| P4 — US2 (Tools) | P1 🎯 | T029-T038 | — | +| P3 — US1 (Streaming) | P1 🎯 | T015-T028a | — | +| P4 — US2 (Tools) | P1 🎯 | T026-T038 | — | | P5 — US3 (Visibility) | P2 | T039-T044 | — | | P6 — US4 (Context) | P2 | T045-T051 | — | | P7 — US5 (Persistence) | P3 | T052-T064 | — | | P8 — US6 (Files) | P2 | T065-T072 | — | | P9 — Polish + Gaps | — | T073-T095 | 12 | -| **Total** | | **95** | **19** | +| **Total** | | **96** | **20** | ## Story Independence