fix: predict button, ty diagnostics, and deprecation warnings#6
fix: predict button, ty diagnostics, and deprecation warnings#6davidchris merged 2 commits intomainfrom
Conversation
…ict button bug - Fix predict button disappearing after retraining by exposing classes_ attribute on EnsembleCategorizer (duck-typing with TransactionCategorizer) - Keep predict button always visible when model is loaded - Fix all 197 ty type checker diagnostics with proper code fixes: cast ORM Column attributes, add None guards, fix import paths, use explicit kwargs for FastHTML components, add type narrowing - Replace deprecated datetime.utcnow() with datetime.now(UTC) everywhere, eliminating 1340 pytest DeprecationWarning instances - Fix typo in pyproject.toml ty rule name (non-subscriptable -> not-subscriptable) Closes #2 Closes #3 Closes #4 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bc57b8e07f
ℹ️ 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".
web/pages/settings_page.py
Outdated
| onclick="confirmAndPredictUnpredicted({unpredicted_count})" | ||
| class="inline-flex items-center px-3 py-2 text-sm font-medium text-blue-700 bg-blue-100 rounded-md hover:bg-blue-200" | ||
| title="Run ML predictions on all transactions that don't have predictions yet" | ||
| title="Run ML predictions on transactions without predictions" |
There was a problem hiding this comment.
Route re-predict button to an endpoint that reprocesses rows
This button now appears with the text “Re-predict Transactions” when unpredicted_count is zero, but it still calls confirmAndPredictUnpredicted(...), which posts to /api/ml/predict/batch-unpredicted; that API only handles transactions where predicted_category_id IS NULL (api/ml.py), so in the exact re-predict state it returns predictions_made: 0 and no existing predictions are refreshed after retraining.
Useful? React with 👍 / 👎.
api/services.py
Outdated
|
|
||
| def _to_datetime(value: Any) -> datetime: | ||
| """Cast an ORM column value to datetime.""" | ||
| return datetime(value.year, value.month, value.day, value.hour, value.minute, value.second) |
There was a problem hiding this comment.
Preserve timezone data when converting ORM datetimes
This conversion rebuilds a new datetime from date/time fields and discards timezone and sub-second precision, and it is now used across response payloads for created_at/updated_at; because this same commit also moved timestamp creation to datetime.now(UTC), UTC-aware values can be returned as naive timestamps, causing incorrect interpretation in timezone-aware clients.
Useful? React with 👍 / 👎.
The "Re-predict Transactions" button was a no-op after retraining because it called the batch-unpredicted endpoint which only targets transactions with predicted_category_id IS NULL. Add a dedicated /predict/batch-repredict endpoint that targets unreviewed transactions with existing predictions, and split the UI into separate Predict/Re-predict buttons with correct counts. Also fix _to_datetime in services.py to use typing.cast instead of reconstructing datetime objects, which was losing timezone and sub-second precision. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
EnsembleCategorizernow exposesclasses_attribute (duck-typing withTransactionCategorizer), and the predict button stays visible when a model is loaded (shows "Re-predict Transactions" when count is 0)typing.cast(), None guards, import path corrections (fafycat.→src.fafycat.), explicit kwargs for FastHTML components, and type narrowingdatetime.utcnow()warnings (Fix pytest warnings #2): Replaced withdatetime.now(UTC)across all files, eliminating 1340 pytest DeprecationWarning instancesCloses #2
Closes #3
Closes #4
Test plan
ty checkpasses with 0 diagnosticsruff checkandruff formatpass🤖 Generated with Claude Code