Python: fix(redis): type-check the history provider across the supported redis range - #7604
Open
Chinmay V (chinmayv095) wants to merge 1 commit into
Open
Conversation
…ted redis range The dependency-range validator fails pyright on agent-framework-redis at redis 8.0.1 with five unnecessary-type-ignore errors. Those same ignores are required at redis 7.1.1 and 6.4.0, where redis-py annotates the asyncio commands as returning the sync/async union, so deleting them is not a fix either: no single ignore comment satisfies the whole supported range. Normalise the affected results through one helper that accepts the union, and route lrange through an explicitly Any-typed client so the call site does not depend on how precisely redis-py annotates it. Pyright is now clean at 6.4.0, 7.1.1 and 8.0.1.
Chinmay V (chinmayv095)
deployed
to
github-app-auth
August 10, 2026 18:31 — with
GitHub Actions
Active
Chinmay V (chinmayv095)
deployed
to
github-app-auth
August 10, 2026 18:31 — with
GitHub Actions
Active
Chinmay V (chinmayv095)
deployed
to
github-app-auth
August 10, 2026 18:31 — with
GitHub Actions
Active
Chinmay V (chinmayv095)
deployed
to
github-app-auth
August 10, 2026 18:32 — with
GitHub Actions
Active
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes Redis history-provider type checking across supported redis-py versions without changing runtime behavior.
Changes:
- Adds a helper that normalizes awaitable and synchronous Redis results.
- Removes version-dependent type-ignore comments and adds focused tests.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
python/packages/redis/agent_framework_redis/_history_provider.py |
Normalizes Redis command results across annotation versions. |
python/packages/redis/tests/test_providers.py |
Tests both helper branches and synchronous lrange results. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation & Context
The weekly dependency-range validator reported in #7340 that
agent-framework-rediscannot move past its currentredisbound because pyright fails atredis==8.0.1with fivereportUnnecessaryTypeIgnoreCommenterrors in_history_provider.py.Removing those five comments does not fix it. I reproduced the validator locally and checked each version in the supported range:
The cause is redis-py itself. On the older releases the asyncio client annotates
lrange,rpush,llenandltrimas returningAwaitable[T] | T, soawait-ing them directly does not type-check ("int" is not awaitable) and the ignore is required. Newer releases narrow those annotations to the awaitable alone, which makes the very same comment unnecessary. No single# type: ignorecomment satisfies the whole supported range, which is why this needs a code change rather than a comment sweep.This matters beyond the bound bump:
reportUnnecessaryTypeIgnoreComment = "error"is enabled precisely so these comments do not accumulate, and today five of them are load-bearing on one redis version and dead weight on another.Description & Review Guide
What are the major changes?
_redis_resulthelper accepts theAwaitable[T] | Tunion and returnsT. It usesisawaitable, matching the existing pattern inagent_framework/_types.py.rpush,llenandltrimnow go through it and carry no ignore comment.lrangeis a slightly different case: it is partially unknown on the older annotations rather than merely union-typed, so even the helper leaves a strict-mode complaint about the argument expression. It now reads through an explicitlyAny-typed local, which makes the call site independent of how precisely redis-py annotates it, with acastpinning thelist[str]thatdecode_responses=Trueguarantees. That also removes the twounion-attrignores on the loop below it.if redis_messages:guard was dropped as dead — iterating an empty list is already a no-op.What is the impact of these changes?
_redis_resultawaits exactly what the previousawaitawaited; the sync arm only exists to satisfy the union that redis-py declares.redis>=8, and I want to flag why: the package also pinsredisvl>=0.11.0,<0.16, andredisvl 0.15.0requiresredis<7.2,>=5.0. So redis 8.x is not co-installable with the currentredisvlpin regardless of typing, and the validator's proposedredis<8.0.0is already in tension with it. Raisingredisvlis a separate call I have deliberately left to you — I have not touched any bound in this PR.What do you want reviewers to focus on?
client: Anyescape hatch forlrangeis acceptable, or whether you would rather keep an ignore there and accept one error on newer redis. I went this way because it is the only form I found that is clean on all three versions, but it is a real trade of local type precision and it is your call._redis_resultbelongs in this module or in a shared location, sinceagent_framework_redisis not the only place that awaits redis-py commands.Related Issue
Fixes #7340
Verification
--project pyproject.toml, strict, against each installed redis): 0 errors at 6.4.0, 7.1.1 and 8.0.1 on this branch. The table above is the before/after.packages/redisonmain45 passed / 0 failed / 0 errors, branch 48 passed / 0 failed / 0 errors. The FAILED/ERROR sets are identical (both empty) and the delta is exactly the three new tests.ruff format --diffandruff checkclean on both touched files.Three tests added, all offline:
test_returns_messages_when_lrange_is_synchronousget_messages, which the existingAsyncMock-based tests never exerciseTestRedisResultHelper::test_awaits_an_awaitable_resultTestRedisResultHelper::test_passes_through_a_plain_resultOverlap note: #7470, also mine, touches
save_messagesin this same file. The two changes are independent and either merge order works, but whichever lands second will want a trivial rebase — happy to do that whenever you tell me which you prefer first.Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.