Use module-qualified imports for storage.memory.propindex - #302
Merged
Bernhard Merkle (bmerkle) merged 1 commit intoSep 28, 2026
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Aligns storage.memory.propindex usage with the repository’s module-qualified import style by switching low-frequency function/class imports to propindex.<symbol> while keeping PropertyNames as a direct import where it’s used heavily.
Changes:
- Switched call sites to
from ...storage.memory import propindexand updated calls to be module-qualified (propindex.*). - Updated in-memory provider to construct and type-annotate the property index via
propindex.PropertyIndex. - Updated sqlite property index to call
make_property_term_text/split_property_term_textvia the module import.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/typeagent/storage/sqlite/propindex.py | Replaces direct function imports from memory propindex with a module import and qualified calls. |
| src/typeagent/storage/memory/provider.py | Uses module-qualified propindex.PropertyIndex for instantiation and type annotation. |
| src/typeagent/knowpro/secindex.py | Uses propindex.build_property_index(...) instead of importing the function directly. |
| src/typeagent/knowpro/query.py | Uses propindex.lookup_property_in_property_index(...) while keeping PropertyNames as a direct import. |
This was referenced Aug 1, 2026
Collaborator
Author
|
robgruen would you mind taking a look at this one when you get a chance? Thanks! |
propindex (memory variant) exports a mix of an enum, several functions, and a class. Per AGENTS.md's import style guidelines, low-frequency symbols default to module-qualified: - query.py: lookup_property_in_property_index - secindex.py: build_property_index - storage/memory/provider.py: PropertyIndex - storage/sqlite/propindex.py: make_property_term_text, split_property_term_text (cross-imported from the memory variant) PropertyNames is left as a direct-symbol import everywhere (query.py, searchlang.py, searchlib.py): it's used 20+ times across those files, squarely matching the "avoiding severe repetitive visual clutter" exception in AGENTS.md. The sqlite variant's own class (SqlitePropertyIndex) exports a single class and is left as direct-symbol import, already compliant. Scope: src/typeagent only, per microsoft#298. Phase 2 (2/3) of the import-consistency cleanup tracked in microsoft#298.
Bernhard Merkle (bmerkle)
force-pushed
the
fix-import-style-propindex
branch
from
September 26, 2026 07:45
2c21851 to
590c907
Compare
Collaborator
Author
|
robgruen for rewiew/approval: refactorings |
robgruen
approved these changes
Sep 28, 2026
Bernhard Merkle (bmerkle)
added a commit
that referenced
this pull request
Sep 28, 2026
## Summary - `semrefindex` (memory variant) is overwhelmingly function-oriented (18 functions + 1 class), so per the import style guidelines in #299/AGENTS.md 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 for its 4 function usages, so no change was needed there — this PR brings the rest of `src/typeagent` in line with that existing convention. - The sqlite variant's own class (`SqliteTermToSemanticRefIndex`) exports a single class and is left as direct-symbol import — already compliant, no change. - Scope: `src/typeagent` only, per #298. - Phase 2 (3/3, final) of the import-consistency cleanup tracked in #298. Companion PRs: #301 (reltermsindex), #302 (propindex). ## Test plan - [x] `make` (format, check on 3.12/3.14, test, build) — 0 pyright errors, 737 passed / 12 skipped, wheel builds Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>
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
propindex(memory variant) exports an enum (PropertyNames), ~10 functions, and a class (PropertyIndex). Per the import style guidelines in Document Python import style guidelines in AGENTS.md #299/AGENTS.md, low-frequency symbols default to module-qualified:knowpro/query.py:lookup_property_in_property_indexknowpro/secindex.py:build_property_indexstorage/memory/provider.py:PropertyIndexstorage/sqlite/propindex.py:make_property_term_text,split_property_term_text(cross-imported from the memory variant)PropertyNamesis deliberately left as a direct-symbol import everywhere it's used (query.py,searchlang.py,searchlib.py) — it's referenced 20+ times across those files, which squarely matches AGENTS.md's "avoiding severe repetitive visual clutter" exception. Qualifying everyPropertyNames.Xreference would add clutter with no disambiguation benefit.SqlitePropertyIndex) 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