Fix TDLib auth recovery and full-history analytics#3
Conversation
crimeacs
left a comment
crimeacs
left a comment
There was a problem hiding this comment.
Ruff passes, and a clean full test run passes (47 tests). The hosted CI run is currently action_required with no jobs. I reproduced the three blockers below, so this head should not merge yet. Please fix the authorization-state freshness race and make the full-history/snapshot reads genuinely complete (or explicitly report truncation), with boundary regressions.
| self._last_authorization_state | ||
| and self._last_authorization_state.get("@type") != current_type | ||
| ): | ||
| return self._last_authorization_state |
There was a problem hiding this comment.
[P1] Wait for a state observed after the transient response. current_type comes from the current synchronous getAuthorizationState response, but _last_authorization_state can predate it. With cached authorizationStateReady and current authorizationStateLoggingOut, this branch returns Ready immediately; I reproduced configure() falsely reporting Ready and skipping the restart. Do not accept an arbitrary differing cached value—consume an update known to be newer than the observed transient state (for example with a generation/sequence), and add that stale-cache regression.
| return count | ||
|
|
||
| def list_posts(self, channel: str, *, limit: int = 1000) -> list[dict[str, Any]]: | ||
| def list_posts(self, channel: str, *, limit: int = 10000) -> list[dict[str, Any]]: |
There was a problem hiding this comment.
[P1] Do not replace the old cap with another silent cap for a “full-history” path. This default still emits LIMIT 10000; a store with 10,001 posts returns 10,000, so no-argument callers such as briefing and ideas silently omit rows and under-report availability. list_comments has the same issue, while some CLI paths retain explicit 5k/10k caps. Add a genuinely unbounded/paginated read for complete analytics (or explicit truncation metadata), and test beyond the new boundary rather than only 1,001 rows.
| def run(): | ||
| store = _store(ctx) | ||
| posts = filter_posts_by_period(store.list_posts(channel, limit=5000), period) | ||
| posts = filter_posts_by_period(store.list_posts(channel, limit=10000), period) |
There was a problem hiding this comment.
[P1] Fetch snapshots consistently with the widened post set. This now analyzes up to 10,000 posts, but list_post_snapshots() returns only the oldest 10,000 snapshots globally. With two captures of 10,000 posts, 20,000 snapshots exist yet only the first capture is returned, so share_velocity_analysis() reports zero delta intervals despite valid newer data. Query all relevant snapshots (or an adequate recent history per selected post) and add a boundary regression.
2 participants
Summary
--question-limittocomments mineTesting
.venv/bin/ruff check ..venv/bin/pytest -q— 47 passed