-
Notifications
You must be signed in to change notification settings - Fork 2
Testing architecture overhaul: real-DB fixtures, i18n fixes, and the production bugs they surfaced #13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Testing architecture overhaul: real-DB fixtures, i18n fixes, and the production bugs they surfaced #13
Changes from all commits
e909392
30ab9d9
c177b27
c48c744
c3f6089
7410364
c50b6a2
8fcbee4
71f4184
29f4bf7
bb9b0af
71a8877
67feeaa
c348f97
0f790bc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,7 @@ | |
| from rich.console import Console | ||
| from rich.prompt import Confirm, FloatPrompt, IntPrompt, Prompt | ||
| from rich.table import Table | ||
| from sqlalchemy.exc import SQLAlchemyError | ||
|
|
||
| from openfatture.cli.lifespan import get_event_bus | ||
| from openfatture.core.events import ( | ||
|
|
@@ -56,6 +57,19 @@ def crea_fattura( | |
|
|
||
| console.print(f"\n{_('cli-fattura-create-title')}\n") | ||
|
|
||
| try: | ||
| _crea_fattura_in_db(cliente_id) | ||
| except (SQLAlchemyError, ValueError) as exc: | ||
| # db_session() has already rolled back; convert a database/validation | ||
| # failure into a clean error message and non-zero exit instead of a raw | ||
| # traceback. typer.Exit raised inside (e.g. no clients) is not caught | ||
| # here and propagates unchanged. | ||
| console.print(_("cli-fattura-create-error", error=str(exc))) | ||
| raise typer.Exit(1) from exc | ||
|
Comment on lines
+60
to
+68
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't map post-commit failures to "invoice creation failed".
🤖 Prompt for AI Agents |
||
|
|
||
|
|
||
| def _crea_fattura_in_db(cliente_id: int | None) -> None: | ||
| """Run the invoice-creation transaction. Wrapped by ``crea_fattura``.""" | ||
| with db_session() as db: | ||
| # Select client | ||
| if not cliente_id: | ||
|
|
@@ -80,8 +94,16 @@ def crea_fattura( | |
|
|
||
| console.print(f"{_('cli-fattura-client-selected', client_name=cliente.denominazione)}\n") | ||
|
|
||
| # Invoice details | ||
| anno = date.today().year | ||
| # Invoice details. Derive the year from the entered issue date so the | ||
| # stored ``anno`` (used for FatturaPA numbering/progressivo) stays | ||
| # consistent with ``data_emissione`` even when the user back-dates or | ||
| # forward-dates the invoice into another year. | ||
| data_emissione_input = Prompt.ask( | ||
| "Issue date (YYYY-MM-DD)", default=date.today().isoformat() | ||
| ) | ||
|
Comment on lines
+101
to
+103
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use the translated issue-date prompt here. This new prompt is hard-coded in English, so localized CLI sessions will still see English at the first new step of the wizard. Reuse the existing translation key instead of a literal string. 🤖 Prompt for AI Agents |
||
| data_emissione = date.fromisoformat(data_emissione_input) | ||
| anno = data_emissione.year | ||
|
|
||
| ultimo_numero = ( | ||
| db.query(Fattura).filter(Fattura.anno == anno).order_by(Fattura.numero.desc()).first() | ||
| ) | ||
|
|
@@ -92,13 +114,12 @@ def crea_fattura( | |
| prossimo_numero = 1 | ||
|
|
||
| numero = Prompt.ask("Invoice number", default=str(prossimo_numero)) | ||
| data_emissione = Prompt.ask("Issue date (YYYY-MM-DD)", default=date.today().isoformat()) | ||
|
|
||
| # Create invoice | ||
| fattura = Fattura( | ||
| numero=numero, | ||
| anno=anno, | ||
| data_emissione=date.fromisoformat(data_emissione), | ||
| data_emissione=data_emissione, | ||
| cliente_id=cliente.id, | ||
| tipo_documento=TipoDocumento.TD01, | ||
| stato=StatoFattura.BOZZA, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,14 @@ | ||
| """Streaming JSON Lines formatter.""" | ||
|
|
||
| import json | ||
| from typing import Any | ||
| from typing import TYPE_CHECKING, Any | ||
|
|
||
| from openfatture.ai.domain.response import AgentResponse | ||
| from openfatture.cli.formatters.base import BaseFormatter | ||
|
|
||
| if TYPE_CHECKING: | ||
| from openfatture.ai.streaming.events import StreamEvent | ||
|
|
||
|
|
||
| class StreamJSONFormatter(BaseFormatter): | ||
| """Formatter that outputs responses as JSON Lines (JSONL) format. | ||
|
|
@@ -84,28 +87,66 @@ def format_response(self, response: AgentResponse) -> str: | |
|
|
||
| return "\n".join(lines) | ||
|
|
||
| def format_stream_chunk(self, chunk: str) -> str: | ||
| def format_stream_chunk(self, chunk: "str | StreamEvent") -> str: | ||
| """Format a streaming chunk as JSON Line. | ||
|
|
||
| Each chunk is formatted as a separate JSON object with: | ||
| - type: "chunk" | ||
| - content: the chunk text | ||
| - index: sequential chunk number | ||
|
|
||
| Accepts either a plain string (raw text chunk) or a | ||
| :class:`~openfatture.ai.streaming.events.StreamEvent`, which is what | ||
| streaming agents such as ``ChatAgent.execute_stream()`` actually yield. | ||
| For a ``StreamEvent`` the textual payload is extracted (``data`` for | ||
| content/string events, or its JSON form for structured events) so the | ||
| emitted JSON line is always serializable. | ||
|
|
||
| Args: | ||
| chunk: A chunk of response content | ||
| chunk: A chunk of response content (string or StreamEvent) | ||
|
|
||
| Returns: | ||
| JSONL formatted chunk with newline | ||
| """ | ||
| content = self._extract_chunk_content(chunk) | ||
| chunk_obj = { | ||
| "type": "chunk", | ||
| "content": chunk, | ||
| "content": content, | ||
| "index": self.chunk_index, | ||
| } | ||
| self.chunk_index += 1 | ||
| return json.dumps(chunk_obj, ensure_ascii=self.ensure_ascii) + "\n" | ||
|
|
||
| @staticmethod | ||
| def _extract_chunk_content(chunk: "str | StreamEvent") -> str: | ||
| """Normalize a stream chunk into a JSON-serializable string. | ||
|
|
||
| Streaming agents yield ``StreamEvent`` objects rather than raw strings. | ||
| A ``StreamEvent`` is not JSON-serializable on its own, so extract its | ||
| textual payload: the ``data`` field for string payloads (content, | ||
| thinking, status, ...) or a compact JSON encoding for structured | ||
| payloads (tool events, metrics, ...). | ||
|
|
||
| Args: | ||
| chunk: A raw string chunk or a StreamEvent. | ||
|
|
||
| Returns: | ||
| A plain string suitable for embedding in the JSON line. | ||
| """ | ||
| if isinstance(chunk, str): | ||
| return chunk | ||
|
|
||
| # Lazy import keeps the CLI startup free of the AI stack. | ||
| from openfatture.ai.streaming.events import StreamEvent | ||
|
|
||
| if isinstance(chunk, StreamEvent): | ||
| data = chunk.data | ||
| if isinstance(data, str): | ||
| return data | ||
| return json.dumps(data, ensure_ascii=False) | ||
|
Comment on lines
+142
to
+146
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Handle
Proposed fix if isinstance(chunk, StreamEvent):
data = chunk.data
if isinstance(data, str):
return data
- return json.dumps(data, ensure_ascii=False)
+ return json.dumps(data, ensure_ascii=False, default=str)As per coding guidelines "Use Decimal for all currency and amount values (never float)" and "Use datetime.date for invoice dates and datetime.datetime for timestamps". 🤖 Prompt for AI Agents |
||
|
|
||
| return str(chunk) | ||
|
|
||
| def format_stream_complete( | ||
| self, | ||
| status: str = "success", | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Scope this handler to the transactional failure path.
db.commit()completes before event publication and summary rendering, but thisexceptstill converts any laterValueError/SQLAlchemyErrorintocli-cliente-add-error. That can tell the user the save failed after the client is already persisted, which makes retrying dangerous, anderror=str(exc)also exposes raw DB details to the CLI. Catch the insert/commit failure separately, then handle post-commit publish/render errors without claiming the create failed.🤖 Prompt for AI Agents