Refactor: package fafycat as an installable Python package#19
Refactor: package fafycat as an installable Python package#19davidchris merged 10 commits intomainfrom
Conversation
Centralize refactor-fragile imports in a single conftest touchpoint (importlib block), add an app_factory fixture that replaces import-time app construction, and autouse-isolate FAFYCAT_* env vars across tests. Add parity tests (app factory, static serving, CLI via subprocess, AppConfig env-var contract) that hold both pre- and post-refactor and serve as the green light when packaging lands. Partition manual, perf, and server-required tests behind opt-in markers so the default pytest run is hermetic. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Remaining pre-refactor test-suite changes missed in the previous commit (ede7d79): the conftest.py rewrite with importlib touchpoint, pytest marker partitioning in pyproject.toml + manual/perf test modules, and fixture rebases in test_analytics_api.py / test_ml_api.py / test_ml_endpoints_simple.py / test_ml_e2e.py. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 971aa5e850
ℹ️ 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".
|
|
||
| resolved_data_dir = data_dir.resolve() | ||
| os.environ["FAFYCAT_DATA_DIR"] = str(resolved_data_dir) | ||
| os.environ.setdefault("FAFYCAT_DB_URL", f"sqlite:///{resolved_data_dir / 'fafycat.db'}") |
There was a problem hiding this comment.
Preserve dev/prod DB split when --data-dir is provided
cmd_serve computes mode-specific database names (fafycat_dev.db / fafycat_prod.db), but _apply_data_dir_override sets FAFYCAT_DB_URL to .../fafycat.db first. Because the later mode-specific branch only runs when FAFYCAT_DB_URL is absent, both fafycat serve --dev --data-dir ... and fafycat serve --data-dir ... end up using the same database file, which can mix synthetic development data into production data whenever users rely on --data-dir.
Useful? React with 👍 / 👎.
Code reviewFound 2 issues:
Lines 51 to 65 in 971aa5e
Lines 174 to 200 in 971aa5e 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
_apply_data_dir_override set FAFYCAT_DB_URL to the generic fafycat.db before cmd_serve could determine the mode-specific name, causing both modes to share one file when --data-dir was given. Also remove dead _add_data_dir_argument call on the root parser — argparse subparser defaults silently overwrote it, so `fafycat --data-dir /x serve` yielded args.data_dir=None.
Summary
src/fafycat/(api, web, static, app.py, cli.py)uv_buildbuild backend souv tool install fafycatworks end-to-endrun_dev.py,run_prod.py, and rootcli.pywith a singlesrc/fafycat/cli.pyentry point (fafycat serve,fafycat import,fafycat init)platformdirs;--data-diror env vars still overridesrc/,tests/,scripts/, andsimulations/pythonpath = ["."]to pytest config sosimulations/tests remain importable under thesrclayout.claude/worktrees fromtytype checkingTest plan
uv syncsucceedsuv run fafycat --helpworksuv run fafycat serve --devstarts the appuv tool install . --force && fafycat --helpworksuv run pytest)🤖 Generated with Claude Code