[codex] Align local state persistence and schedule docs#5
Conversation
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
gp300 | 6607fbe | Commit Preview URL Branch Preview URL |
Apr 12 2026, 06:43 PM |
There was a problem hiding this comment.
Pull request overview
Aligns local SQLite state persistence and user-facing schedule documentation with the production Cloudflare Worker/D1 behavior, particularly around last_rebase tracking and cron cadence.
Changes:
- Persist/restore
last_rebasein the localstateJSON blob viadb.save_state/db.load_state. - Add a regression test ensuring
last_rebaseround-trips through local persistence. - Update README +
main.py --helpschedule language and ignore local.trees/worktrees.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
db.py |
Persists last_rebase into the local state blob and restores it on load. |
test_db.py |
Adds a regression test for last_rebase round-tripping through SQLite state. |
README.md |
Updates schedule text and local run/test commands. |
main.py |
Updates CLI usage/help strings to reflect the real cadence. |
.gitignore |
Ignores .trees/ to prevent accidental commits of worktrees. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def main(): | ||
| parser = argparse.ArgumentParser(description="G&P 300 Index Engine") | ||
| parser.add_argument( | ||
| "--update", action="store_true", help="Regular price update (30-min cycle)" | ||
| "--update", action="store_true", help="Scheduled 5-minute price update" | ||
| ) | ||
| parser.add_argument( | ||
| "--rebalance", action="store_true", help="Recompute weights (weekly cycle)" | ||
| "--rebalance", action="store_true", help="Scheduled daily rebalance" | ||
| ) | ||
| parser.add_argument( | ||
| "--reconstitute", | ||
| action="store_true", | ||
| help="Re-select constituents (biweekly cycle)", | ||
| help="Scheduled weekly reconstitution + rebalance", | ||
| ) |
There was a problem hiding this comment.
src/engine.run_full_pipeline docstring still describes reconstitute as biweekly and rebalance as weekly, which now conflicts with the updated CLI/help text (5-minute/daily/weekly). Consider updating the engine docstring too so schedule documentation is consistent across the codebase.
| ts = datetime(2026, 4, 12, 18, 30, tzinfo=timezone.utc) | ||
| last_rebase = datetime(2026, 4, 6, 0, 0, tzinfo=timezone.utc) | ||
| state = IndexState( | ||
| value=1000.0, | ||
| divisor=0.0005, | ||
| weighted_entropy=0.5, | ||
| num_constituents=0, | ||
| timestamp=ts, | ||
| constituents=[], | ||
| last_rebase=last_rebase, | ||
| ) | ||
|
|
||
| db.save_state(conn, state) | ||
| loaded = db.load_state(conn) | ||
|
|
||
| assert loaded is not None | ||
| assert loaded.last_rebase == last_rebase |
There was a problem hiding this comment.
This test opens a SQLite connection but never closes it. Closing the connection (e.g., via try/finally or a fixture) avoids leaking file handles and reduces flakiness on platforms with strict file locking.
| ts = datetime(2026, 4, 12, 18, 30, tzinfo=timezone.utc) | |
| last_rebase = datetime(2026, 4, 6, 0, 0, tzinfo=timezone.utc) | |
| state = IndexState( | |
| value=1000.0, | |
| divisor=0.0005, | |
| weighted_entropy=0.5, | |
| num_constituents=0, | |
| timestamp=ts, | |
| constituents=[], | |
| last_rebase=last_rebase, | |
| ) | |
| db.save_state(conn, state) | |
| loaded = db.load_state(conn) | |
| assert loaded is not None | |
| assert loaded.last_rebase == last_rebase | |
| try: | |
| ts = datetime(2026, 4, 12, 18, 30, tzinfo=timezone.utc) | |
| last_rebase = datetime(2026, 4, 6, 0, 0, tzinfo=timezone.utc) | |
| state = IndexState( | |
| value=1000.0, | |
| divisor=0.0005, | |
| weighted_entropy=0.5, | |
| num_constituents=0, | |
| timestamp=ts, | |
| constituents=[], | |
| last_rebase=last_rebase, | |
| ) | |
| db.save_state(conn, state) | |
| loaded = db.load_state(conn) | |
| assert loaded is not None | |
| assert loaded.last_rebase == last_rebase | |
| finally: | |
| conn.close() |
| # Run the CLI locally | ||
| python -m cli | ||
| python3 -m cli | ||
|
|
||
| # Run tests | ||
| python test_engine.py | ||
| python3 -m pytest -q |
There was a problem hiding this comment.
The README now uses python3 explicitly for running the CLI/tests. This can be less portable on some environments (notably Windows) where the command is typically python/py. Consider using python -m ... (or mentioning both) to keep the instructions cross-platform.
|
@copilot review |
All three review findings were addressed in commit
|
What changed
last_rebasein the local SQLite state path sodb.pymatches the Cloudflare Worker/D1 behaviorsave_state/load_stateround-trippinglast_rebase.trees/to.gitignoreso local worktrees stay untrackedWhy
There were two repo/runtime mismatches:
db.pydroppedlast_rebase, so local runs were not behavior-equivalent to production around rebase trackingImpact
.trees/worktrees can be used safely without risking accidental pushesValidation
/Users/yu_zheng/workspace/gp300/.venv/bin/python -m pytest -q test_db.py/Users/yu_zheng/workspace/gp300/.venv/bin/python -m pytest -q/Users/yu_zheng/workspace/gp300/.venv/bin/python main.py --help