Surface swallowed errors across pipeline and UI - #13
Surface swallowed errors across pipeline and UI#13devin-ai-integration[bot] wants to merge 2 commits into
Conversation
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 06bc155384
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| monkeypatch.setattr(update_master, "build_restricted_datasets", _boom) | ||
| monkeypatch.setattr(update_master, "_load_dataframe", lambda *_a, **_k: pd.DataFrame()) | ||
| monkeypatch.setattr(update_master, "_write_master_outputs", lambda *_a, **_k: None, raising=False) |
There was a problem hiding this comment.
Isolate the update_master test from runtime CSVs
This monkeypatch creates _write_master_outputs, but update_master_database() never calls that helper; it writes to SOLD_FILE, REFERRED_FILE, and ACTIVE_FILE directly via _merge_preserving_history()/_atomic_write() before build_restricted_datasets() is reached. In a normal pytest run against the tracked CSV_data baseline, this test can rewrite the real runtime CSVs while only intending to assert the raised RuntimeError; redirect the file constants or write helpers to tmp_path instead.
AGENTS.md reference: AGENTS.md:L101-L102
Useful? React with 👍 / 👎.
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Summary
An audit of every
excepthandler outsideautotrader_isolated/found ~107 broad handlers; most were legitimate fallbacks, but a large group turned real operational failures (unreadable CSV, missing model artifact, corrupt lock/marker JSON, bad timezone config) into empty frames,False, or partial results with no record anywhere. This PR keeps the resilience but makes the failure visible, and fixes two places where a failure was reported as success.Two deliberate behaviour changes:
The emergency snapshot is still taken; the caller now learns the run failed.
pages/3_ACTIVE_LISTINGS.pywraps the call and rendersst.error/st.exceptioninstead of continuing as if bids were refreshed.This makes the daily job fail loudly (health report + Telegram failure alert) rather than leaving the restricted datasets that feed AI valuation silently stale.
Everything else is narrowing + reporting. A shared tuple centralises what a CSV read can legitimately raise:
(pandas
ParserError/EmptyDataErrorboth derive fromValueError.) Handlers inshared/(data_loader,csv_utils,telegram_alerts,scraper_health,curves,canonical_tagging,repair_pricing*,repair_review,repair_ai_classifier,manual_curve_evidence,missed_opportunities,auction_model,ui_helpers),scripts/(scheduled_jobs,update_bids,update_master,extract_vehicle_details,ai_price_analysis,ai_listing_valuation,scrape_bid_history,scrape_autotrader_rego,process_curve_candidates,run_grays_pipeline_loop),ops/active_monitor.py, two Streamlit pages, andstatus_app.pynow catch only what they expect and log/print the path plustype(exc).__name__: excbefore falling back. Notable ones where silence was actively misleading:auction_model._try_load_models()(valuations silently drop to curve-only pricing),_load_alert_log/_load_alert_state(duplicate Telegram alerts get re-sent),_should_refresh()(stale-data marker), and_local_timezone()(coverage dates silently shift to UTC).Two broad handlers were removed rather than narrowed —
derive_auction_site()(pure string split) andrecord_snapshot()'s.iloc[0]lookup, which now uses an explicitmatches.emptycheck. Also fixedupdate_bids()returning[]instead of a DataFrame on the empty-seed path.tests/test_error_visibility.pyasserts the new diagnostics are emitted (viacaplog) for CSV/alert-log/state-file failures and that the restricted-build failure propagates.Verified:
634 passed,ruff check .clean,readiness_smoke,governance_checks check,project_memory check(and--staged),check_commit_hygiene --stagedall pass.project_memory/02_state/recent_changes.mdupdated.Link to Devin session: https://app.devin.ai/sessions/86d7aa075214441192e549f6879fbad4
Requested by: @fallen-pc