Testing architecture overhaul: real-DB fixtures, i18n fixes, and the production bugs they surfaced#13
Merged
Conversation
The default pytest run mixed fast unit/integration tests with wall-clock performance asserts, external-service (Ollama) and e2e tests, making the gate slow and non-deterministic. Deselect the heavy markers by default (opt-in via `pytest -m performance` / `-m "ollama and e2e"`), register the `unit`/`integration` markers, fix two mismarked tests, and remove a legacy e2e script (init_db at import time, no assertions, real network calls). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Three production-facing i18n defects from the locale rollout:
1. Block values whose first content line starts with Rich '[markup]' were
parsed by Fluent as a standalone variant key, silently dropping the
message so callers rendered the raw key (e.g. 'cli-report-iva-title
(anno=2025)'). Inline those 24 entries/locale across all 5 locales.
2. FluentBundle ran with use_isolating=True, injecting U+2068/U+2069 bidi
isolation marks around every placeable; these corrupt Rich markup and CLI
output. Disable isolation.
3. Year placeables ({ $anno }/{ $year }) were formatted as quantities and
gained a thousands separator ('2025' -> '2,025'/'2.025'). Render year-like
variables verbatim while keeping count/months numeric for plural selectors.
Includes scripts/_fix_ftl_blank_values.py (idempotent, deterministic).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a runtime_db/runtime_session/seed_* fixture set that points an isolated, file-backed SQLite database at the global session factory AND get_settings() (via env), so CLI/web/agent code under test shares one database with the test's seeded data — replacing brittle patch(module.SessionLocal) + mocked SQLAlchemy query chains that broke when commands migrated to db_session(). Convert tests/cli/test_report_commands.py to seed real rows and assert real, locale-pinned (English) command output. 14/14 pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace patch(module.SessionLocal) + mocked SQLAlchemy query chains (broken since commands migrated to db_session()) with real rows seeded through the runtime_db fixtures, asserting real English (locale-pinned) command output and post-state. Error-injection tests mock the real db_session seam as a context manager. 52 tests, 0 skips. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Pin locale to English and assert real command output (the i18n rollout made output Italian); convert remaining SessionLocal/query-chain mocks to real runtime_db seeding. pec/lightning-compliance were pure i18n-assertion drift; config 'set' tests rebound to the real TOML save_config path. 46 tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Repoint the mock_database fixture and the four inline node patches from the removed _get_session to the real db_session context-manager seam. The compliance node persists a Fattura and needs a real flush() to assign its id, so mark it real_db and run it against the real in-memory database with a seeded client. 18 pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ai-validation: capture usage/missing-argument text from result.stderr (Click separates streams), pin English locale and assert real strings, correct stale mock import paths (patch where names are bound), and make awaited collaborators AsyncMock so happy paths return 0. events: give the show mock a real published_at datetime; update search default limit 100 -> 50 to match current behavior. 31 tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Wire web service-layer tests and the CLI end-to-end workflow through the runtime_db fixtures so production code (db_session_scope/get_session and the CLI's init_db) shares one isolated database with the seeded setup data, instead of failing on an uninitialized global SessionLocal or operating on a separate empty in-memory DB. Pin English locale; correct stale command names/flags; mock only true external boundaries (AI provider, PEC send). 18 tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- invoice_service: generate_xml raises XMLValidationError on validation failure (documented contract) -> assert the raise, not an error tuple. - rag search missing-argument usage text is on stderr (Click stream split). - cache_config invalid-strategy test was missing its env patch and asserted a pre-structlog message string; set OPENFATTURE_CACHE_STRATEGY=invalid and assert the structlog 'invalid_cache_strategy' warning + lru fallback. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Three real bugs caught by previously-failing tests (sources were wrong, tests were right): - liquidity_service.get_rebalancing_opportunities targeted low-inbound channels (the same depleted-remote side as the source) so it never produced a valid source->target pair. Target high-inbound channels (those short on outbound) and compute amounts against the target outbound ratio. - liquidity_service.stop_monitoring set _monitoring_task = None, discarding the handle; keep the cancelled (done) task so state is inspectable. start_monitoring already restarts on a done task. - stream_json formatter assumed str chunks and crashed (TypeError) on the StreamEvent objects that agents actually yield; normalize StreamEvent.data (text or compact JSON) before serializing. Also restore the in-memory invoice repo fixture in tests/lightning/conftest.py dropped during an earlier lint cleanup (test_basic_integration depended on it). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- config set: dirs.user_config_dir is a str (platformdirs), so 'dir / file' raised TypeError on every invocation; wrap in Path before joining. - lightning compliance-check: the bare 'except Exception' caught the control-flow 'raise typer.Exit(1)' emitted when compliance issues are found and printed a spurious empty 'compliance error' line before re-raising; let typer.Exit propagate untouched. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Real bugs caught while converting tests to a real DB (sources were wrong): - cliente add / fattura crea: a DB failure escaped as a raw traceback (the clean error handling was lost in the db_session() migration). Catch SQLAlchemyError/ValueError, print a clear message, exit 1. Add the cli-cliente-add-error / cli-fattura-create-error messages in all 5 locales. - fattura crea: the invoice year was taken from today() while the issue date was user-entered, so numbering and data_emissione could disagree. Derive anno from the entered issue date. - web match_transaction: built PaymentAllocation with non-existent fattura_id/ payment_date kwargs (TypeError on every real call). Link the invoice's Pagamento to the BankTransaction with the real columns (payment_id/ transaction_id/amount/match_type), with clean failures when prerequisites are missing. - bank-statement importer: import_transactions counted 'Success: N' but never added/committed the parsed rows, so nothing persisted. add() each non-dup row and commit in both session branches. - chat: add the missing cli-ai-chat-assistant-title / -exit-message keys in all 5 locales (were rendering as raw keys). Tests tightened to assert the corrected behavior (clean exit + message, derived year, persisted PaymentAllocation, persisted BankTransaction rows). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- custom_commands streaming test joined StreamEvent objects directly; assemble text from their string .data payloads. - mark TestCustomCommandsPerformance as performance (wall-clock load-time assert belongs in the perf tier, not the functional gate). - cli_e2e full-lifecycle: feed the crea prompts in the new order (issue date first) and assert the year derived from the entered date. - remove tests/lightning/test_lightning_simple.py: a print-only demo script with no test functions or assertions, superseded by the structured lightning tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR modernizes the test infrastructure by replacing mocked database interactions with real isolated runtime fixtures, adds explicit CLI error handling with localized messages, enhances internationalization support, and improves domain logic for payments and imports. Changes span CLI commands, test files, locale files, service layers, and testing utilities across the entire codebase. ChangesTest Infrastructure Modernization & Error Handling
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
openfatture/cli/commands/config.py (1)
227-287:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLet
typer.Exitescape the generic handler
openfatture/cli/commands/config.pycatchestyper.Exit(1)from the invalid-keyraise typer.Exit(1)paths in theexcept Exception as eblock, causing the genericcli-config-errorto print as well. Addexcept typer.Exit: raisebeforeexcept Exception as e(around lines 285-287).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@openfatture/cli/commands/config.py` around lines 227 - 287, The try/except around get_settings()/setting assignment currently catches typer.Exit raised for invalid keys and prints the generic cli-config-error; add an explicit handler for typer.Exit to re-raise it before the broad Exception handler. Specifically, immediately before the existing "except Exception as e" block insert "except typer.Exit: raise" so that the typer.Exit raised from the invalid-key checks (the raise typer.Exit(1) paths inside the nested-key and top-level key checks that use getattr/setattr on settings) is not swallowed, and only other exceptions are handled by the console.print(_("cli-config-error", error=str(e))) clause; leave get_settings, save_config, and the final console prints unchanged.tests/ai/test_invoice_creation_workflow.py (1)
196-197:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMock the actual client lookup used by
_enrich_context_node.The workflow fetches the client through
db.query(Cliente).filter(...).first(), notdb.get(...), so this test never feedsreal_clientinto the exercised branch. As written it can pass on a truthyMagicMockeven if the client-loading path regresses.Suggested fix
- # Mock client query - mock_session.get.return_value = real_client + # Mock client query + mock_session.query.return_value.filter.return_value.first.return_value = real_client🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/ai/test_invoice_creation_workflow.py` around lines 196 - 197, The test is mocking the wrong DB lookup so _enrich_context_node never receives real_client; replace the mock of mock_session.get with a mock that mirrors the actual lookup used in the code (db.query(Cliente).filter(...).first()) so the query().filter(...).first() call returns real_client. Locate the test where mock_session.get.return_value = real_client and instead configure mock_session.query.return_value.filter.return_value.first.return_value = real_client (or the test framework equivalent) so the exercised branch in _enrich_context_node receives the real client object.tests/cli/test_lightning_compliance_commands.py (1)
208-216:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMake the verbose-path assertion unconditional.
sample_compliance_invoicesalways creates one unverified AML payment, so guarding the hash check withif "Unverified AML Payments:" in result.stdout:lets this test pass even when--verbosestops rendering the details block entirely.Suggested test tightening
def test_compliance_check_verbose(test_db, sample_compliance_invoices): """Test compliance-check command with verbose flag.""" result = runner.invoke(app, ["compliance-check", "--tax-year", "2025", "--verbose"]) assert "Lightning Compliance Check - 2025" in result.stdout - # With verbose flag and unverified payments, should show details - if "Unverified AML Payments:" in result.stdout: - # Should show payment hash of unverified payment (truncated to 8 chars) - assert "cccccccc" in result.stdout + # With verbose flag and unverified payments, should show details + assert "Unverified AML Payments:" in result.stdout + # Should show payment hash of unverified payment (truncated to 8 chars) + assert "cccccccc" in result.stdout🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/cli/test_lightning_compliance_commands.py` around lines 208 - 216, The test_compliance_check_verbose should assert the verbose output unconditionally because sample_compliance_invoices always creates an unverified AML payment; remove the conditional block that checks for "Unverified AML Payments:" and instead assert that the details header ("Unverified AML Payments:") appears in result.stdout and that the truncated payment hash "cccccccc" is present (keep references to the test function test_compliance_check_verbose and the fixture sample_compliance_invoices to locate the change).
🧹 Nitpick comments (1)
tests/ai/test_cache_config.py (1)
153-160: ⚡ Quick winRemove the duplicated environment patch.
This test applies the same
@patch.dicttwice forOPENFATTURE_CACHE_STRATEGY. Keeping only one avoids accidental drift if the setup changes later.Proposed fix
`@patch.dict`( os.environ, { "OPENFATTURE_CACHE_STRATEGY": "invalid", }, ) - `@patch.dict`(os.environ, {"OPENFATTURE_CACHE_STRATEGY": "invalid"}) def test_get_cache_config_invalid_strategy(self, caplog):🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/ai/test_cache_config.py` around lines 153 - 160, The test test_get_cache_config_invalid_strategy has the same `@patch.dict` decorator applied twice for OPENFATTURE_CACHE_STRATEGY; remove the duplicate so only a single `@patch.dict`(os.environ, {"OPENFATTURE_CACHE_STRATEGY": "invalid"}) remains above the test function (locate the decorators on test_get_cache_config_invalid_strategy in tests/ai/test_cache_config.py) to avoid redundant environment patching.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@openfatture/cli/commands/cliente.py`:
- Around line 113-183: The current try/except around the whole block treats
errors during post-commit work (event_bus.publish, table rendering) as
persistence failures and leaks DB details; split the error handling so the
transactional steps (creating Cliente, db.add, db.commit, db.refresh) are
wrapped in their own try/except that catches SQLAlchemyError/ValueError and
logs/prints the cli-cliente-add-error without raw DB exception text, then allow
post-commit operations
(get_event_bus()/event_bus.publish(ClientCreatedEvent(...)), console.print table
rendering) to run in a separate try/except that handles non-transactional errors
differently (e.g., report a post-create publication/render failure without
claiming the save failed and avoid including raw DB error strings). Reference
symbols: db_session(), Cliente, db.add/db.commit/db.refresh,
get_event_bus()/event_bus.publish, ClientCreatedEvent, and console.print.
In `@openfatture/cli/commands/fattura.py`:
- Around line 101-103: The prompt text for data_emissione_input is hard-coded in
English; replace the literal "Issue date (YYYY-MM-DD)" in the Prompt.ask call
with the project's translation lookup (the same translation key used for other
CLI prompts) so the prompt is localized — i.e., change the Prompt.ask invocation
for variable data_emissione_input to call the translation function with the
existing issue-date translation key instead of the literal string.
- Around line 60-68: The handler currently maps any exception from
_crea_fattura_in_db (which both commits and performs post-commit work like
publishing InvoiceCreatedEvent and rendering) to "invoice creation failed",
which can hide cases where the DB commit succeeded; fix by separating
transactional failures from post-commit failures: refactor so
_crea_fattura_in_db either only does transactional work (including commit) and
returns the created invoice/id, or have it raise distinct exceptions for
pre-commit vs post-commit phases; then wrap only the transactional call in the
try/except that catches SQLAlchemyError/ValueError and prints the existing
cli-fattura-create-error + typer.Exit, and perform publishing/rendering
(InvoiceCreatedEvent, summary rendering) outside that except with its own error
handling that emits a different, non-misleading message (and does not claim
creation failed or implicitly retry the DB write). Ensure references to
_crea_fattura_in_db and InvoiceCreatedEvent are used to locate and adjust the
code.
In `@openfatture/cli/formatters/stream_json.py`:
- Around line 142-146: The branch handling StreamEvent currently calls
json.dumps(data, ensure_ascii=False) which will raise TypeError for
Decimal/date/datetime payloads; update the serialization to pass a default
serializer function to json.dumps that converts Decimal to str (to preserve
precision) and date/datetime to ISO strings (using .isoformat()), then call
json.dumps(data, ensure_ascii=False, default=serializer) in the StreamEvent
branch (referencing StreamEvent and the existing code that returns
json.dumps(data, ...)). Ensure the serializer handles decimal.Decimal,
datetime.date, and datetime.datetime types.
In `@openfatture/i18n/locales/fr/cli.ftl`:
- Line 1165: The French error message identifier cli-fattura-create-error
contains a typo: change "creation" to the French accented form "création" in the
value string so it reads "[red]Erreur lors de la création de la facture : {
$error }[/red]"; update the FTL entry for cli-fattura-create-error accordingly.
In `@openfatture/lightning/application/services/liquidity_service.py`:
- Around line 226-245: The target channel selection uses a fixed threshold (1.0
- self.max_outbound_ratio) instead of the computed target_outbound_ratio, which
can skip channels that actually need outbound liquidity or include ones that
don't; update the high_inbound list comprehension in the rebalancing loop to
filter using target_outbound_ratio (i.e., select channels where
target.outbound_ratio < target_outbound_ratio) so that subsequent calculations
of target_needed in the rebalancing logic (source_excess, target_needed) only
run against channels that genuinely need outbound liquidity.
In `@openfatture/payment/infrastructure/importers/base.py`:
- Around line 187-191: The session.add(transaction) call causes same-file
duplicates to be invisible to the subsequent _transaction_exists() check because
that check runs inside session.no_autoflush when skip_duplicates=True; fix by
flushing the pending insert immediately after adding the transaction (i.e., call
session.flush() right after session.add(transaction)) so the row becomes visible
to later existence queries, or alternatively implement an in-memory per-batch
seen-signature set and check/update it alongside _transaction_exists() to avoid
relying on DB autoflush.
- Around line 145-152: The code currently calls session.commit() on a
caller-owned session in the object_session(account) branch and relies on
_transaction_exists() under with session.no_autoflush: which prevents visibility
of pending additions; change this to avoid committing a session you don't own
(remove session.commit() and only flush as needed) and ensure newly-added
transactions are visible to existence checks by flushing the session before
running _transaction_exists() (or implement an in-memory duplicate check inside
_process_transactions() to detect duplicates within the same batch); update the
object_session(account) branch to call session.flush() where needed and let the
caller manage commit/rollback, and modify
_process_transactions()/_transaction_exists() so existence queries see pending
adds (or consult an in-memory set) rather than relying on no_autoflush hiding
them.
In `@openfatture/web/services/payment_service.py`:
- Around line 367-374: The lookup rejects valid UUID-string IDs because
BankTransaction.id is serialized as a string but the service compares it to an
int; update the lookup in the payment service to accept the persisted ID type by
converting or parsing transaction_id to the stored type before querying (e.g.,
treat transaction_id as str/UUID) and use the same identifier when calling
session.query(BankTransaction).filter(BankTransaction.id ==
transaction_id).first() so valid UI selections aren't misidentified as
"Transazione non trovata"; ensure any parsing handles invalid values and
preserves the existing error return path.
In `@scripts/_fix_ftl_blank_values.py`:
- Around line 61-72: The script's main function silently succeeds when argv[1]
is wrong because Path.rglob("") yields no files; update main to verify the
computed root Path (variable root) exists and is a directory before iterating
with root.rglob("*.ftl"), and if it's missing or not a dir, print an error and
exit with a non-zero code (or raise SystemExit) so failures are reported;
reference main, argv, root and root.rglob("*.ftl") when adding this guard.
In `@tests/ai/test_invoice_creation_workflow.py`:
- Around line 113-116: The test branch guarded by `@pytest.mark.real_db` currently
spins up a separate in-memory DB instead of using the standardized fixtures;
change the real_db path to reuse the shared file-backed fixtures by wiring the
test to use runtime_db and runtime_session (and the appropriate seed_* fixture)
rather than creating a bespoke in-memory engine/session, so the test exercises
the same runtime session-factory and file-backed DB as other integration tests.
In `@tests/cli/integration/test_custom_commands_integration.py`:
- Around line 239-244: The test currently builds full_response by concatenating
any chunk with a string data; narrow this to only content stream events by
filtering chunks from ChatAgent.execute_stream() to those that are content
events (e.g., check chunk.type == StreamEventType.CONTENT or call
chunk.is_content()) and ensure isinstance(chunk.data, str) before joining, so
full_response is assembled only from StreamEvent.content(...) payloads.
In `@tests/cli/test_ai_validation.py`:
- Around line 301-303: Tests currently only assert non-zero exit codes; update
the assertions to explicitly check Typer/Click conversion error text in
result.stderr (e.g., assert "Invalid value for" in result.stderr or assert
"Error: Invalid value for" in result.stderr) so the failure is tied to
type-conversion behavior. Apply this change to the assertion blocks using the
result variable near the lines shown (both the block around the assert
result.exit_code != 0 at ~301-303 and the similar block at ~514-516) to verify
result.stderr contains the conversion/error message.
In `@tests/cli/test_cliente_commands_unit.py`:
- Line 30: The test uses the default CliRunner which can truncate Rich output;
replace the instantiation runner = CliRunner() with runner = _WideCliRunner()
(the helper used elsewhere that sets COLUMNS=220) so Rich-rendered table output
isn't wrapped; ensure the test imports/uses the existing _WideCliRunner symbol
and the _english_locale fixture is present on the test if not already.
In `@tests/cli/test_cliente_commands.py`:
- Around line 309-312: The test currently asserts the untranslated message key
"cli-cliente-invalid-piva" which will break once translations are added; update
the assertion in the test (the block using result and result.stdout in
tests/cli/test_cliente_commands.py) to assert the actual English message text
that the command prints (or alternatively stub/mock the translation call at the
command boundary so it returns a stable string) instead of asserting the raw
message id; keep the existing exit_code assertion and change only the stdout
check to match the rendered English string or a mocked translation return.
In `@tests/integration/test_cli_e2e_workflow.py`:
- Around line 109-110: Several test functions (e.g., test_create_client_via_app
and the others listed) are end-to-end flows and need the `@pytest.mark.e2e`
decorator; add the decorator above each test function definition (for example
add `@pytest.mark.e2e` immediately above def test_create_client_via_app(...)) and
ensure pytest is imported in the file (import pytest) if not already present;
also verify the e2e marker is declared in pytest.ini if your test suite enforces
marker registration.
---
Outside diff comments:
In `@openfatture/cli/commands/config.py`:
- Around line 227-287: The try/except around get_settings()/setting assignment
currently catches typer.Exit raised for invalid keys and prints the generic
cli-config-error; add an explicit handler for typer.Exit to re-raise it before
the broad Exception handler. Specifically, immediately before the existing
"except Exception as e" block insert "except typer.Exit: raise" so that the
typer.Exit raised from the invalid-key checks (the raise typer.Exit(1) paths
inside the nested-key and top-level key checks that use getattr/setattr on
settings) is not swallowed, and only other exceptions are handled by the
console.print(_("cli-config-error", error=str(e))) clause; leave get_settings,
save_config, and the final console prints unchanged.
In `@tests/ai/test_invoice_creation_workflow.py`:
- Around line 196-197: The test is mocking the wrong DB lookup so
_enrich_context_node never receives real_client; replace the mock of
mock_session.get with a mock that mirrors the actual lookup used in the code
(db.query(Cliente).filter(...).first()) so the query().filter(...).first() call
returns real_client. Locate the test where mock_session.get.return_value =
real_client and instead configure
mock_session.query.return_value.filter.return_value.first.return_value =
real_client (or the test framework equivalent) so the exercised branch in
_enrich_context_node receives the real client object.
In `@tests/cli/test_lightning_compliance_commands.py`:
- Around line 208-216: The test_compliance_check_verbose should assert the
verbose output unconditionally because sample_compliance_invoices always creates
an unverified AML payment; remove the conditional block that checks for
"Unverified AML Payments:" and instead assert that the details header
("Unverified AML Payments:") appears in result.stdout and that the truncated
payment hash "cccccccc" is present (keep references to the test function
test_compliance_check_verbose and the fixture sample_compliance_invoices to
locate the change).
---
Nitpick comments:
In `@tests/ai/test_cache_config.py`:
- Around line 153-160: The test test_get_cache_config_invalid_strategy has the
same `@patch.dict` decorator applied twice for OPENFATTURE_CACHE_STRATEGY; remove
the duplicate so only a single `@patch.dict`(os.environ,
{"OPENFATTURE_CACHE_STRATEGY": "invalid"}) remains above the test function
(locate the decorators on test_get_cache_config_invalid_strategy in
tests/ai/test_cache_config.py) to avoid redundant environment patching.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: abfe6e83-dedd-49e1-ac0b-b1843ab6912d
📒 Files selected for processing (41)
CLAUDE.mdopenfatture/cli/commands/cliente.pyopenfatture/cli/commands/config.pyopenfatture/cli/commands/fattura.pyopenfatture/cli/commands/lightning.pyopenfatture/cli/formatters/stream_json.pyopenfatture/i18n/loader.pyopenfatture/i18n/locales/de/cli.ftlopenfatture/i18n/locales/en/cli.ftlopenfatture/i18n/locales/es/cli.ftlopenfatture/i18n/locales/fr/cli.ftlopenfatture/i18n/locales/it/cli.ftlopenfatture/i18n/translator.pyopenfatture/lightning/application/services/liquidity_service.pyopenfatture/payment/infrastructure/importers/base.pyopenfatture/web/services/payment_service.pypyproject.tomlscripts/_fix_ftl_blank_values.pytests/ai/test_cache_config.pytests/ai/test_invoice_creation_workflow.pytests/cli/integration/test_custom_commands_integration.pytests/cli/test_ai_validation.pytests/cli/test_cliente_commands.pytests/cli/test_cliente_commands_unit.pytests/cli/test_config_commands.pytests/cli/test_events.pytests/cli/test_fattura_commands.pytests/cli/test_fattura_commands_unit.pytests/cli/test_lightning_compliance_commands.pytests/cli/test_pec_commands.pytests/cli/test_rag_validation.pytests/cli/test_report_commands.pytests/conftest.pytests/e2e/test_ai_assistant_e2e.pytests/integration/test_cli_e2e_workflow.pytests/lightning/conftest.pytests/lightning/test_lightning_simple.pytests/payment/infrastructure/importers/test_base_importer_unit.pytests/payment/matchers/test_fuzzy_matcher_performance.pytests/unit/test_invoice_service.pytests/web/test_invoice_creation_wizard.py
💤 Files with no reviewable changes (2)
- tests/e2e/test_ai_assistant_e2e.py
- tests/lightning/test_lightning_simple.py
Comment on lines
+113
to
+183
| try: | ||
| with db_session() as db: | ||
| # Parse name into first and last name if possible | ||
| nome = None | ||
| cognome = None | ||
| if denominazione and " " in denominazione: | ||
| # Try to split the name into first and last name | ||
| parts = denominazione.split(" ", 1) | ||
| if len(parts) == 2: | ||
| nome = parts[0] | ||
| cognome = parts[1] | ||
|
|
||
| cliente = Cliente( | ||
| denominazione=denominazione, | ||
| nome=nome, | ||
| cognome=cognome, | ||
| partita_iva=partita_iva, | ||
| codice_fiscale=codice_fiscale, | ||
| codice_destinatario=codice_destinatario, | ||
| pec=pec, | ||
| indirizzo=indirizzo if interactive else None, | ||
| numero_civico=numero_civico if interactive else None, | ||
| cap=cap if interactive else None, | ||
| comune=comune if interactive else None, | ||
| provincia=provincia if interactive else None, | ||
| nazione="IT", # Default to Italy | ||
| email=email if interactive else None, | ||
| telefono=telefono if interactive else None, | ||
| note=note if interactive else None, | ||
| ) | ||
|
|
||
| console.print(_("cli-cliente-added-success", id=cliente.id)) | ||
|
|
||
| # Show summary | ||
| table = Table(title=_("cli-cliente-table-title", name=denominazione)) | ||
| table.add_column(_("cli-cliente-table-field"), style="cyan") | ||
| table.add_column(_("cli-cliente-table-value"), style="white") | ||
|
|
||
| if partita_iva: | ||
| table.add_row(_("cli-cliente-label-piva"), partita_iva) | ||
| if codice_fiscale: | ||
| table.add_row(_("cli-cliente-label-cf"), codice_fiscale) | ||
| if codice_destinatario: | ||
| table.add_row(_("cli-cliente-label-sdi"), codice_destinatario) | ||
| if pec: | ||
| table.add_row(_("cli-cliente-label-pec"), pec) | ||
| db.add(cliente) | ||
| db.commit() | ||
| db.refresh(cliente) | ||
|
|
||
| # Publish ClientCreatedEvent | ||
| event_bus = get_event_bus() | ||
| if event_bus: | ||
| event_bus.publish( | ||
| ClientCreatedEvent( | ||
| client_id=cliente.id, | ||
| client_name=cliente.denominazione, | ||
| partita_iva=cliente.partita_iva, | ||
| codice_fiscale=cliente.codice_fiscale, | ||
| codice_destinatario=cliente.codice_destinatario, | ||
| pec=cliente.pec, | ||
| ) | ||
| ) | ||
|
|
||
| console.print(table) | ||
| console.print(_("cli-cliente-added-success", id=cliente.id)) | ||
|
|
||
| # Show summary | ||
| table = Table(title=_("cli-cliente-table-title", name=denominazione)) | ||
| table.add_column(_("cli-cliente-table-field"), style="cyan") | ||
| table.add_column(_("cli-cliente-table-value"), style="white") | ||
|
|
||
| if partita_iva: | ||
| table.add_row(_("cli-cliente-label-piva"), partita_iva) | ||
| if codice_fiscale: | ||
| table.add_row(_("cli-cliente-label-cf"), codice_fiscale) | ||
| if codice_destinatario: | ||
| table.add_row(_("cli-cliente-label-sdi"), codice_destinatario) | ||
| if pec: | ||
| table.add_row(_("cli-cliente-label-pec"), pec) | ||
|
|
||
| console.print(table) | ||
| except (SQLAlchemyError, ValueError) as exc: | ||
| # db_session() has already rolled back; convert the failure into a | ||
| # clean error message and non-zero exit instead of a raw traceback. | ||
| console.print(_("cli-cliente-add-error", error=str(exc))) | ||
| raise typer.Exit(1) from exc |
There was a problem hiding this comment.
Scope this handler to the transactional failure path.
db.commit() completes before event publication and summary rendering, but this except still converts any later ValueError/SQLAlchemyError into cli-cliente-add-error. That can tell the user the save failed after the client is already persisted, which makes retrying dangerous, and error=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
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@openfatture/cli/commands/cliente.py` around lines 113 - 183, The current
try/except around the whole block treats errors during post-commit work
(event_bus.publish, table rendering) as persistence failures and leaks DB
details; split the error handling so the transactional steps (creating Cliente,
db.add, db.commit, db.refresh) are wrapped in their own try/except that catches
SQLAlchemyError/ValueError and logs/prints the cli-cliente-add-error without raw
DB exception text, then allow post-commit operations
(get_event_bus()/event_bus.publish(ClientCreatedEvent(...)), console.print table
rendering) to run in a separate try/except that handles non-transactional errors
differently (e.g., report a post-create publication/render failure without
claiming the save failed and avoid including raw DB error strings). Reference
symbols: db_session(), Cliente, db.add/db.commit/db.refresh,
get_event_bus()/event_bus.publish, ClientCreatedEvent, and console.print.
Comment on lines
+60
to
+68
| 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 |
There was a problem hiding this comment.
Don't map post-commit failures to "invoice creation failed".
_crea_fattura_in_db() commits before it publishes InvoiceCreatedEvent and renders the summary, so this outer handler can still emit cli-fattura-create-error after the invoice is already stored. That creates the same duplicate-retry hazard as cliente add, and error=str(exc) will surface raw DB/parser details to the user. Limit this handler to the transactional failure path and treat post-commit errors separately.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@openfatture/cli/commands/fattura.py` around lines 60 - 68, The handler
currently maps any exception from _crea_fattura_in_db (which both commits and
performs post-commit work like publishing InvoiceCreatedEvent and rendering) to
"invoice creation failed", which can hide cases where the DB commit succeeded;
fix by separating transactional failures from post-commit failures: refactor so
_crea_fattura_in_db either only does transactional work (including commit) and
returns the created invoice/id, or have it raise distinct exceptions for
pre-commit vs post-commit phases; then wrap only the transactional call in the
try/except that catches SQLAlchemyError/ValueError and prints the existing
cli-fattura-create-error + typer.Exit, and perform publishing/rendering
(InvoiceCreatedEvent, summary rendering) outside that except with its own error
handling that emits a different, non-misleading message (and does not claim
creation failed or implicitly retry the DB write). Ensure references to
_crea_fattura_in_db and InvoiceCreatedEvent are used to locate and adjust the
code.
Comment on lines
+101
to
+103
| data_emissione_input = Prompt.ask( | ||
| "Issue date (YYYY-MM-DD)", default=date.today().isoformat() | ||
| ) |
There was a problem hiding this comment.
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
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@openfatture/cli/commands/fattura.py` around lines 101 - 103, The prompt text
for data_emissione_input is hard-coded in English; replace the literal "Issue
date (YYYY-MM-DD)" in the Prompt.ask call with the project's translation lookup
(the same translation key used for other CLI prompts) so the prompt is localized
— i.e., change the Prompt.ask invocation for variable data_emissione_input to
call the translation function with the existing issue-date translation key
instead of the literal string.
Comment on lines
+142
to
+146
| if isinstance(chunk, StreamEvent): | ||
| data = chunk.data | ||
| if isinstance(data, str): | ||
| return data | ||
| return json.dumps(data, ensure_ascii=False) |
There was a problem hiding this comment.
Handle Decimal and datetime payloads when serializing structured stream events.
json.dumps(data) will still raise TypeError for the codebase's normal Decimal/date/datetime values, so this path can still break streaming for structured events. Use a serializer hook here before embedding the payload into the JSONL record.
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
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@openfatture/cli/formatters/stream_json.py` around lines 142 - 146, The branch
handling StreamEvent currently calls json.dumps(data, ensure_ascii=False) which
will raise TypeError for Decimal/date/datetime payloads; update the
serialization to pass a default serializer function to json.dumps that converts
Decimal to str (to preserve precision) and date/datetime to ISO strings (using
.isoformat()), then call json.dumps(data, ensure_ascii=False,
default=serializer) in the StreamEvent branch (referencing StreamEvent and the
existing code that returns json.dumps(data, ...)). Ensure the serializer handles
decimal.Decimal, datetime.date, and datetime.datetime types.
| cli-ai-chat-assistant-title = [bold cyan]Assistant IA[/bold cyan] | ||
| cli-ai-chat-exit-message = [yellow]Au revoir ![/yellow] | ||
| cli-cliente-add-error = [red]Erreur lors de l'enregistrement du client : { $error }[/red] | ||
| cli-fattura-create-error = [red]Erreur lors de la creation de la facture : { $error }[/red] |
There was a problem hiding this comment.
Fix French typo in invoice creation error message.
creation should be création in this user-facing string.
Suggested fix
-cli-fattura-create-error = [red]Erreur lors de la creation de la facture : { $error }[/red]
+cli-fattura-create-error = [red]Erreur lors de la création de la facture : { $error }[/red]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| cli-fattura-create-error = [red]Erreur lors de la creation de la facture : { $error }[/red] | |
| cli-fattura-create-error = [red]Erreur lors de la création de la facture : { $error }[/red] |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@openfatture/i18n/locales/fr/cli.ftl` at line 1165, The French error message
identifier cli-fattura-create-error contains a typo: change "creation" to the
French accented form "création" in the value string so it reads "[red]Erreur
lors de la création de la facture : { $error }[/red]"; update the FTL entry for
cli-fattura-create-error accordingly.
Comment on lines
+239
to
+244
| # Verify streaming. execute_stream yields StreamEvent objects; assemble | ||
| # the textual content from their string payloads. | ||
| assert len(chunks) > 0 | ||
| full_response = "".join(chunks) | ||
| full_response = "".join( | ||
| chunk.data for chunk in chunks if isinstance(getattr(chunk, "data", None), str) | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Inspect StreamEvent definition"
fd -i 'events.py' openfatture tests | while read -r file; do
rg -n -C3 'class StreamEvent|event_type|kind|type:|data:' "$file" || true
done
echo
echo "Inspect execute_stream implementations and StreamEvent construction sites"
rg -n -C3 'async def execute_stream|yield .*StreamEvent|StreamEvent\(' openfatture testsRepository: gianlucamazza/openfatture
Length of output: 28736
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the relevant test code around the cited lines
FILE="tests/cli/integration/test_custom_commands_integration.py"
if [ -f "$FILE" ]; then
sed -n '200,280p' "$FILE"
echo "----"
sed -n '340,420p' "$FILE"
else
echo "Missing $FILE"
fi
echo
echo "Search for StreamEvent.thinking/status emissions"
rg -n "StreamEvent\.(thinking|status)\(" openfatture/ai/agents openfatture/ai/streaming openfatture -S || true
echo
echo "Inspect stream_json formatter content extraction logic"
sed -n '80,180p' openfatture/cli/formatters/stream_json.pyRepository: gianlucamazza/openfatture
Length of output: 9395
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="tests/cli/integration/test_custom_commands_integration.py"
sed -n '1,120p' "$FILE"
echo "----"
rg -n "StreamEventType\.CONTENT|execute_stream" "$FILE"
echo "----"
rg -n "StreamEventType\." openfatture/ai | head -n 50
echo "----"
rg -n "StreamEventType\.THINKING|StreamEventType\.STATUS|StreamEvent\.thinking\(|StreamEvent\.status\(" openfatture/ai openfatture/cliRepository: gianlucamazza/openfatture
Length of output: 6117
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Inspect StreamEvent and StreamEventType definitions
sed -n '1,260p' openfatture/ai/streaming/events.py
echo "----"
sed -n '1,120p' openfatture/ai/streaming/__init__.py
echo "----"
# 2) Inspect ChatAgent.execute_stream streaming wrapper logic
sed -n '300,720p' openfatture/ai/agents/chat_agent.py
echo "----"
# 3) Search for thinking/status emissions across the streaming/agent code
rg -n "StreamEvent\.(thinking|status)\(" openfatture/ai -S
echo "----"
rg -n "StreamEventType\.(THINKING|STATUS)" openfatture/ai -SRepository: gianlucamazza/openfatture
Length of output: 26411
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect BaseAgent.execute_stream implementation
sed -n '200,380p' openfatture/ai/domain/agent.py
echo "----"
rg -n "async def execute_stream|yield " openfatture/ai/domain/agent.py
echo "----"
# Inspect ReActOrchestrator.stream implementation
rg -n "class ReActOrchestrator|async def stream" openfatture/ai -S
echo "----"
# Read the likely orchestrator file(s)
fd -e py "react_orchestrator*.py" openfatture/ai -t f 2>/dev/null || true
# Fallback: print first ~260 lines around the stream method locations
rg -n "async def stream\\(" openfatture/ai -S | head -n 20Repository: gianlucamazza/openfatture
Length of output: 7358
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Search for StreamEvent.thinking/status usage"
rg -n "StreamEvent\.(thinking|status)\(" openfatture -S || true
echo
echo "Search for StreamEventType.THINKING/STATUS usage outside events.py"
rg -n "StreamEventType\.(THINKING|STATUS)" openfatture -S || true
echo
echo "Inspect ChatAgent defaults for enable_tools"
rg -n "class ChatAgent|def __init__|enable_tools" openfatture/ai/agents/chat_agent.py
echo
echo "Inspect any ReActOrchestrator stream yields thinking/status"
sed -n '200,330p' openfatture/ai/orchestration/react.py
sed -n '1,120p' openfatture/ai/orchestration/react.pyRepository: gianlucamazza/openfatture
Length of output: 9912
Narrow streaming transcript assembly to StreamEventType.CONTENT events
tests/cli/integration/test_custom_commands_integration.py (lines 239-244) currently concatenates any streamed item whose data is a string. While ChatAgent.execute_stream() yields StreamEvent.content(...) for content (and tool/metrics events use dict payloads), filter explicitly to content events (e.g., if chunk.type == StreamEventType.CONTENT or if chunk.is_content() and isinstance(chunk.data, str)) before building full_response to keep the assertion aligned with the streaming contract.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/cli/integration/test_custom_commands_integration.py` around lines 239 -
244, The test currently builds full_response by concatenating any chunk with a
string data; narrow this to only content stream events by filtering chunks from
ChatAgent.execute_stream() to those that are content events (e.g., check
chunk.type == StreamEventType.CONTENT or call chunk.is_content()) and ensure
isinstance(chunk.data, str) before joining, so full_response is assembled only
from StreamEvent.content(...) payloads.
Comment on lines
301
to
303
| assert result.exit_code != 0 | ||
| # Should show argument type conversion error | ||
| # Should show argument type conversion error (on stderr) | ||
|
|
There was a problem hiding this comment.
Assert the stderr conversion error explicitly.
Both tests only check for a non-zero exit, so they still pass if the command fails for an unrelated reason. Add a concrete result.stderr assertion for Typer's invalid-value/type-conversion message to keep these tied to the behavior the test names describe.
Also applies to: 514-516
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/cli/test_ai_validation.py` around lines 301 - 303, Tests currently only
assert non-zero exit codes; update the assertions to explicitly check
Typer/Click conversion error text in result.stderr (e.g., assert "Invalid value
for" in result.stderr or assert "Error: Invalid value for" in result.stderr) so
the failure is tied to type-conversion behavior. Apply this change to the
assertion blocks using the result variable near the lines shown (both the block
around the assert result.exit_code != 0 at ~301-303 and the similar block at
~514-516) to verify result.stderr contains the conversion/error message.
| TipoDocumento, | ||
| ) | ||
|
|
||
| runner = CliRunner() |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Use the wide CLI runner here as well.
These tests now assert Rich-rendered CLI output, so the default 80-column CliRunner can still truncate cells and make substring assertions flaky. Reuse the _WideCliRunner pattern already adopted elsewhere in this PR.
As per coding guidelines, "Test CLI output using _english_locale fixture and _WideCliRunner with COLUMNS=220 for wide table assertions".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/cli/test_cliente_commands_unit.py` at line 30, The test uses the
default CliRunner which can truncate Rich output; replace the instantiation
runner = CliRunner() with runner = _WideCliRunner() (the helper used elsewhere
that sets COLUMNS=220) so Rich-rendered table output isn't wrapped; ensure the
test imports/uses the existing _WideCliRunner symbol and the _english_locale
fixture is present on the test if not already.
Comment on lines
309
to
+312
| assert result.exit_code == 0 | ||
| assert "Invalid Partita IVA" in result.stdout | ||
| # The command warns about the invalid P.IVA. The English locale has no | ||
| # translation for this key yet, so it renders the message id verbatim. | ||
| assert "cli-cliente-invalid-piva" in result.stdout |
There was a problem hiding this comment.
Do not lock this test to an untranslated message key.
Asserting cli-cliente-invalid-piva bakes the current English-locale defect into the suite, so the test will fail when the real fix lands. Assert the rendered English message instead, or stub translation at the command boundary if you need a stable token.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/cli/test_cliente_commands.py` around lines 309 - 312, The test
currently asserts the untranslated message key "cli-cliente-invalid-piva" which
will break once translations are added; update the assertion in the test (the
block using result and result.stdout in tests/cli/test_cliente_commands.py) to
assert the actual English message text that the command prints (or alternatively
stub/mock the translation call at the command boundary so it returns a stable
string) instead of asserting the raw message id; keep the existing exit_code
assertion and change only the stdout check to match the rendered English string
or a mocked translation return.
Comment on lines
+109
to
+110
| def test_create_client_via_app(self, app_runner, runtime_db): | ||
| """Test creating a client through CLI (interactive).""" |
There was a problem hiding this comment.
Mark these end-to-end flows with @pytest.mark.e2e.
These tests drive the full CLI, real database fixtures, filesystem IO, and mocked external boundaries, so leaving them unmarked means the new default deselection will not filter them out.
As per coding guidelines, "Mark end-to-end tests with @pytest.mark.e2e decorator".
Also applies to: 148-150, 157-158, 191-193, 257-259, 303-305, 325-327, 344-346, 363-365, 401-403, 433-435
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/integration/test_cli_e2e_workflow.py` around lines 109 - 110, Several
test functions (e.g., test_create_client_via_app and the others listed) are
end-to-end flows and need the `@pytest.mark.e2e` decorator; add the decorator
above each test function definition (for example add `@pytest.mark.e2e`
immediately above def test_create_client_via_app(...)) and ensure pytest is
imported in the file (import pytest) if not already present; also verify the e2e
marker is declared in pytest.ini if your test suite enforces marker
registration.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The default
pytestgate had 197 failures (160 failed + 37 errors). Root causes were architectural, not flaky: tests mocked internal import names that the 2026 refactors had moved/renamed, the i18n rollout shipped broken Fluent messages, and several real production bugs were masked by query-chain mocks. This branch fixes all of it and brings the functional gate to 2004 passed, 0 failed.Testing architecture (no more brittle mocks)
runtime_db/runtime_session/seed_*fixtures point an isolated, file-backed SQLite DB at the global session factory andget_settings()(via env), so CLI commands, web services and agents under test share one real database with the test's seeded data — replacingpatch(module.SessionLocal)+ mocked SQLAlchemy query chains that broke when the code migrated todb_session().performance/benchmark/slow/e2e/ollama(opt-in via-m), so it is fast and deterministic. Two mismarked perf tests fixed; a legacy print-only "test" script removed.stderr(Click splits streams); wide Rich tables use a 220-col runner.Real production bugs fixed (caught by the tests)
[markup]were silently dropped (rendered as raw keys) in all 5 locales; bidi isolation marks (U+2068/9) corrupted output; years rendered as2,025. Fixed loader, 120.ftlentries, and year formatting.get_rebalancing_opportunitiestargeted the wrong channel side (never produced a pair);stop_monitoringdiscarded the task handle.config setcrashed (str / str);lightning compliance-checkprinted a spurious error on its owntyper.Exit;cliente add/fattura crealeaked raw tracebacks on DB errors;fattura creaused today's year instead of the issue-date year.match_transactionbuiltPaymentAllocationwith non-existent fields (TypeError on every call); the bank-statement importer reported success but never persisted rows.StreamEventobjects agents actually yield.Every source fix is paired with a test asserting the corrected behavior.
Validation
uv run pytest(default gate): 2004 passed, 10 skipped, 176 deselected, 0 failed.ruff check openfatture/: clean.black --check: clean.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores