PR #575: Fix run_xgboost_tasklet — exclude seed rows, fix graded_at crash, fix DB persist#445
Conversation
…rash, fix DB persist
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
 |
|
Overall Grade |
Security Reliability Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| Docker |  |
May 16, 2026 7:33a.m. | Review ↗ |
| JavaScript | |
May 16, 2026 7:33a.m. | Review ↗ |
| Python | |
May 16, 2026 7:33a.m. | Review ↗ |
| SQL | |
May 16, 2026 7:33a.m. | Review ↗ |
| Secrets | |
May 16, 2026 7:33a.m. | Review ↗ |
Important
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
Up to standards ✅🟢 Issues
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the XGBoost tasklet by excluding 'seed' agents from the training set, reducing the minimum training data requirement to 50 rows, and improving the robustness of the recency decay calculation. It also transitions the model persistence logic to store base64-encoded pickles in the database. Feedback highlights a critical TypeError in the datetime subtraction logic due to mixed timezone awareness and a potential breaking change where the new storage format may be incompatible with existing model loaders.
| now_utc = datetime.datetime.now(datetime.timezone.utc) | ||
| _default_graded = now_utc - datetime.timedelta(days=30) # PR #575: default for NULL graded_at | ||
| def _parse_graded_at(v): | ||
| if v is None: | ||
| return _default_graded | ||
| if isinstance(v, datetime.datetime): | ||
| return v | ||
| try: | ||
| return datetime.datetime.fromisoformat(str(v)) | ||
| except Exception: | ||
| return _default_graded | ||
| sample_weights = np.array([ | ||
| np.exp(-0.01 * max((now_utc - ( | ||
| r[2] if isinstance(r[2], datetime.datetime) | ||
| else datetime.datetime.fromisoformat(str(r[2])) | ||
| ).replace(tzinfo=None)).days, 0)) | ||
| np.exp(-0.01 * max((now_utc - _parse_graded_at(r[2]).replace(tzinfo=None)).days, 0)) | ||
| for r in rows | ||
| ], dtype=np.float32) |
There was a problem hiding this comment.
The sample_weights calculation will raise a TypeError because it attempts to subtract an offset-naive datetime from an offset-aware one. now_utc is initialized as an aware datetime (UTC), but the expression _parse_graded_at(r[2]).replace(tzinfo=None) explicitly produces a naive datetime.
To fix this, you should make now_utc naive before using it in the subtraction.
| now_utc = datetime.datetime.now(datetime.timezone.utc) | |
| _default_graded = now_utc - datetime.timedelta(days=30) # PR #575: default for NULL graded_at | |
| def _parse_graded_at(v): | |
| if v is None: | |
| return _default_graded | |
| if isinstance(v, datetime.datetime): | |
| return v | |
| try: | |
| return datetime.datetime.fromisoformat(str(v)) | |
| except Exception: | |
| return _default_graded | |
| sample_weights = np.array([ | |
| np.exp(-0.01 * max((now_utc - ( | |
| r[2] if isinstance(r[2], datetime.datetime) | |
| else datetime.datetime.fromisoformat(str(r[2])) | |
| ).replace(tzinfo=None)).days, 0)) | |
| np.exp(-0.01 * max((now_utc - _parse_graded_at(r[2]).replace(tzinfo=None)).days, 0)) | |
| for r in rows | |
| ], dtype=np.float32) | |
| now_utc = datetime.datetime.now(datetime.timezone.utc).replace(tzinfo=None) | |
| _default_graded = now_utc - datetime.timedelta(days=30) # PR #575: default for NULL graded_at | |
| def _parse_graded_at(v): | |
| if v is None: | |
| return _default_graded | |
| if isinstance(v, datetime.datetime): | |
| return v | |
| try: | |
| return datetime.datetime.fromisoformat(str(v)) | |
| except Exception: | |
| return _default_graded | |
| sample_weights = np.array([ | |
| np.exp(-0.01 * max((now_utc - _parse_graded_at(r[2]).replace(tzinfo=None)).days, 0)) | |
| for r in rows | |
| ], dtype=np.float32) |
| import base64 as _b64 # noqa: PLC0415 | ||
| _model_bytes = pickle.dumps(model) | ||
| _model_b64str = _b64.b64encode(_model_bytes).decode("utf-8") |
There was a problem hiding this comment.
Changing the persistence format to a base64-encoded pickle string in the model_json column introduces an incompatibility with the existing model loader _load_xgb_model (line 1692), which expects this column to contain a raw XGBoost JSON string. While xgb_k_layer might expect a pickle, the global model used by agents will fail to load after a system restart.
Consider maintaining compatibility by storing the model in the format expected by the primary loader, or ensuring that _load_xgb_model is updated to handle the new format.
1 participant
Root Cause Analysis
xgb_model_storehas been empty since day 1 — XGBoost has never successfully trained.Three compounding bugs in
run_xgboost_tasklet():Bug 1:
graded_at = NULLcrash (seed rows) — the smoking gun885,672 seed rows have
graded_at = NULL. The sample_weights calculation does:datetime.datetime.fromisoformat(str(None))→fromisoformat("None")→ ValueError every 2:30 AM.Bug 2: Training on 885K synthetic rows instead of 86 real ones
Seed rows have
model_prob = 55.0(league fallback) for ALL rows — pure noise. Live rows have realfeatures_jsonarrays, realmodel_prob(68.3% avg), and 59.3% win rate.Bug 3: DB persist reads from ephemeral JSON file
with open(model_path, "r")reads from/app/api/models/prop_model_v1.json— fails on Railway (ephemeral FS), leavingxgb_model_storeempty after every restart.Fixes (4 surgical edits)
AND agent_name NOT ILIKE '%seed%'— train on 86 real legs only< 200→< 50— 86 rows now qualify_parse_graded_at()helper — NULL-safe graded_at (returns 30-days-ago for NULL)base64(pickle(model))— no file path dependencyAfter this PR
2:30 AM retrain tonight trains on real data →
xgb_model_storepopulates → K-blend + hit-blend activate.Summary by cubic
Fixes nightly XGBoost training by excluding seed rows, handling NULL
graded_at, and persisting the model in Postgres as a base64pickle. Training now uses 86 live legs andxgb_model_storewill populate and survive restarts.agent_name NOT ILIKE '%seed%'; lower training minimum from 200 to 50 so 86 real rows qualify._parse_graded_at()handles NULL/invalid values and defaults to 30 days ago for recency weights.pickletoxgb_model_store(addsprop_typeandn_samples) and keep only the last 3 models.Written for commit 7bdd109. Summary will update on new commits. Review in cubic