ChatStream moves to a light module so chat()'s type hints resolve at runtime - #471
Merged
Conversation
…ve at runtime ChatStream was importable in client.py only under TYPE_CHECKING (a real import would have dragged local_chat's asyncio stack into `import pageindex`), which left chat()'s return annotation a dangling string: typing.get_type_hints(PageIndexClient.chat) raised NameError, and so did anything that introspects signatures — agents' function_tool (client.chat) died on it before looking at a single parameter. The class touches neither asyncio nor the agent frameworks, so it moves to pageindex/chat_stream.py, client.py imports it for real, and the package exports it directly instead of lazily. `import pageindex` still leaves local_chat unloaded. Claude-Session: https://claude.ai/code/session_01PYr9yG1FPQxKCA9m7ECQWY
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| extra_headers: Optional[dict[str, str]] = None, | ||
| extra_body: Optional[dict[str, Any]] = None, | ||
| ) -> "ChatStream": ... | ||
| ) -> ChatStream: ... |
| extra_headers: Optional[dict[str, str]] = None, | ||
| extra_body: Optional[dict[str, Any]] = None, | ||
| ) -> Union[str, "ChatStream"]: ... | ||
| ) -> Union[str, ChatStream]: ... |
| extra_headers: Optional[dict[str, str]] = None, | ||
| extra_body: Optional[dict[str, Any]] = None, | ||
| ) -> Union[str, "ChatStream", dict[str, Any], Iterator[Any]]: ... | ||
| ) -> Union[str, ChatStream, dict[str, Any], Iterator[Any]]: ... |
rejojer
added a commit
that referenced
this pull request
Sep 3, 2026
…s, a guarded eager path, the old import path pinned Review of #471 found the move's guards thinner than they look: - chat_stream.py had no `from __future__ import annotations`, unlike every sibling module, which made its `-> "ChatStream"` quotes load-bearing: unquoting them — the very edit this move made in client.py, and what `ruff --select UP037 --fix` does — broke `import pageindex` outright. - The import in client.py is the whole fix and reads like a typing-only one; a comment says why it must stay real. - The lazy-import test's denylist named no framework, so agents, litellm, openai or anthropic could join the eager path with a green suite — the cost the module was split out to avoid. - The type-hints walk had no floor, so it could silently stop covering anything, and nothing pinned `pageindex.local_chat.ChatStream`, the path the class shipped under in 0.2.11-0.2.14. - local_chat's module docstring still claimed the class. All four guards mutation-checked red.
…s, a guarded eager path, the old import path pinned Review of #471 found the move's guards thinner than they look: - chat_stream.py had no `from __future__ import annotations`, unlike every sibling module, which made its `-> "ChatStream"` quotes load-bearing: unquoting them — the very edit this move made in client.py, and what `ruff --select UP037 --fix` does — broke `import pageindex` outright. - The import in client.py is the whole fix and reads like a typing-only one; a comment says why it must stay real. - The lazy-import test's denylist named no framework, so agents, litellm, openai or anthropic could join the eager path with a green suite — the cost the module was split out to avoid. - The type-hints walk had no floor, so it could silently stop covering anything, and nothing pinned `pageindex.local_chat.ChatStream`, the path the class shipped under in 0.2.11-0.2.14. - local_chat's module docstring still claimed the class. All four guards mutation-checked red.
| method's hints must resolve, ChatStream included.""" | ||
| import inspect | ||
| import typing | ||
| import pageindex |
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
1 participant
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.
ChatStreamwas importable inclient.pyonly underTYPE_CHECKING(a real import would have draggedlocal_chat's asyncio stack intoimport pageindex), which leftchat()'s return annotation a dangling string:typing.get_type_hints(PageIndexClient.chat)raisedNameError, and so did anything that introspects signatures.agents.function_tool(client.chat)died on it before looking at a single parameter.The class touches neither asyncio nor the agent frameworks, so it moves to
pageindex/chat_stream.py;client.pyimports it for real, and the package exports it directly instead of lazily.import pageindexstill leaveslocal_chatunloaded, and import cost is unchanged.A review of the first commit found the move's own guards thinner than they looked, fixed in the second:
chat_stream.pyhad nofrom __future__ import annotations, unlike every sibling module, so its-> "ChatStream"quotes were load-bearing: unquoting them — the very edit this move makes inclient.py— brokeimport pageindexoutright.client.pyis the whole fix and reads like a typing-only one, so a comment says why it must stay real.agents,litellm,openaioranthropiccould join the eager path with a green suite — the cost this module was split out to avoid.pageindex.local_chat.ChatStream, the path the class shipped under in 0.2.11-0.2.14.local_chat's module docstring still claimed the class.Sits on the review view #457. The
client.pyhunk was merged by hand onto main'schat()signatures (#460), keeping main'sTYPE_CHECKINGimport, which its runtime-only__getattr__still uses.Verification: 476 passed; whole suite with
openai-agentsblocked 370 passed / 106 skipped; pyright 0 on the touched files, package unchanged (236). The type-hints test is red on base (NameError), and all four new guards were mutation-checked red. Notefunction_tool(client.chat)also needsstrict_mode=False:chat()'s object-typed parameters are whatpageindex/integrations/openai_agents.pyexists to work around, and that is unchanged here.