Replace kplib alias with direct knowledge_schema symbol imports - #300
Merged
Bernhard Merkle (bmerkle) merged 1 commit intoSep 23, 2026
Merged
Conversation
knowledge_schema exports classes only, so per the import style guidelines in AGENTS.md this is a direct-symbol-import case. Drops the confusing `as kplib` alias flagged in microsoft#112 and microsoft#298 in favor of `from .knowledge_schema import ConcreteEntity, Action, ...` at each call site. Mechanical change only; no behavior change.
Contributor
There was a problem hiding this comment.
🟢 Ready to approve
The changes are mechanical import refactors with consistent symbol updates and no remaining kplib references detected in code.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR standardizes imports of typeagent.knowpro.knowledge_schema by removing the knowledge_schema as kplib alias and switching call sites to direct symbol imports for the schema’s class-only exports, aligning with the documented import-style guidance (Phase 1 of the broader import-consistency cleanup).
Changes:
- Replaces
from ... import knowledge_schema as kplibwithfrom ...knowledge_schema import ...across production, tests, and tool scripts. - Updates all
kplib.Xreferences (type annotations andisinstancechecks) to use the directly imported classes. - Minor import-block consolidation/formatting adjustments resulting from the mechanical replacement.
File summaries
| File | Description |
|---|---|
| tools/query.py | Switches kplib.* schema usage to direct knowledge_schema class imports. |
| tools/benchmark_semref_writes.py | Updates synthetic knowledge construction to use direct schema class imports. |
| tests/test_storage_providers_unified.py | Replaces kplib.* knowledge objects with directly imported schema classes. |
| tests/test_related_terms_index_population.py | Updates entity construction and isinstance checks to direct schema imports. |
| tests/test_property_index_population.py | Updates entity/action/facet construction to direct schema imports. |
| tests/test_podcasts.py | Updates extractor return type and response construction to KnowledgeResponse direct import. |
| tests/test_memory_semrefindex.py | Replaces kplib.* schema references with direct imports in semref index tests. |
| tests/test_knowledge.py | Updates mock extractor typing/construction to KnowledgeResponse direct import. |
| tests/test_add_messages_streaming.py | Updates empty knowledge constant and extractor typing to KnowledgeResponse direct import. |
| tests/test_add_messages_pipeline.py | Updates helper knowledge builders/types to direct schema imports. |
| src/typeagent/storage/memory/semrefindex.py | Replaces schema types (KnowledgeResponse, Action, etc.) with direct imports in index pipeline APIs. |
| src/typeagent/storage/memory/propindex.py | Updates schema typing and isinstance checks to direct imports. |
| src/typeagent/knowpro/universal_message.py | Updates message knowledge construction/typing to direct schema imports. |
| src/typeagent/knowpro/serialization.py | Replaces TYPE_MAP schema class references with direct imports. |
| src/typeagent/knowpro/knowledge.py | Updates knowledge extraction/merge typing to direct schema imports. |
| src/typeagent/knowpro/interfaces_core.py | Updates core protocols/type aliases to refer to direct schema imports. |
| src/typeagent/knowpro/convknowledge.py | Updates translator typing/schema reference to KnowledgeResponse direct import. |
| src/typeagent/knowpro/conversation_base.py | Updates extracted-knowledge typing and isinstance checks to direct schema imports. |
| src/typeagent/knowpro/add_messages.py | Updates empty knowledge constant and typing to KnowledgeResponse direct import. |
| src/typeagent/emails/email_message.py | Updates email-derived knowledge typing/construction to direct schema imports. |
Review details
- Files reviewed: 20/20 changed files
- Comments generated: 0
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
This was referenced Aug 1, 2026
Collaborator
Author
|
robgruen would you mind taking a look at this one when you get a chance? Thanks! |
Collaborator
Author
|
ping robgruen :-) |
Collaborator
|
sorry for the delay, looks good! |
robgruen
approved these changes
Sep 23, 2026
Bernhard Merkle (bmerkle)
added a commit
to bmerkle/typeagent-py
that referenced
this pull request
Sep 26, 2026
interfaces.py is a Protocol/dataclass/type-alias-only aggregator (explicitly re-exporting via __all__, exempted from the "no re-export" rule by AGENTS.md), so per the import style guidelines it defaults to direct-symbol import -- same reasoning as the knowledge_schema/kplib fix in microsoft#300. The storage/sqlite/ subpackage was the lone holdout using `import interfaces` + qualified access (7/7 files, internally consistent but diverging from the other 41 files in the codebase, which already import symbols directly). Converts all 7: collections.py, messageindex.py, propindex.py, provider.py, reltermsindex.py, semrefindex.py, timestampindex.py. Found via a whole-project import-style scan, tracked in microsoft#298. The related aggregator-bypass finding (a few files partially importing from .interfaces_core/.interfaces_storage instead of the aggregator) is a separate concern, deferred.
1 task done
Bernhard Merkle (bmerkle)
added a commit
that referenced
this pull request
Sep 28, 2026
) ## Summary - `knowpro/interfaces.py` is a Protocol/dataclass/type-alias-only aggregator (explicitly re-exporting via `__all__` from `interfaces_core`/`interfaces_indexes`/`interfaces_search`/`interfaces_serialization`/`interfaces_storage` — exempted from AGENTS.md's "no re-export" rule by its own exception clause). Per the import style guidelines in #299/AGENTS.md, a classes-only module like this defaults to direct-symbol import — same reasoning as the `knowledge_schema`/kplib fix in #300. - `storage/sqlite/` was the lone subpackage using `import interfaces` + qualified access (7/7 files, internally consistent with itself but diverging from the other 41 files in the codebase, which already import symbols directly from `interfaces`). - Converts all 7: `collections.py`, `messageindex.py`, `propindex.py`, `provider.py`, `reltermsindex.py`, `semrefindex.py`, `timestampindex.py` — ~42 symbol usages total. - `storage/sqlite/provider.py` had two separate `from ...knowpro.interfaces import ...` lines (one pre-existing direct, one newly added by this change) — isort merged them into a single import block. - Found via a whole-project import-style scan, tracked in #298. **Deferred, separate concern** (not part of this PR): a few files (`conversation_base.py`, `add_messages.py`, `storage/sqlite/provider.py`, `podcast_ingest.py`) partially bypass the `interfaces` aggregator, importing some symbols from `.interfaces` and others directly from `.interfaces_core`/`.interfaces_storage` in the same file. That's a different axis of inconsistency (aggregator vs. defining-submodule) — tracked in #298 as a follow-up, not addressed here. ## 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
knowledge_schemaexports classes only (Quantity,Quantifier,Facet,ConcreteEntity,ActionParam,Action,KnowledgeResponse), so per the import style guidelines in Document Python import style guidelines in AGENTS.md #299/AGENTS.md this is a direct-symbol-import case.from . import knowledge_schema as kplibalias (flagged as a smell in #112) everywhere it appears — 10 files insrc/typeagent, 8 test files, 2 tool scripts — replacingkplib.Xwith a directfrom .knowledge_schema import X, Y, Zimport of the symbols actually used at each call site.Test plan
make format check test— 0 pyright errors (Python 3.12 and 3.14), 737 passed / 12 skipped