From 0732b48f6c6a326ac607b121e742d10cd0b88d09 Mon Sep 17 00:00:00 2001 From: tarannum-2002 Date: Thu, 29 Oct 2026 14:34:57 +0530 Subject: [PATCH 1/2] feat: send a filled form to the DSS and hand form errors back - A request carries `query` or one `form` in `content`, never both. - A `form_error` from the DSS was dropped as unknown; it now reaches the client. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + docs/api-contracts/api-contract.md | 89 +++++++++++- .../chat/adapters/dss/mapping.py | 56 ++++++- .../chat/adapters/dss/schemas.py | 30 +++- .../chat/adapters/http/mapping.py | 39 ++++- .../chat/adapters/http/schemas.py | 50 ++++++- src/experience_api/chat/domain.py | 49 ++++++- tests/api/test_chat_validation.py | 22 ++- tests/support/examples.py | 50 ++++++- tests/unit/chat/test_dss_frames.py | 137 ++++++++++++++++++ tests/unit/chat/test_dss_request.py | 50 ++++++- tests/unit/chat/test_final_answer.py | 46 ++++++ tests/unit/chat/test_http_mapping.py | 10 +- tests/unit/chat/test_request_schema.py | 77 ++++++++++ vulture_whitelist.py | 3 + 15 files changed, 679 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bc4332f..9cae86c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ All notable changes to this project are recorded here, in the format of ## [Unreleased] ### Added +- A filled form sent back as `content` in place of `query`, passed to the DSS; a `form_error` item in the answer hands it back. - A `form` item in the answer: a form the DSS offers reaches the client instead of being dropped. - CI publishes the image to `ghcr.io/openagrinet/experience-api` from main and release tags. - `EXPERIENCE_API_DSS_MODE=http` calls the real DSS: the §7 request, its event stream read leniently. diff --git a/docs/api-contracts/api-contract.md b/docs/api-contracts/api-contract.md index 4ed6d06..8f57504 100644 --- a/docs/api-contracts/api-contract.md +++ b/docs/api-contracts/api-contract.md @@ -189,8 +189,9 @@ Example in the parked fixtures: `request/follow-up.json` | Field | Type | Required | Rule | |---|---|:--:|---| | `sessionId` | string, UUID | Yes | | -| `messageId` | string, UUID | Yes | Id of the message in `query` | -| `query` | string | Yes | Not empty after trimming | +| `messageId` | string, UUID | Yes | Id of the message in `query`, or of the form in `content` | +| `query` | string | One of `query` or `content` | A typed question. Not empty after trimming | +| `content` | array of one `form` | One of `query` or `content` | A filled form sent back (§4.2). Exactly one item | | `history` | array | Yes | May be empty. Oldest first. Alternating is not required. Too long → `413` (§6.2) | | `history[].role` | `user` \| `assistant` | Yes | | | `history[].text` | string | Yes | Not empty after trimming | @@ -201,6 +202,9 @@ Example in the parked fixtures: `request/follow-up.json` | `location.longitude` | number | With `location` | −180 to 180 | | `channel` | `web` | No | Where the chat comes from. Missing or `null` → the deployment's default. Any other value → `422` | +A request carries **exactly one** of `query` and `content`. Both, or neither, +is a `422`: the DSS takes one kind of message per turn. + Why `query` is separate from `history`: the current message is explicit rather than "the last user entry". The DSS treats the last `user` message as the question; the API builds that ordering, so the client cannot get it wrong. @@ -231,6 +235,44 @@ rejection therefore always means an API bug, and is reported as - When stubs are on, nothing reaches the API, so stub replies never enter a real request. +### 4.2 Sending a form back + +When the farmer fills in a `form` (§5.2) and confirms, the client sends it as +`content` in place of `query`: + +```json +{ + "sessionId": "68a3872f-3f0d-4cf6-99a3-a350132a0080", + "messageId": "5b0e1c2a-7d4f-4a8e-9c3b-1f2e3d4c5b6a", + "content": [ + { + "type": "form", + "formId": "f-1", + "context": { "scheme": "PMFBY", "category": "Grievance", "action": "support" }, + "submissionId": "9e8d7c6b-5a4f-4e3d-8c2b-1a0f9e8d7c6b", + "fields": [ + { "name": "applicantPhone", "value": "9876543210" }, + { "name": "season", "value": "" } + ] + } + ], + "history": [], + "language": { "source": "en", "target": "en" } +} +``` + +| Field | Type | Required | Rule | +|---|---|:--:|---| +| `content[].type` | `form` | Yes | | +| `content[].formId` | string | Yes | As the `form` item gave it. Not empty | +| `content[].context` | object of strings | Yes | As the `form` item gave it, unchanged. The API does not read it | +| `content[].submissionId` | string, UUID | Yes | Made by the client, new for every submit, including a resubmit after errors | +| `content[].fields[].name` | string | Yes | Not empty | +| `content[].fields[].value` | string | Yes | May be empty: an optional field left blank | + +The API checks the shape only. The DSS checks the values against the form's +rules and hands back what is wrong as a `form_error` item (§5.2). + --- ## 5. Response — Experience API to client @@ -316,10 +358,11 @@ Example in the parked fixtures: `response/answered.json` | `outcome.status` | Yes | `answered`, `partially_answered`, `rejected`, `no_match`, `requires_input`, `unavailable`. For logs; the client does not branch on it (§5.3) | | `outcome.cause` | Yes, may be `null` | Open set, passed through. Treated as an opaque string | | `content` | Yes, may be `[]` | In order. Empty only when `error` is present | -| `content[].type` | Yes | `text`, `refusal` or `form` | +| `content[].type` | Yes | `text`, `refusal`, `form` or `form_error` | | `content[].text` | `text` and `refusal` | Markdown, as today | | `content[].citations` | `text` only; may be `[]` | `start`/`end` are Unicode code-point offsets into `text` | | `content[].formId`, `context`, `fields` | `form` only | An empty form to fill in. Shape below | +| `content[].formId`, `context`, `submissionId`, `fields` | `form_error` only | A sent form handed back. Shape below | | `sources` | Yes, may be `[]` | `{ id, name, url? }`. What citations point at | | `error` | No | Present only when the turn failed. `Error` shape (§6) | @@ -349,8 +392,36 @@ A `form` item asks the farmer for details, such as for a grievance: | `fields[].pattern`, `minLength`, `maxLength` | No | Present only when the field has the rule | | `fields[].options` | No | `{ value, label }`. Present only for a `select` | -Sending a filled form back is not in this contract yet. The answer still -carries the form, so the client can draw it. +The client sends a filled form back as `content` (§4.2). + +A `form_error` item hands a sent form back. **It means the form is still the +farmer's**: the client keeps it open and lets them send it again. + +```json +{ + "type": "form_error", + "formId": "f-1", + "context": { "scheme": "PMFBY", "category": "Grievance", "action": "support" }, + "submissionId": "9e8d7c6b-5a4f-4e3d-8c2b-1a0f9e8d7c6b", + "fields": [ + { "name": "applicantPhone", "label": "Phone", "error": "Enter a 10-digit phone number" } + ] +} +``` + +| Field | Required | Notes | +|---|:--:|---| +| `formId`, `context` | Yes | The form's, as sent | +| `submissionId` | Yes | The submission this answers | +| `fields` | Yes, may be `[]` | `{ name, label, error }`: what to fix, in the user's language. **Empty** when the values were fine but the office could not take the form; `outcome` and `error` then say why | + +How a sent form ends, by what the answer carries: + +| `outcome.status` | `content` | Meaning | +|---|---|---| +| `requires_input` | text and a `form_error` with fields | Fix those fields and send again | +| `unavailable` | text and a `form_error` with no fields, plus `error` | Not filed. The form stays; `error.retryable` says if trying later can help | +| `answered` | text | Filed. The text names the ticket | ### 5.3 How the client renders each outcome @@ -370,7 +441,8 @@ through untouched. The client branches on exactly two things: -1. **`content[].type`** — `text` or `refusal` for styling; `form` to draw a form. +1. **`content[].type`** — `text` or `refusal` for styling; `form` to draw a + form; `form_error` to reopen a sent form. 2. **`error`** — present means the turn failed: show Retry when `error.retryable` is true, and keep the turn out of history. @@ -520,7 +592,7 @@ is what the API sends for `request/follow-up.json`. | `context.messageId` | Client `messageId` | | `context.version` | Omitted. It is a DSS response field | | `context.senderId` | API config (`EXPERIENCE_API_SENDER_ID`): the network participant this deployment serves. Always sent | -| `message.input` | `history` as sent, then `query` as the last `user` item. Each text becomes `[{ "type": "text", "text": … }]` | +| `message.input` | `history` as sent, then `query` as the last `user` item. Each text becomes `[{ "type": "text", "text": … }]`. A form in `content` is the last `user` item instead: `[{ "type": "form", formId, context, submissionId, fields }]`, unchanged | | `attributes.channel` | Client `channel`; when missing, API config (`EXPERIENCE_API_CHANNEL`) | | `attributes.sourceLanguage` / `targetLanguage` | Client `language.source` / `.target` | | `attributes.location.geometry.coordinates` | `[longitude, latitude]` — note the swap | @@ -563,6 +635,7 @@ Reshaping a terminal frame into `FinalAnswer`: | `content[].annotations[]` (`url_citation`) | `citations[]`: `sourceId`, `startIndex` → `start`, `endIndex` → `end`. `url`/`sourceName` dropped — they are on `sources` | | `content[]` `refusal` | `{ type: "refusal", text }` | | `content[]` `form` | `{ type: "form", formId, context, fields }`, unchanged. A form without `formId` or `context` fails the frame | +| `content[]` `form_error` | `{ type: "form_error", formId, context, submissionId, fields }`, unchanged. One without `formId`, `context` or `submissionId` fails the frame | | `sources[]` | `{ id, name, url }`. `kind` dropped | | `error` (`code`, `message`, `retryable`, `retryAfterSeconds`) | `error`, same fields | @@ -591,7 +664,7 @@ Sample DSS streams, one per outcome: `dss-stream/` in the parked fixtures 1. **The DSS's prose and code disagree in places.** The live code is what counts: `transactionId` is required, `messageId` optional, only `text` - content is accepted (no `image` yet), unknown fields are rejected, and an + and `form` content are accepted (no `image` yet), unknown fields are rejected, and an `unavailable` turn ends with `turn.failed`, not `turn.completed`. ADR-0004 and `orchestration/envelope.py` describe an older request shape — worth flagging to the DSS team. diff --git a/src/experience_api/chat/adapters/dss/mapping.py b/src/experience_api/chat/adapters/dss/mapping.py index eac2f71..aa1f7a5 100644 --- a/src/experience_api/chat/adapters/dss/mapping.py +++ b/src/experience_api/chat/adapters/dss/mapping.py @@ -18,9 +18,12 @@ DssEvent, DssFinished, DssStarted, + FieldError, FormBlock, + FormErrorBlock, FormField, FormOption, + FormSubmission, Message, Outcome, RefusalBlock, @@ -55,8 +58,8 @@ def to_dss_request( """The `/v1/turns` body for one turn, contract §7. `now` is the API's clock, not the browser's. The history goes first, then - the query as the last `user` message, so the DSS finds the question where - it looks for it. + the query, a question or a filled form, as the last `user` message, so the + DSS finds it where it looks for it. """ location = turn.location @@ -69,10 +72,7 @@ def to_dss_request( sender_id=sender_id, ), message=schemas.Message( - input=[ - _input(message) - for message in (*turn.history, Message("user", turn.query)) - ], + input=[*(_input(message) for message in turn.history), _query(turn.query)], attributes=schemas.Attributes( channel=turn.channel, source_language=turn.source_language, @@ -100,6 +100,25 @@ def _input(message: Message) -> schemas.InputMessage: ) +def _query(query: str | FormSubmission) -> schemas.InputMessage: + if isinstance(query, str): + return _input(Message("user", query)) + return schemas.InputMessage( + role="user", + content=[ + schemas.FormPart( + form_id=query.form_id, + context=dict(query.context), + submission_id=query.submission_id, + fields=[ + schemas.SubmittedField(name=v.name, value=v.value) + for v in query.values + ], + ) + ], + ) + + # The DSS's event names (its `adapters/http/v1/sse.py`). _CREATED = "turn.created" _DELTA = "claim.delta" @@ -182,6 +201,8 @@ def _answer(event: str, message: schemas.ResponseMessage) -> Answer: def _block(item: schemas.ContentItem) -> Block | None: if item.type == "form": return _form(item) + if item.type == "form_error": + return _form_error(item) if item.text is None: return None if item.type == "refusal": @@ -206,6 +227,8 @@ def _form(item: schemas.ContentItem) -> FormBlock: # Without its id and context the client cannot send the form back. if item.form_id is None or item.context is None: raise DssProtocolError("malformed form item: formId or context missing") + if not all(isinstance(f, schemas.FormField) for f in item.fields): + raise DssProtocolError("malformed form item: a field has no kind") return FormBlock( form_id=item.form_id, context=item.context, @@ -223,5 +246,26 @@ def _form(item: schemas.ContentItem) -> FormBlock: ), ) for f in item.fields + if isinstance(f, schemas.FormField) + ), + ) + + +def _form_error(item: schemas.ContentItem) -> FormErrorBlock: + # Without these the client cannot tell which form, or which send, it is. + if item.form_id is None or item.context is None or item.submission_id is None: + raise DssProtocolError( + "malformed form_error item: formId, context or submissionId missing" + ) + if not all(isinstance(f, schemas.FieldError) for f in item.fields): + raise DssProtocolError("malformed form_error item: a field has no error") + return FormErrorBlock( + form_id=item.form_id, + context=item.context, + submission_id=item.submission_id, + fields=tuple( + FieldError(name=f.name, label=f.label, error=f.error) + for f in item.fields + if isinstance(f, schemas.FieldError) ), ) diff --git a/src/experience_api/chat/adapters/dss/schemas.py b/src/experience_api/chat/adapters/dss/schemas.py index 5bc110b..5950084 100644 --- a/src/experience_api/chat/adapters/dss/schemas.py +++ b/src/experience_api/chat/adapters/dss/schemas.py @@ -42,9 +42,24 @@ class TextPart(_Request): text: str +class SubmittedField(_Request): + name: str + value: str + + +class FormPart(_Request): + """A filled form, sent as the current user message (DSS #205).""" + + type: Literal["form"] = "form" + form_id: str + context: dict[str, str] + submission_id: str + fields: list[SubmittedField] + + class InputMessage(_Request): role: Literal["user", "assistant"] - content: list[TextPart] + content: list[TextPart | FormPart] class Geometry(_Request): @@ -123,15 +138,24 @@ class FormField(_Response): options: list[FormOption] = [] +class FieldError(_Response): + name: str + label: str + error: str + + class ContentItem(_Response): - """One item of any type. `form_id`, `context` and `fields` are a `form`'s.""" + """One item of any type. `form_id`, `context` and `fields` are a `form`'s or + a `form_error`'s; `submission_id` is a `form_error`'s. Each reads `fields` + in its own shape, so both are accepted here.""" type: str text: str | None = None annotations: list[Annotation] = [] form_id: str | None = None context: dict[str, str] | None = None - fields: list[FormField] = [] + submission_id: str | None = None + fields: list[FormField | FieldError] = [] class Outcome(_Response): diff --git a/src/experience_api/chat/adapters/http/mapping.py b/src/experience_api/chat/adapters/http/mapping.py index ccb4f68..bbed72c 100644 --- a/src/experience_api/chat/adapters/http/mapping.py +++ b/src/experience_api/chat/adapters/http/mapping.py @@ -13,7 +13,10 @@ ChatTurn, Completed, Delta, + FieldValue, FormBlock, + FormErrorBlock, + FormSubmission, Location, Message, RefusalBlock, @@ -27,7 +30,7 @@ def to_chat_turn(body: schemas.ChatRequest, *, default_channel: Channel) -> Chat return ChatTurn( session_id=body.session_id, message_id=body.message_id, - query=body.query, + query=_query(body), history=tuple(Message(item.role, item.text) for item in body.history), source_language=body.language.source, target_language=body.language.target, @@ -40,6 +43,19 @@ def to_chat_turn(body: schemas.ChatRequest, *, default_channel: Channel) -> Chat ) +def _query(body: schemas.ChatRequest) -> str | FormSubmission: + if body.query is not None: + return body.query + # The schema holds exactly one of the two, and one form at most. + (form,) = body.content or [] + return FormSubmission( + form_id=form.form_id, + context=form.context, + submission_id=form.submission_id, + values=tuple(FieldValue(f.name, f.value) for f in form.fields), + ) + + def to_final_answer(ids: TurnIds, answer: Answer) -> schemas.FinalAnswer: error = answer.error return schemas.FinalAnswer( @@ -69,11 +85,18 @@ def to_final_answer(ids: TurnIds, answer: Answer) -> schemas.FinalAnswer: def _block( block: Block, -) -> schemas.TextContent | schemas.RefusalContent | schemas.FormContent: +) -> ( + schemas.TextContent + | schemas.RefusalContent + | schemas.FormContent + | schemas.FormErrorContent +): if isinstance(block, RefusalBlock): return schemas.RefusalContent(text=block.text) if isinstance(block, FormBlock): return _form(block) + if isinstance(block, FormErrorBlock): + return _form_error(block) return schemas.TextContent( text=block.text, citations=[ @@ -108,6 +131,18 @@ def _form(block: FormBlock) -> schemas.FormContent: ) +def _form_error(block: FormErrorBlock) -> schemas.FormErrorContent: + return schemas.FormErrorContent( + form_id=block.form_id, + context=dict(block.context), + submission_id=block.submission_id, + fields=[ + schemas.FormFieldError(name=f.name, label=f.label, error=f.error) + for f in block.fields + ], + ) + + def to_wire_event(event: ChatEvent) -> tuple[str, BaseModel]: """The event's name on the stream, and its data.""" diff --git a/src/experience_api/chat/adapters/http/schemas.py b/src/experience_api/chat/adapters/http/schemas.py index 1890a0b..3813425 100644 --- a/src/experience_api/chat/adapters/http/schemas.py +++ b/src/experience_api/chat/adapters/http/schemas.py @@ -10,7 +10,7 @@ from __future__ import annotations -from typing import Annotated, Any, Literal +from typing import Annotated, Any, Literal, Self from pydantic import ( AfterValidator, @@ -19,6 +19,7 @@ Field, SerializerFunctionWrapHandler, model_serializer, + model_validator, ) from pydantic.alias_generators import to_camel @@ -71,10 +72,32 @@ class Location(_WireIn): longitude: float = Field(ge=-180, le=180, strict=True, allow_inf_nan=False) +class SubmittedField(_WireIn): + name: Text + # May be empty: an optional field left blank. The DSS checks the rules. + value: str = Field(strict=True) + + +class FormSubmission(_WireIn): + """A filled form sent back, contract §4.2. Only its shape is checked here.""" + + type: Literal["form"] + form_id: Text + # Opaque: sent back exactly as the `form` item gave it. + context: dict[str, Annotated[str, Field(strict=True)]] + submission_id: Uuid + fields: list[SubmittedField] + + class ChatRequest(_WireIn): session_id: Uuid message_id: Uuid - query: Text + # Exactly one of `query` and `content` (contract §4): the DSS takes one + # kind of message per turn. + query: Text | None = None + content: ( + Annotated[list[FormSubmission], Field(min_length=1, max_length=1)] | None + ) = None history: list[HistoryItem] language: Language location: Location | None = None @@ -82,6 +105,12 @@ class ChatRequest(_WireIn): # (contract §4). The mapping fills it in. channel: Channel | None = None + @model_validator(mode="after") + def _one_message(self) -> Self: + if (self.query is None) == (self.content is None): + raise ValueError("send exactly one of query and content") + return self + # --------------------------------------------------------------------------- # Outbound, contract §5.2. Built by field name, sent as camelCase. @@ -144,6 +173,21 @@ class FormContent(_WireOut): fields: list[FormField] +class FormFieldError(_WireOut): + name: str + label: str + error: str + + +class FormErrorContent(_WireOut): + type: Literal["form_error"] = "form_error" + form_id: str + context: dict[str, str] + submission_id: str + # Kept when empty: `[]` means the office could not take the form. + fields: list[FormFieldError] + + class Source(_WireOut): id: str name: str @@ -175,7 +219,7 @@ class FinalAnswer(_WireOut): assistant_message_id: str trace_id: str outcome: Outcome - content: list[TextContent | RefusalContent | FormContent] + content: list[TextContent | RefusalContent | FormContent | FormErrorContent] sources: list[Source] error: Error | None = None diff --git a/src/experience_api/chat/domain.py b/src/experience_api/chat/domain.py index c976767..994f1ac 100644 --- a/src/experience_api/chat/domain.py +++ b/src/experience_api/chat/domain.py @@ -30,13 +30,36 @@ class Location: longitude: float +@dataclass(frozen=True) +class FieldValue: + """What the farmer put in one field. Empty when they left it blank.""" + + name: str + value: str + + +@dataclass(frozen=True) +class FormSubmission: + """A filled form the farmer sent back. `context` goes back exactly as the + DSS sent it, so the DSS can find the form again. `submission_id` is new for + every send.""" + + form_id: str + context: Mapping[str, str] + submission_id: str + values: tuple[FieldValue, ...] + + @dataclass(frozen=True) class ChatTurn: - """One user message and the conversation before it, as the client sent it.""" + """One user message and the conversation before it, as the client sent it. + + The message is `query`: a typed question, or a filled form. + """ session_id: str message_id: str - query: str + query: str | FormSubmission history: tuple[Message, ...] source_language: str target_language: str @@ -99,7 +122,27 @@ class FormBlock: fields: tuple[FormField, ...] -Block = TextBlock | RefusalBlock | FormBlock +@dataclass(frozen=True) +class FieldError: + """What is wrong with one field, in the user's language.""" + + name: str + label: str + error: str + + +@dataclass(frozen=True) +class FormErrorBlock: + """A sent form the DSS handed back: it is still the farmer's. `fields` names + what to fix, and is empty when the office could not take the form.""" + + form_id: str + context: Mapping[str, str] + submission_id: str + fields: tuple[FieldError, ...] + + +Block = TextBlock | RefusalBlock | FormBlock | FormErrorBlock @dataclass(frozen=True) diff --git a/tests/api/test_chat_validation.py b/tests/api/test_chat_validation.py index b5c89c8..a2c4d88 100644 --- a/tests/api/test_chat_validation.py +++ b/tests/api/test_chat_validation.py @@ -10,7 +10,7 @@ from experience_api.app import create_app from experience_api.chat.adapters.dss.fake import FakeDssClient from experience_api.chat.domain import ChatTurn, DssEvent -from tests.support.examples import FOLLOW_UP +from tests.support.examples import FOLLOW_UP, FORM_SENT, FORM_SENT_TURN class _SpyDss(FakeDssClient): @@ -74,3 +74,23 @@ async def test_a_web_channel_reaches_the_dss( assert response.status_code == 200 assert [turn.channel for turn in spy.turns] == ["web"] + + +async def test_a_sent_form_reaches_the_dss( + running: httpx.AsyncClient, spy: _SpyDss +) -> None: + response = await running.post("/v1/chat", json=FORM_SENT) + + assert response.status_code == 200 + assert [turn.query for turn in spy.turns] == [FORM_SENT_TURN.query] + + +async def test_a_query_beside_a_form_is_a_422_and_never_reaches_the_dss( + running: httpx.AsyncClient, spy: _SpyDss +) -> None: + response = await running.post( + "/v1/chat", json=FORM_SENT | {"query": "And what about tomorrow?"} + ) + + assert response.status_code == 422 + assert spy.turns == [] diff --git a/tests/support/examples.py b/tests/support/examples.py index 8588fd9..306392b 100644 --- a/tests/support/examples.py +++ b/tests/support/examples.py @@ -3,7 +3,14 @@ from typing import Any -from experience_api.chat.domain import Channel, ChatTurn, Location, Message +from experience_api.chat.domain import ( + Channel, + ChatTurn, + FieldValue, + FormSubmission, + Location, + Message, +) # Contract §4, the follow-up request. FOLLOW_UP: dict[str, Any] = { @@ -129,3 +136,44 @@ "userContext": [{"type": "identity", "userId": "anonymous"}], }, } + +# Contract §4.2: a filled form sent back, in place of `query`. +FORM_CONTEXT = {"scheme": "PMFBY", "category": "Grievance", "action": "support"} +FORM_SENT: dict[str, Any] = { + "sessionId": "68a3872f-3f0d-4cf6-99a3-a350132a0080", + "messageId": "5b0e1c2a-7d4f-4a8e-9c3b-1f2e3d4c5b6a", + "content": [ + { + "type": "form", + "formId": "f-1", + "context": FORM_CONTEXT, + "submissionId": "9e8d7c6b-5a4f-4e3d-8c2b-1a0f9e8d7c6b", + "fields": [ + {"name": "applicantPhone", "value": "9876543210"}, + {"name": "season", "value": ""}, + ], + } + ], + "history": [], + "language": {"source": "en", "target": "en"}, +} + +# The same request as a turn. +FORM_SENT_TURN = ChatTurn( + session_id="68a3872f-3f0d-4cf6-99a3-a350132a0080", + message_id="5b0e1c2a-7d4f-4a8e-9c3b-1f2e3d4c5b6a", + query=FormSubmission( + form_id="f-1", + context=FORM_CONTEXT, + submission_id="9e8d7c6b-5a4f-4e3d-8c2b-1a0f9e8d7c6b", + values=( + FieldValue("applicantPhone", "9876543210"), + FieldValue("season", ""), + ), + ), + history=(), + source_language="en", + target_language="en", + location=None, + channel=Channel.WEB, +) diff --git a/tests/unit/chat/test_dss_frames.py b/tests/unit/chat/test_dss_frames.py index 36efaf9..483d0e5 100644 --- a/tests/unit/chat/test_dss_frames.py +++ b/tests/unit/chat/test_dss_frames.py @@ -17,7 +17,9 @@ DssEvent, DssFinished, DssStarted, + FieldError, FormBlock, + FormErrorBlock, FormField, FormOption, Outcome, @@ -259,3 +261,138 @@ def test_a_terminal_frame_without_an_outcome_is_a_protocol_error() -> None: with pytest.raises(DssProtocolError, match="outcome"): to_dss_events("turn.completed", data) + + +_CONTEXT = {"scheme": "PMFBY", "category": "Grievance", "action": "support"} + + +def _form_error(fields: list[dict[str, str]], **overrides: object) -> dict: + return { + "type": "form_error", + "formId": "f-1", + "context": _CONTEXT, + "submissionId": "9e8d7c6b-5a4f-4e3d-8c2b-1a0f9e8d7c6b", + "fields": fields, + } | overrides + + +def test_a_form_error_comes_through_whole() -> None: + raw = frame( + "turn.completed", + 2, + { + "outcome": {"status": "requires_input", "cause": "invalid_input"}, + "content": [ + {"type": "text", "text": "Please check the phone number."}, + _form_error( + [ + { + "name": "applicantPhone", + "label": "Phone", + "error": "Enter a 10-digit phone number", + } + ] + ), + ], + "sources": [], + }, + ) + (finished,) = _events(raw) + + assert isinstance(finished, DssFinished) + assert finished.answer.content == ( + TextBlock("Please check the phone number."), + FormErrorBlock( + form_id="f-1", + context=_CONTEXT, + submission_id="9e8d7c6b-5a4f-4e3d-8c2b-1a0f9e8d7c6b", + fields=( + FieldError( + name="applicantPhone", + label="Phone", + error="Enter a 10-digit phone number", + ), + ), + ), + ) + + +def test_a_form_the_office_could_not_take_comes_back_with_no_fields() -> None: + raw = frame( + "turn.failed", + 2, + { + "outcome": {"status": "unavailable", "cause": "provider_rejected"}, + "content": [ + {"type": "text", "text": "Not filed. Please try again later."}, + _form_error([]), + ], + "sources": [], + "error": { + "code": "provider_rejected", + "message": "The desk refused the filing", + "retryable": False, + }, + }, + ) + (finished,) = _events(raw) + + assert isinstance(finished, DssFinished) + form_error = finished.answer.content[1] + assert isinstance(form_error, FormErrorBlock) + assert form_error.fields == () + assert finished.answer.error is not None + assert finished.answer.error.retryable is False + + +@pytest.mark.parametrize("missing", ["formId", "context", "submissionId"]) +def test_a_form_error_without_its_ids_is_a_protocol_error(missing: str) -> None: + item = {k: v for k, v in _form_error([]).items() if k != missing} + raw = frame( + "turn.completed", + 2, + { + "outcome": {"status": "requires_input", "cause": "invalid_input"}, + "content": [item], + "sources": [], + }, + ) + data = raw.decode().split("data: ", 1)[1].strip() + + with pytest.raises(DssProtocolError, match="form_error"): + to_dss_events("turn.completed", data) + + +@pytest.mark.parametrize( + ("item", "match"), + [ + ( + { + "type": "form", + "formId": "f-1", + "context": _CONTEXT, + "fields": [{"name": "a", "label": "A", "error": "Bad"}], + }, + "form item", + ), + ( + _form_error([{"name": "a", "label": "A", "kind": "text"}]), + "form_error item", + ), + ], + ids=["a form field with no kind", "a form_error field with no error"], +) +def test_a_field_in_the_wrong_shape_is_a_protocol_error(item: dict, match: str) -> None: + raw = frame( + "turn.completed", + 2, + { + "outcome": {"status": "requires_input", "cause": None}, + "content": [item], + "sources": [], + }, + ) + data = raw.decode().split("data: ", 1)[1].strip() + + with pytest.raises(DssProtocolError, match=match): + to_dss_events("turn.completed", data) diff --git a/tests/unit/chat/test_dss_request.py b/tests/unit/chat/test_dss_request.py index 8525bd6..7a002f0 100644 --- a/tests/unit/chat/test_dss_request.py +++ b/tests/unit/chat/test_dss_request.py @@ -4,8 +4,14 @@ from datetime import UTC, datetime from experience_api.chat.adapters.dss.mapping import to_dss_request -from experience_api.chat.domain import Channel -from tests.support.examples import DSS_REQUEST, FOLLOW_UP_TURN, SENDER_ID +from experience_api.chat.domain import Channel, Message +from tests.support.examples import ( + DSS_REQUEST, + FOLLOW_UP_TURN, + FORM_CONTEXT, + FORM_SENT_TURN, + SENDER_ID, +) TURN = FOLLOW_UP_TURN NOW = datetime(2026, 9, 23, 8, 0, tzinfo=UTC) @@ -51,3 +57,43 @@ def test_the_channel_comes_from_the_turn() -> None: attributes = _request(replace(TURN, channel=Channel.WEB))["message"]["attributes"] assert attributes["channel"] == "web" + + +def test_a_sent_form_is_the_last_user_message() -> None: + turn = replace( + FORM_SENT_TURN, + history=( + Message("user", "I want to file a PMFBY grievance"), + Message("assistant", "Please fill in this form."), + ), + ) + + inputs = _request(turn)["message"]["input"] + + assert inputs[:2] == [ + { + "role": "user", + "content": [{"type": "text", "text": "I want to file a PMFBY grievance"}], + }, + { + "role": "assistant", + "content": [{"type": "text", "text": "Please fill in this form."}], + }, + ] + assert inputs[2] == { + "role": "user", + "content": [ + { + "type": "form", + "formId": "f-1", + "context": FORM_CONTEXT, + "submissionId": "9e8d7c6b-5a4f-4e3d-8c2b-1a0f9e8d7c6b", + "fields": [ + {"name": "applicantPhone", "value": "9876543210"}, + # An optional field left blank still goes: the DSS reads + # blank as not given. + {"name": "season", "value": ""}, + ], + } + ], + } diff --git a/tests/unit/chat/test_final_answer.py b/tests/unit/chat/test_final_answer.py index c886201..725b312 100644 --- a/tests/unit/chat/test_final_answer.py +++ b/tests/unit/chat/test_final_answer.py @@ -5,7 +5,9 @@ from experience_api.chat.domain import ( Answer, Citation, + FieldError, FormBlock, + FormErrorBlock, FormField, FormOption, Outcome, @@ -140,3 +142,47 @@ def test_a_form_goes_out_in_camel_case_with_optional_rules_left_out() -> None: ], } ] + + +def test_a_form_error_goes_out_in_camel_case() -> None: + answer = Answer( + outcome=Outcome(status="requires_input", cause="invalid_input"), + content=( + FormErrorBlock( + form_id="f-1", + context={"scheme": "PMFBY"}, + submission_id="9e8d7c6b-5a4f-4e3d-8c2b-1a0f9e8d7c6b", + fields=(FieldError(name="applicantPhone", label="Phone", error="Bad"),), + ), + ), + sources=(), + ) + + body = wire(to_final_answer(IDS, answer)) + + assert body["content"] == [ + { + "type": "form_error", + "formId": "f-1", + "context": {"scheme": "PMFBY"}, + "submissionId": "9e8d7c6b-5a4f-4e3d-8c2b-1a0f9e8d7c6b", + "fields": [{"name": "applicantPhone", "label": "Phone", "error": "Bad"}], + } + ] + + +def test_a_form_error_with_no_fields_keeps_the_empty_list() -> None: + # The client reads an empty list as "the office could not take it"; a + # missing key would be a different, broken answer. + answer = Answer( + outcome=Outcome(status="unavailable", cause="provider_unavailable"), + content=( + FormErrorBlock(form_id="f-1", context={}, submission_id="s-1", fields=()), + ), + sources=(), + error=TurnError(code="provider_unavailable", message="m", retryable=True), + ) + + body = wire(to_final_answer(IDS, answer)) + + assert body["content"][0]["fields"] == [] diff --git a/tests/unit/chat/test_http_mapping.py b/tests/unit/chat/test_http_mapping.py index 67904b4..89127db 100644 --- a/tests/unit/chat/test_http_mapping.py +++ b/tests/unit/chat/test_http_mapping.py @@ -6,7 +6,7 @@ from experience_api.chat.adapters.http.schemas import ChatRequest from experience_api.chat.domain import Channel from tests.support.examples import FOLLOW_UP as EXAMPLE -from tests.support.examples import FOLLOW_UP_TURN +from tests.support.examples import FOLLOW_UP_TURN, FORM_SENT, FORM_SENT_TURN def test_the_contract_example_becomes_a_turn() -> None: @@ -54,3 +54,11 @@ def test_no_channel_takes_the_default() -> None: ) assert turn.channel is Channel.WEB + + +def test_a_sent_form_becomes_the_turns_query() -> None: + turn = to_chat_turn( + ChatRequest.model_validate(FORM_SENT), default_channel=Channel.WEB + ) + + assert turn == FORM_SENT_TURN diff --git a/tests/unit/chat/test_request_schema.py b/tests/unit/chat/test_request_schema.py index b03e509..f5137d4 100644 --- a/tests/unit/chat/test_request_schema.py +++ b/tests/unit/chat/test_request_schema.py @@ -12,6 +12,7 @@ from experience_api.chat.adapters.http.schemas import ChatRequest from tests.support.examples import FOLLOW_UP as EXAMPLE +from tests.support.examples import FORM_SENT def _with(path: str, value: Any) -> dict[str, Any]: @@ -144,3 +145,79 @@ def test_a_null_channel_is_none() -> None: def test_web_is_a_channel() -> None: assert ChatRequest.model_validate(_with("channel", "web")).channel == "web" + + +# --------------------------------------------------------------------------- +# A filled form sent back, contract §4.2. +# --------------------------------------------------------------------------- + + +def _form_with(path: str, value: Any) -> dict[str, Any]: + """The form example with one field of its form replaced, as `_with` does.""" + + body = copy.deepcopy(FORM_SENT) + *parents, last = f"content.0.{path}".split(".") + node: Any = body + for key in parents: + node = node[int(key)] if key.isdigit() else node[key] + if value is ...: + del node[int(last) if last.isdigit() else last] + else: + node[int(last) if last.isdigit() else last] = value + return body + + +def test_the_form_example_is_valid() -> None: + request = ChatRequest.model_validate(FORM_SENT) + + assert request.query is None + assert request.content is not None + assert request.content[0].fields[1].value == "" + + +@pytest.mark.parametrize( + "body", + [ + FORM_SENT | {"query": "And what about tomorrow?"}, + {k: v for k, v in FORM_SENT.items() if k != "content"}, + FORM_SENT | {"content": []}, + FORM_SENT | {"content": FORM_SENT["content"] * 2}, + FORM_SENT | {"content": None}, + _form_with("type", "text"), + _form_with("formId", ...), + _form_with("formId", " "), + _form_with("context", ...), + _form_with("context", {"scheme": 1}), + _form_with("submissionId", ...), + _form_with("submissionId", "s-1"), + _form_with("fields", ...), + _form_with("fields.0.name", ""), + _form_with("fields.0.value", ...), + _form_with("fields.0.value", 9876543210), + _form_with("fields.0.label", "Phone"), + _form_with("extra", True), + ], + ids=[ + "query and content both", + "neither query nor content", + "content empty", + "two forms", + "content null", + "item not a form", + "formId missing", + "formId blank", + "context missing", + "context value not a string", + "submissionId missing", + "submissionId not a UUID", + "fields missing", + "field name empty", + "field value missing", + "field value a number", + "unknown field in a field", + "unknown field in the form", + ], +) +def test_form_rejected(body: dict[str, Any]) -> None: + with pytest.raises(ValidationError): + ChatRequest.model_validate(body) diff --git a/vulture_whitelist.py b/vulture_whitelist.py index e780f93..5e077fe 100644 --- a/vulture_whitelist.py +++ b/vulture_whitelist.py @@ -20,3 +20,6 @@ # An autouse pytest fixture: pytest calls it, our code never does. _._settings_from_the_test_only + +# A pydantic model validator: pydantic calls it on every ChatRequest. +_._one_message From 425015638c98ccb297c18c6a1be0fee2fadef0bc Mon Sep 17 00:00:00 2001 From: tarannum-2002 Date: Thu, 29 Oct 2026 15:12:46 +0530 Subject: [PATCH 2/2] refactor: always call the DSS; keep the fake for tests only - The fake DSS was the default, so a sent form was never filed and nothing said so. - `EXPERIENCE_API_DSS_MODE` is gone; tests pass the fake to `create_app(dss=...)`. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 10 +++--- CHANGELOG.md | 3 ++ README.md | 17 ++++------ docker-compose.yml | 4 +-- docs/ADR/0001-architecture.md | 18 +++++------ docs/api-contracts/api-contract.md | 4 +-- docs/spec/chat.md | 3 +- src/experience_api/app.py | 3 -- src/experience_api/settings.py | 6 ++-- tests/api/test_chat_json.py | 2 +- tests/api/test_chat_sse.py | 2 +- tests/api/test_chat_validation.py | 2 +- tests/integration/test_app_with_http_dss.py | 1 - .../dss/fake.py => tests/support/fake_dss.py | 7 ++-- tests/unit/chat/test_fake_dss.py | 32 ------------------- tests/unit/chat/test_service.py | 2 +- tests/unit/test_settings.py | 25 ++++----------- 17 files changed, 43 insertions(+), 98 deletions(-) rename src/experience_api/chat/adapters/dss/fake.py => tests/support/fake_dss.py (87%) delete mode 100644 tests/unit/chat/test_fake_dss.py diff --git a/AGENTS.md b/AGENTS.md index 246a747..2ae0aae 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -41,8 +41,8 @@ The full shapes live in `chat/domain.py`. The words to know: - `GET /healthz`: the process is up. It does not probe the DSS. ## What this service depends on -- **DSS**, `POST /v1/turns` over SSE, at `EXPERIENCE_API_DSS_BASE_URL`. - `EXPERIENCE_API_DSS_MODE=fake` (the default) answers from memory instead. +- **DSS**, `POST /v1/turns` over SSE, at `EXPERIENCE_API_DSS_BASE_URL`. Always + the real one: the fake DSS lives in `tests/support/` and only tests use it. - No database, no messaging. ## Conventions @@ -68,7 +68,7 @@ src/experience_api/ ├── ports.py # Protocols for what the feature needs from outside └── adapters/ ├── http/ # inbound: routes, wire schemas (camelCase), mapping - └── / # outbound: implements a port (dss/ today, with a fake) + └── / # outbound: implements a port (dss/ today) ``` ## Testing @@ -76,8 +76,8 @@ TDD: write the failing test first. Every commit is green. | Tier | Path | What | Uses | |---|---|---|---| -| Unit | `tests/unit//` | mapping, the service, SSE framing | the fake DSS; fixed clock and ids | -| API | `tests/api/` | the whole app through httpx's ASGI transport | the fake DSS | +| Unit | `tests/unit//` | mapping, the service, SSE framing | the fake DSS (`tests/support/fake_dss.py`); fixed clock and ids | +| API | `tests/api/` | the whole app through httpx's ASGI transport | the fake DSS, passed to `create_app(dss=...)` | | Integration | `tests/integration/` | `HttpDssClient` against a local HTTP server | pytest-httpserver | | E2E | `tests/e2e/` | one turn against a real DSS | marker `e2e`, not run by default | | Boundaries | `tests/test_boundaries.py` | the dependency rule | the AST | diff --git a/CHANGELOG.md b/CHANGELOG.md index 9cae86c..ae5336b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ All notable changes to this project are recorded here, in the format of ## [Unreleased] +### Removed +- `EXPERIENCE_API_DSS_MODE` and the fake DSS mode. The API always calls the DSS; the fake is now a test double only. + ### Added - A filled form sent back as `content` in place of `query`, passed to the DSS; a `form_error` item in the answer hands it back. - A `form` item in the answer: a form the DSS offers reaches the client instead of being dropped. diff --git a/README.md b/README.md index d575835..f500aa2 100644 --- a/README.md +++ b/README.md @@ -25,24 +25,19 @@ The API listens on `http://localhost:8078`. | `GET /healthz` | `{"status": "ok"}` when the process is up. For Docker and the proxy | | `GET /docs` | The interactive OpenAPI page | -By default a fake DSS answers every turn, so the API runs with nothing else -installed. To call a real one: - -```bash -EXPERIENCE_API_DSS_MODE=http uv run uvicorn --factory experience_api.app:create_app --port 8078 -``` +Every turn goes to the DSS at `EXPERIENCE_API_DSS_BASE_URL`, so run a DSS +first. There is no stand-in: with no DSS, a turn fails. | Variable | Default | What | |---|---|---| | `EXPERIENCE_API_SENDER_ID` | none, required | The network participant this deployment serves. Sent to the DSS as `senderId` | -| `EXPERIENCE_API_DSS_MODE` | `fake` | `fake`, or `http` to call the DSS | | `EXPERIENCE_API_DSS_BASE_URL` | `http://localhost:8077` | Where the DSS listens | | `EXPERIENCE_API_CHANNEL` | `web` | The channel for a chat whose client sends none | | `EXPERIENCE_API_MAX_CHARACTERS` | `1200` | The longest answer to ask the DSS for | Each can also be set in `.env`; a real environment variable wins over it. -A missing sender id, or a bad mode, channel or length, stops the API from +A missing sender id, or a bad channel or length, stops the API from starting; a wrong DSS URL fails the first turn, like a DSS that is down. Errors from the DSS itself (it being down, busy, or refusing the request) come back as a plain `500` for now. @@ -60,9 +55,9 @@ docker compose up --build # add -d to run it in the background docker compose down ``` -Same port, same fake DSS, and the sender id from `.env`. `EXPERIENCE_API_DSS_MODE=http docker compose up ---build` calls a DSS on the host's port 8077; the other variables pass through -too. Set `EXPERIENCE_API_HOST_PORT` to publish on another port. The container +Same port, and the sender id from `.env`. It calls a DSS on the host's port +8077; set `EXPERIENCE_API_DSS_BASE_URL` for another. The other variables pass +through too. Set `EXPERIENCE_API_HOST_PORT` to publish on another port. The container reports healthy once `/healthz` answers. ### The published image diff --git a/docker-compose.yml b/docker-compose.yml index 19f16bd..3e5a12c 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -1,6 +1,5 @@ # Run: docker compose up --build -# The API on http://localhost:8078, answered by the fake DSS. For a real one: -# EXPERIENCE_API_DSS_MODE=http docker compose up --build +# The API on http://localhost:8078, calling the DSS at EXPERIENCE_API_DSS_BASE_URL. services: experience-api: build: @@ -9,7 +8,6 @@ services: environment: # No default: Compose reads it from `.env` (copy `.env.example`). EXPERIENCE_API_SENDER_ID: ${EXPERIENCE_API_SENDER_ID} - EXPERIENCE_API_DSS_MODE: ${EXPERIENCE_API_DSS_MODE:-fake} # Default: a DSS on this machine's port 8077, reached from the container. EXPERIENCE_API_DSS_BASE_URL: ${EXPERIENCE_API_DSS_BASE_URL:-http://host.docker.internal:8077} EXPERIENCE_API_CHANNEL: ${EXPERIENCE_API_CHANNEL:-web} diff --git a/docs/ADR/0001-architecture.md b/docs/ADR/0001-architecture.md index 33ed558..5d8196e 100644 --- a/docs/ADR/0001-architecture.md +++ b/docs/ADR/0001-architecture.md @@ -37,7 +37,7 @@ HTTP, JSON or the services it calls. | Service | `chat/service.py` — `ChatService` | the feature's rules | | Domain | `chat/domain.py` | plain types and errors | | Port | `chat/ports.py` — `DssClient` | what the feature needs from outside, in its own terms | -| Outbound adapter | `chat/adapters/dss/` | the HTTP client and a fake, each fulfilling the port | +| Outbound adapter | `chat/adapters/dss/` | the HTTP client, fulfilling the port. A fake fulfils it in tests *(amended)* | The service does not know it is called over HTTP. @@ -66,7 +66,7 @@ experience-api/ ├── pyproject.toml # uv; fastapi, uvicorn, httpx, pydantic-settings ├── uv.lock ├── Dockerfile -├── docker-compose.yml # the API, in fake-DSS mode by default (§7) +├── docker-compose.yml # the API, calling the DSS ├── README.md ├── CLAUDE.md # layout, stack, test tiers ├── CONVENTIONS.md # naming, commits, PRs @@ -98,7 +98,6 @@ experience-api/ │ │ └── errors.py # exception handlers: chat errors → code + status (§6.1) │ └── dss/ # OUTBOUND — contract §7; implements ports.DssClient │ ├── client.py # HttpDssClient: httpx stream to POST /v1/turns -│ ├── fake.py # FakeDssClient: canned scenarios (§7) │ ├── schemas.py # the DSS wire shapes we send and read │ └── mapping.py # ChatTurn → DSS request; DSS frames → domain events │ @@ -372,22 +371,22 @@ A feature with many settings can later get its own nested settings class. | Env var | Default | Contract | |---|---|---| -| `EXPERIENCE_API_DSS_MODE` | `fake` | `http` or `fake` (§7) | | `EXPERIENCE_API_DSS_BASE_URL` | `http://localhost:8077` | where the DSS listens | | `EXPERIENCE_API_CHANNEL` | `web` | §7 `attributes.channel` | | `EXPERIENCE_API_MAX_CHARACTERS` | `1200` | §7 `attributes.response.maxCharacters` | | `EXPERIENCE_API_ACCEPT_TIMEOUT_SECONDS` | `30` | §6.3 — DSS headers and first event | | `EXPERIENCE_API_TURN_TIMEOUT_SECONDS` | `120` | §6.3 | -| `EXPERIENCE_API_FAKE_SCENARIO` | `answered` | which canned scenario the fake plays (§7) | -| `EXPERIENCE_API_FAKE_DELAY_SECONDS` | `0` | silence before the fake's first frame | - -The default is `fake` so the API runs with nothing else installed. A -deployment sets `http`. +*(Amended.)* There is no DSS mode: the running API always calls the DSS (§7). --- ## 7. The fake DSS +*(Amended.)* The fake is a test double only. It lives in +`tests/support/fake_dss.py`, and tests pass it to `create_app(dss=...)`. The +running API always calls the real DSS, so no reply a person sees can be a +canned one. Purposes 2 and 3 below, and the scenarios, no longer apply. + `FakeDssClient` yields canned `DssEvent`s, one scenario per outcome, written in `fake.py` itself. It serves three purposes: @@ -483,3 +482,4 @@ of the contract shows up at the seam. Each phase is one or two PRs. | Adapters may import only their feature's `domain` and `ports` | The inbound `adapters/http/` may also import its `service` | The route in §5.1 takes a `ChatService`; calling the service is an inbound adapter's job | | `DssClient` has `aclose()`, called by the lifespan | The port has only `stream_turn`. `HttpDssClient` is an async context manager, entered by the lifespan | Closing a pool is lifecycle, not something chat needs. Whoever builds a client closes it | | Timeouts in step 5 | After `HttpDssClient`, with httpx timeouts as the interim safety net | Not needed for the first cut | +| A fake DSS mode, the default, for running the API with no DSS (§6, §7) | No mode. The fake is a test double in `tests/support/`; the app always calls the DSS | A canned reply looked like a real one. A form sent to the default API was never filed, and nothing said so | diff --git a/docs/api-contracts/api-contract.md b/docs/api-contracts/api-contract.md index 8f57504..56ee629 100644 --- a/docs/api-contracts/api-contract.md +++ b/docs/api-contracts/api-contract.md @@ -26,8 +26,8 @@ Rules for working in parallel: - **§3–§6 are the contract.** Neither side changes its behaviour away from them. A change is made **here first**, then in code. -- The client builds against its own stubs, with no running API. The API builds - against a fake DSS, with no running client. +- Each side calls the real one: the client the API, the API the DSS. Neither + ships a stand-in *(amended)*. The API's tests use a fake DSS. --- diff --git a/docs/spec/chat.md b/docs/spec/chat.md index 69179c9..2ac46e2 100644 --- a/docs/spec/chat.md +++ b/docs/spec/chat.md @@ -32,7 +32,7 @@ Chat is lenient here. New DSS fields, events and content types are ignored, so a DSS release cannot break a turn. ### Without a DSS -With `DSS_MODE=fake`, chat plays one canned weather answer for every turn. +There is no stand-in. With no DSS, a turn fails. Only tests use the fake DSS. ## Key flows A streamed turn: client → chat → DSS, then `started`, `delta`…, `completed` @@ -55,7 +55,6 @@ can echo the user's words. ## Configuration and limits | Setting | Default | Notes | |---|---|---| -| `EXPERIENCE_API_DSS_MODE` | `fake` | `fake` or `http` | | `EXPERIENCE_API_DSS_BASE_URL` | `http://localhost:8077` | Checked on the first turn, not at start | | `EXPERIENCE_API_CHANNEL` | `web` | Sent to the DSS | | `EXPERIENCE_API_MAX_CHARACTERS` | `1200` | Sent to the DSS | diff --git a/src/experience_api/app.py b/src/experience_api/app.py index 69cc1e2..4674218 100644 --- a/src/experience_api/app.py +++ b/src/experience_api/app.py @@ -21,7 +21,6 @@ from fastapi import APIRouter, FastAPI from experience_api.chat.adapters.dss.client import HttpDssClient -from experience_api.chat.adapters.dss.fake import FakeDssClient from experience_api.chat.adapters.http.routes import router as chat_router from experience_api.chat.domain import Channel from experience_api.chat.ports import DssClient @@ -72,8 +71,6 @@ async def lifespan(app: FastAPI) -> AsyncIterator[None]: async def _dss_client(settings: Settings, stack: AsyncExitStack) -> DssClient: - if settings.dss_mode == "fake": - return FakeDssClient() return await stack.enter_async_context( HttpDssClient( base_url=settings.dss_base_url, diff --git a/src/experience_api/settings.py b/src/experience_api/settings.py index 0fb3635..2d0fd02 100644 --- a/src/experience_api/settings.py +++ b/src/experience_api/settings.py @@ -3,7 +3,7 @@ `.env` is for local runs; copy `.env.example`. A real environment variable always wins over it, and the image never carries one (`.dockerignore`). -Read only by `app.py`, which hands services plain values. A bad mode or length +Read only by `app.py`, which hands services plain values. A bad length stops the app from starting here; a bad channel stops it in `app.py`, which owns the channel enum. The DSS URL is not checked: a wrong one fails like an unreachable DSS, on the first turn. @@ -11,7 +11,7 @@ from __future__ import annotations -from typing import Annotated, Literal +from typing import Annotated from pydantic import Field, StringConstraints from pydantic_settings import BaseSettings, SettingsConfigDict @@ -23,8 +23,6 @@ class Settings(BaseSettings): ) sender_id: Annotated[str, StringConstraints(strip_whitespace=True, min_length=1)] - # `fake` answers every turn from memory; `http` calls the DSS. - dss_mode: Literal["fake", "http"] = "fake" dss_base_url: str = "http://localhost:8077" # The channel for a turn whose client sends none (contract §4, §7). A plain # string: `app.py` converts it to the chat `Channel` enum at startup. diff --git a/tests/api/test_chat_json.py b/tests/api/test_chat_json.py index 1b3c6fe..f08d0f8 100644 --- a/tests/api/test_chat_json.py +++ b/tests/api/test_chat_json.py @@ -9,8 +9,8 @@ from fastapi import FastAPI from experience_api.app import create_app -from experience_api.chat.adapters.dss.fake import FakeDssClient from tests.support.examples import ANSWERED, FAKE_ANSWERED, FOLLOW_UP +from tests.support.fake_dss import FakeDssClient @pytest.fixture diff --git a/tests/api/test_chat_sse.py b/tests/api/test_chat_sse.py index 8d44db8..f0ca81c 100644 --- a/tests/api/test_chat_sse.py +++ b/tests/api/test_chat_sse.py @@ -11,8 +11,8 @@ from fastapi import FastAPI from experience_api.app import create_app -from experience_api.chat.adapters.dss.fake import FakeDssClient from tests.support.examples import ANSWERED, FAKE_ANSWERED, FAKE_PIECES, FOLLOW_UP +from tests.support.fake_dss import FakeDssClient SSE = {"Accept": "text/event-stream"} diff --git a/tests/api/test_chat_validation.py b/tests/api/test_chat_validation.py index a2c4d88..d201681 100644 --- a/tests/api/test_chat_validation.py +++ b/tests/api/test_chat_validation.py @@ -8,9 +8,9 @@ from fastapi import FastAPI from experience_api.app import create_app -from experience_api.chat.adapters.dss.fake import FakeDssClient from experience_api.chat.domain import ChatTurn, DssEvent from tests.support.examples import FOLLOW_UP, FORM_SENT, FORM_SENT_TURN +from tests.support.fake_dss import FakeDssClient class _SpyDss(FakeDssClient): diff --git a/tests/integration/test_app_with_http_dss.py b/tests/integration/test_app_with_http_dss.py index f5d3499..738a21e 100644 --- a/tests/integration/test_app_with_http_dss.py +++ b/tests/integration/test_app_with_http_dss.py @@ -20,7 +20,6 @@ def app(httpserver: HTTPServer) -> FastAPI: return create_app( Settings( sender_id=SENDER_ID, - dss_mode="http", dss_base_url=httpserver.url_for(""), ) ) diff --git a/src/experience_api/chat/adapters/dss/fake.py b/tests/support/fake_dss.py similarity index 87% rename from src/experience_api/chat/adapters/dss/fake.py rename to tests/support/fake_dss.py index 4017a62..d4a4b22 100644 --- a/src/experience_api/chat/adapters/dss/fake.py +++ b/tests/support/fake_dss.py @@ -1,8 +1,7 @@ -"""A stand-in DSS that plays canned turns. +"""A stand-in DSS for tests. The running app always calls the real DSS. -It lets the API run with no DSS behind it: for tests, and for the web client to -try the real API. It ignores what the turn asks and plays the same answer every -time: a short weather answer shaped like the contract's example. +It ignores what the turn asks and plays the same answer every time: a short +weather answer shaped like the contract's example. """ from __future__ import annotations diff --git a/tests/unit/chat/test_fake_dss.py b/tests/unit/chat/test_fake_dss.py deleted file mode 100644 index 0b3c6d3..0000000 --- a/tests/unit/chat/test_fake_dss.py +++ /dev/null @@ -1,32 +0,0 @@ -"""The fake DSS answers every turn the same way: a few sentences, in pieces.""" - -from experience_api.chat.adapters.dss.fake import FakeDssClient -from experience_api.chat.domain import ( - Citation, - DssDelta, - DssFinished, - DssStarted, - TextBlock, -) -from experience_api.chat.ports import DssClient -from tests.support.examples import FAKE_PIECES, FAKE_TEXT, FOLLOW_UP_TURN - -TURN = FOLLOW_UP_TURN - - -async def test_starts_streams_the_pieces_then_finishes() -> None: - dss: DssClient = FakeDssClient(new_id=lambda: "assistant-1") - - events = [e async for e in dss.stream_turn(TURN, transaction_id="tx-1")] - - assert events[0] == DssStarted(assistant_message_id="assistant-1", trace_id="tx-1") - assert events[1:-1] == [DssDelta(piece) for piece in FAKE_PIECES] - finished = events[-1] - assert isinstance(finished, DssFinished) - assert finished.answer.outcome.status == "answered" - assert finished.answer.content == ( - TextBlock( - text=FAKE_TEXT, - citations=(Citation(source_id="src_1", start=0, end=len(FAKE_TEXT)),), - ), - ) diff --git a/tests/unit/chat/test_service.py b/tests/unit/chat/test_service.py index 40db292..ea90496 100644 --- a/tests/unit/chat/test_service.py +++ b/tests/unit/chat/test_service.py @@ -1,6 +1,5 @@ """`ChatService` opens a DSS turn and relays it as the client's events.""" -from experience_api.chat.adapters.dss.fake import FakeDssClient from experience_api.chat.domain import ( Completed, Delta, @@ -11,6 +10,7 @@ ) from experience_api.chat.service import ChatService from tests.support.examples import FAKE_PIECES, FOLLOW_UP_TURN +from tests.support.fake_dss import FakeDssClient TURN = FOLLOW_UP_TURN diff --git a/tests/unit/test_settings.py b/tests/unit/test_settings.py index 2fae4c8..48c4e9c 100644 --- a/tests/unit/test_settings.py +++ b/tests/unit/test_settings.py @@ -14,53 +14,42 @@ def _from_env() -> Settings: return Settings() # pyright: ignore[reportCallIssue] -def test_defaults_run_against_the_fake_dss(monkeypatch: pytest.MonkeyPatch) -> None: - for name in ("DSS_MODE", "DSS_BASE_URL", "CHANNEL", "MAX_CHARACTERS"): +def test_defaults_call_a_dss_on_this_machine(monkeypatch: pytest.MonkeyPatch) -> None: + for name in ("DSS_BASE_URL", "CHANNEL", "MAX_CHARACTERS"): monkeypatch.delenv(f"EXPERIENCE_API_{name}", raising=False) settings = _from_env() - assert settings.dss_mode == "fake" assert settings.dss_base_url == "http://localhost:8077" assert settings.channel == "web" assert settings.max_characters == 1200 def test_read_from_the_prefixed_environment(monkeypatch: pytest.MonkeyPatch) -> None: - monkeypatch.setenv("EXPERIENCE_API_DSS_MODE", "http") monkeypatch.setenv("EXPERIENCE_API_DSS_BASE_URL", "http://dss:8077") monkeypatch.setenv("EXPERIENCE_API_MAX_CHARACTERS", "600") settings = _from_env() - assert (settings.dss_mode, settings.dss_base_url, settings.max_characters) == ( - "http", + assert (settings.dss_base_url, settings.max_characters) == ( "http://dss:8077", 600, ) -@pytest.mark.parametrize( - ("name", "value"), - [("DSS_MODE", "real"), ("MAX_CHARACTERS", "0")], -) -def test_a_bad_value_stops_the_app_from_starting( - monkeypatch: pytest.MonkeyPatch, name: str, value: str +def test_a_bad_length_stops_the_app_from_starting( + monkeypatch: pytest.MonkeyPatch, ) -> None: - monkeypatch.setenv(f"EXPERIENCE_API_{name}", value) + monkeypatch.setenv("EXPERIENCE_API_MAX_CHARACTERS", "0") with pytest.raises(ValidationError): _from_env() -@pytest.mark.parametrize("mode", ["fake", "http"]) @pytest.mark.parametrize("value", [None, "", " "]) def test_a_missing_or_blank_sender_id_stops_the_app( - monkeypatch: pytest.MonkeyPatch, mode: str, value: str | None + monkeypatch: pytest.MonkeyPatch, value: str | None ) -> None: - # In every mode: the fake DSS ignores the sender id, but a deployment that - # forgets it must still not start. - monkeypatch.setenv("EXPERIENCE_API_DSS_MODE", mode) if value is None: monkeypatch.delenv("EXPERIENCE_API_SENDER_ID") else: