fix: production hardening — 20 code quality fixes#3
Conversation
Infrastructure:
- Add GitHub Actions CI workflow (Rust tests + clippy + Python tests)
- Add requirements.txt with pinned dependency versions
- Fix Makefile to consistently use python3.11 (was bare python)
- Fix Cargo.toml invalid target-cpu profile key (not a valid Cargo setting)
- Add .gitignore entries for generated docs artifacts
Rust (zero clippy warnings, 70 tests):
- Fix all clippy warnings: &PathBuf→&Path, map_or→is_some_and, manual range contains, is_multiple_of
- Introduce SearchConfig and CandidateParams structs to eliminate too-many-arguments
- Fix double unwrap anti-pattern in crossmatch.rs lookup
- Consolidate chained .replace() calls in normalize_name
- Add --version flag to CLI via clap
- Add doc-tests for median, compute_phases, generate_periods
Python (32 tests pass):
- Replace blanket warnings.filterwarnings("ignore") with specific filters
- Add type hints (from __future__ import annotations) to all 4 Python modules
- Replace 6 bare except Exception blocks with specific exceptions + logging
- Add named constants for magic numbers in validate_candidates.py
- Add CSV column validation (checks for time/flux columns)
- Fix path traversal risk: use Path.stem / os.path.basename for filenames
- Remove global mutable state in deep_analysis.py (findings dict now passed as param)
- Sanitize ADQL query inputs in deep_analysis.py Gaia queries
- Add input file existence check in analyze_candidates.py
https://claude.ai/code/session_01EWyvEJ6ABSWZxKMrTPBdW8
![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: 8208079ec5
ℹ️ 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".
| setup: | ||
| @echo "📦 Installing dependencies..." | ||
| pip install lightkurve astroquery pandas numpy matplotlib tqdm --quiet | ||
| pip install -r requirements.txt --quiet |
There was a problem hiding this comment.
The setup target now installs dependencies with bare pip while the rest of the Makefile runs scripts with python3.11; this can install packages into a different interpreter environment and cause ModuleNotFoundError when make download/make analyze run under Python 3.11. Using python3.11 -m pip here keeps dependency installation aligned with the runtime commands.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 9d4df19 — pip install → python3.11 -m pip install in the setup target.
Generated by Claude Code
|
|
||
| except Exception: | ||
| except (OSError, ConnectionError, ValueError, RuntimeError) as e: | ||
| print(f" Warning: skipping TIC {int(row.get('TIC ID', 0))}: {e}") |
There was a problem hiding this comment.
This handler catches ValueError, but the warning message calls int(row.get('TIC ID', 0)) again; if the original error came from a malformed or missing TIC ID, the same conversion raises a second ValueError inside the except block and terminates the download loop instead of skipping that row. Log the raw value (or guard conversion) in the handler.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 9d4df19 — now uses row.get('TIC ID', 'unknown') directly instead of re-calling int() in the except handler.
Generated by Claude Code
- Makefile: use python3.11 -m pip for setup target to match runtime interpreter - download_lightcurves.py: avoid re-parsing TIC ID in except handler to prevent secondary ValueError from crashing the download loop https://claude.ai/code/session_01EWyvEJ6ABSWZxKMrTPBdW8
2 participants
Summary
Comprehensive production hardening based on a brutal code review. Fixes 20 identified issues across Rust, Python, and project infrastructure — zero clippy warnings, all 102 tests pass (70 Rust + 32 Python).
Infrastructure (Critical)
requirements.txt: Pinned Python dependency versions for reproducible buildspython3.11(was barepythonin 3 targets)Cargo.toml: Remove invalidtarget-cpu = "native"profile key (not a Cargo setting).gitignore: Exclude generateddocs/candidates.jsonanddocs/validation_results.jsonRust (Zero Clippy Warnings)
&PathBuf→&Path,map_or→is_some_and, manual range contains,is_multiple_ofSearchConfigandCandidateParamsstructs to eliminate too-many-arguments warnings.unwrap()anti-pattern incrossmatch.rslookup.replace()calls innormalize_name--versionflag to CLImedian,compute_phases,generate_periodsPython (All 32 Tests Pass)
warnings.filterwarnings("ignore")with specific filters in all 4 modulesfrom __future__ import annotations) to all Python modulesexcept Exceptionblocks with specific exceptions + warning logsvalidate_candidates.pytime/fluxcolumns before processing)Path.stem/os.path.basenamefor filenamesfindings = {}indeep_analysis.py(now passed as parameter)deep_analysis.pyGaia queriesanalyze_candidates.pyTest plan
cargo clippy— zero warningscargo test— 67 unit tests + 3 doc-tests passpython3.11 -m pytest tests/ -v— 32 tests passmake testruns end-to-end on clean checkouthttps://claude.ai/code/session_01EWyvEJ6ABSWZxKMrTPBdW8