Conversation
…(v0.13.0) Address all 5 review priorities from project review: - Centralize duplicated logic: normalize_country(), _db_connection(), _build_result() helpers (#22) - Narrow exception handling: replace 9 bare except Exception blocks with specific types (#23) - Add Makefile, pre-commit hooks, ruff format CI check (#24) - Add 69 pytest tests covering postal_patterns, data_loader, and API endpoints with CI test job (#25) - Version bump to 0.13.0, requirements-dev.txt, .dockerignore updates Closes #22, closes #23, closes #24, closes #25
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 757ac305a8
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| logger.info("Loaded %d estimates from %s", count, csv_path) | ||
| return count > 0 | ||
| except Exception as exc: | ||
| except (OSError, KeyError, ValueError) as exc: |
There was a problem hiding this comment.
The narrowed exception tuple in _load_estimates_from_csv no longer includes csv.Error, so malformed estimate CSVs (for example files containing NUL bytes or parser-level CSV corruption) will now raise out of this function and bubble into load_data() during startup, taking the service down instead of logging a warning and continuing without estimates. This is a behavior regression from the previous broad handling of this optional input path.
Useful? React with 👍 / 👎.
Address review priorities — v0.13.0
1 participant
Summary
Addresses all 5 top priorities identified in the project review (scored 7/10), implemented as GitHub issues #22–#26:
normalize_country()replaces duplicate GR→EL blocks,_db_connection()context manager replaces 6 manual SQLite connect/close patterns,_build_result()helper eliminates repetitive result dict construction across all 5 lookup tiers. Addedrequirements-dev.txtand return type hints.except Exceptionblocks indata_loader.pyreplaced with specific types (sqlite3.Error,httpx.RequestError,OSError, etc.). Silent catch inimport_estimates.pynow logs a message.Makefilewith standard targets,.pre-commit-config.yamlwith ruff hooks,ruff format --checkadded to CI lint job.test_postal_patterns.py(preprocessing, tercet_map, extraction for 11 countries),test_data_loader.py(normalize functions, all 5 lookup tiers),test_api.py(all 3 endpoints). CItestjob added to workflow.Test plan
ruff check app/ tests/— passesruff format --check app/ tests/— passespytest tests/ -v— 69/69 tests passpython -c "from app.main import app; print('imports OK')"— passesCloses #22, closes #23, closes #24, closes #25