Use module-qualified imports for storage.memory.semrefindex - #303
Open
Bernhard Merkle (bmerkle) wants to merge 1 commit into
Open
Use module-qualified imports for storage.memory.semrefindex#303Bernhard Merkle (bmerkle) wants to merge 1 commit into
Bernhard Merkle (bmerkle) wants to merge 1 commit into
Conversation
semrefindex (memory variant) is overwhelmingly function-oriented (18 functions + 1 class), so per AGENTS.md's import style guidelines it defaults to module-qualified. Converts the remaining low-frequency direct-symbol call sites: - knowpro/add_messages.py: collect_action_terms, collect_entity_terms - mcp/server.py: TermToSemanticRefIndex - storage/memory/provider.py: TermToSemanticRefIndex conversation_base.py already used `from ..storage.memory import semrefindex` + qualified calls, so no change needed there. The sqlite variant's own class (SqliteTermToSemanticRefIndex) exports a single class and is left as direct-symbol import, already compliant. Scope: src/typeagent only, per microsoft#298. Phase 2 (3/3) of the import-consistency cleanup tracked in microsoft#298.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR completes Phase 2 of the import-style standardization effort (#298) by switching remaining low-frequency call sites of the in-memory storage.memory.semrefindex module from direct-symbol imports to module-qualified usage, aligning src/typeagent with the established pattern already used in knowpro/conversation_base.py.
Changes:
- Replace direct function imports (
collect_action_terms,collect_entity_terms) withsemrefindex.<fn>calls in the add-messages pipeline. - Replace direct class import (
TermToSemanticRefIndex) withsemrefindex.TermToSemanticRefIndexin MCP server typing and in-memory storage provider. - Keep sqlite
semrefindexdirect-symbol imports unchanged (single-class export, already consistent).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/typeagent/storage/memory/provider.py | Imports semrefindex as a module and qualifies the TermToSemanticRefIndex type/constructor usage. |
| src/typeagent/mcp/server.py | Switches to module import (semrefindex) and updates the QueryEvalContext type parameter accordingly. |
| src/typeagent/knowpro/add_messages.py | Switches to module import (semrefindex) and qualifies calls to term-collection helpers. |
Collaborator
Author
|
robgruen would you mind taking a look at this one when you get a chance? Thanks! |
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.
Summary
semrefindex(memory variant) is overwhelmingly function-oriented (18 functions + 1 class), so per the import style guidelines in Document Python import style guidelines in AGENTS.md #299/AGENTS.md it defaults to module-qualified.knowpro/add_messages.py:collect_action_terms,collect_entity_termsmcp/server.py:TermToSemanticRefIndexstorage/memory/provider.py:TermToSemanticRefIndexconversation_base.pyalready usedfrom ..storage.memory import semrefindex+ qualified calls for its 4 function usages, so no change was needed there — this PR brings the rest ofsrc/typeagentin line with that existing convention.SqliteTermToSemanticRefIndex) exports a single class and is left as direct-symbol import — already compliant, no change.src/typeagentonly, per Standardize import style (module-qualified vs direct-symbol) across the codebase #298.Test plan
make(format, check on 3.12/3.14, test, build) — 0 pyright errors, 737 passed / 12 skipped, wheel builds