Standardize import style (module-qualified vs direct-symbol) across the codebase

[bmerkle]

Context

Follow-up to #112, specifically the discussion started in this comment (import/naming inconsistency slows down both humans and LLM-assisted contributors trying to understand the codebase) and the detailed per-module audit in this comment (module-qualified vs. direct-symbol import style is mixed across knowledge_schema/kplib, secindex, convknowledge, propindex, reltermsindex, semrefindex, textlocindex).

AGENTS.md now documents the decision rule (added in this effort):

Default to Module-Qualified Imports: Prefer importing whole modules and using qualified calls to prevent namespace pollution, avoid name clashes, and provide immediate context for where functions or objects originate.
Use Direct Symbol Imports Cautiously: Restrict direct imports to specific scenarios: classes/exceptions/constants, avoiding severe repetitive clutter, or standard-library patterns.
Prohibit Wildcard Imports.

This issue tracks applying that rule consistently, based on an AST-based census of actual import sites across src/typeagent, tests/, and tools/.

Current state (as audited)

Module	Exports	Target style	Compliance today
knowledge_schema (kplib)	7 classes only	direct-symbol	❌ split: 20 sites alias it as kplib (10 in src, 10 in tests/tools), 32 sites already direct
secindex	1 class + 2 functions	module-qualified	✅ already 100% qualified in src (6/6); tests use direct-symbol (separate, idiomatic convention)
convknowledge	1 class only	either (already consistent)	✅ already 100% qualified everywhere (4/4)
textlocindex	2 classes only	direct-symbol	✅ already 100% direct everywhere (5/5)
propindex (memory variant)	1 enum + 10 functions + 1 class	module-qualified	❌ opposite of target: 23/25 sites are direct-symbol today
reltermsindex (memory variant)	4 classes + 3 functions	module-qualified	❌ 100% direct-symbol today (14/14)
semrefindex (memory variant)	18 functions + 1 class	module-qualified	❌ only 1/22 sites qualified (conversation_base.py); everything else (incl. add_messages.py, mcp/server.py, storage/memory/provider.py) is direct-symbol
propindex/reltermsindex/semrefindex (sqlite variants)	1 class each	direct-symbol	✅ already 100% direct everywhere

Plan

Phase 0: Land the AGENTS.md rule itself (docs-only).

Phase 1 — knowledge_schema/kplib cleanup (bounded, low-risk, directly matches the issue's original complaint):

• Convert the 20 from . import knowledge_schema as kplib + kplib.X sites to from .knowledge_schema import X, Y, Z, in src/typeagent (10 files) and in tests/tools (10 files, same mechanical fix).
• One PR.

Phase 1b: No code changes needed — secindex, convknowledge, textlocindex are already compliant/internally consistent.

Phase 2 (deferred, separate sign-off after Phase 1 lands): reltermsindex → propindex → semrefindex, each its own PR, in that order (ascending size/centrality — semrefindex touches add_messages.py and conversation_base.py, the highest-churn files in the repo per the code-health hotspot data, so it goes last after the mechanical pattern is validated on the smaller modules).

Scope decision: test/tool files get the mechanical kplib-alias fix (Phase 1) since it's zero-risk, but Phase 2's function-import style is not forced onto test files — importing a function directly to call it in an assertion is idiomatic pytest style and is being treated as its own justified exception, independent of what src/typeagent does.

Non-goals

This issue is about import style consistency only. Actually renaming modules (the original ask in #112, e.g. secindex → something more descriptive) is separate and still awaiting a draft rename list, per gvanrossum's request in #112.

[bmerkle]

Status update: all phases now have PRs open.

• Phase 0: Document Python import style guidelines in AGENTS.md #299 (AGENTS.md guidelines)
• Phase 1: Replace kplib alias with direct knowledge_schema symbol imports #300 (kplib alias removal)
• Phase 2: Use module-qualified imports for storage.memory.reltermsindex #301 (reltermsindex), Use module-qualified imports for storage.memory.propindex #302 (propindex), Use module-qualified imports for storage.memory.semrefindex #303 (semrefindex)

All five validated with full make (format, pyright on 3.12/3.14, test, build) — 0 pyright errors, 737 passed / 12 skipped, wheel builds clean, in each.

One refinement worth calling out: for propindex, the PropertyNames enum (used 20+ times across query.py/searchlang.py/searchlib.py) was deliberately left as a direct-symbol import rather than forced to module-qualified — it matches AGENTS.md's "avoiding severe repetitive visual clutter" exception. Everything else in scope (low-frequency function/class imports) converted to module-qualified per the default rule.

[bmerkle]

Whole-project import-style audit (beyond the 7 families originally flagged)

Ran an AST-based scan across src/typeagent for modules where import style is genuinely mixed among current importers (not just non-compliant with the new default rule, but actually inconsistent site-to-site). Beyond the 3 already covered by open PRs (knowledge_schema #300, propindex #302, semrefindex #303, pending merge), found:

1. knowpro/interfaces.py — structural, large scope

This is an explicit re-export aggregator (from .interfaces_core import * etc. — exempted from the "no re-export" rule by AGENTS.md's own exception clause). Two co-existing, each internally 100%-consistent conventions:

• storage/sqlite/ subpackage (7/7 files): import interfaces then interfaces.IMessage, interfaces.TextLocation, etc. — 3-9 symbols per file this way.
• Everywhere else (41 files): from .interfaces import (A, B, C, ...) direct-symbol, large parenthesized blocks.

Same module, same "many symbols per file" usage shape, split cleanly along package boundaries. Unifying in either direction touches 40+ files — this needs a dedicated plan (queued as a follow-up, not yet scheduled).

2. mcp/server.py — single-file outlier (fix in progress)

Lone outlier for three modules where every other consumer uses direct-symbol import for the same 1-2 symbols:

• embeddings.IEmbeddingModel (vs. 21 files using direct)
• query.QueryEvalContext (vs. 2 files)
• podcast.PodcastMessage/podcast.Podcast (vs. podcast_ingest.py)

Same "lone-outlier" shape already fixed for semrefindex in #303 (same file, in fact). Fix in progress.

3. Not real inconsistencies — two legitimate conventions co-existing

model_adapters, aitools.utils, answer_response_schema, search_query_schema showed up as "mixed" in the mechanical scan, but it's explained by conversation_base.py and email_memory.py both deliberately grouping several sibling knowpro modules into one qualified-import block (from ..knowpro import (answer_response_schema, answers, search_query_schema, searchlang, ...)), while single-purpose files (convknowledge.py, answers.py, searchlang.py) import the one symbol they need directly. Two reasonable, pre-existing conventions, not an accident — no action planned here.

[bmerkle]

Status update: whole-project scan follow-ups now have PRs too

• Use direct-symbol imports for embeddings/query/podcast in mcp/server.py #304 — mcp/server.py outlier fix (embeddings/query/podcast → direct-symbol)
• Use direct-symbol imports for knowpro.interfaces in storage/sqlite #305 — knowpro.interfaces unified to direct-symbol in storage/sqlite/ (7 files, ~42 symbols)

All PRs currently open for this effort: #299 (Phase 0, guidelines), #300 (Phase 1, kplib), #301/#302/#303 (Phase 2, reltermsindex/propindex/semrefindex), #304, #305. All validated independently with full make (format, pyright 3.12+3.14, test, build).

Remaining deferred item: the aggregator-bypass finding (a few files partially importing from .interfaces_core/.interfaces_storage instead of the .interfaces aggregator) — separate concern, not yet scheduled.

[bmerkle]

Aggregator-bypass finding: investigated, fix in progress

Dug into the deferred aggregator-bypass finding (files partially importing from .interfaces_core/.interfaces_storage instead of the .interfaces aggregator). Conclusion: no functional justification, incidental rather than deliberate.

• Not a circular-import workaround. interfaces_core.py, interfaces_indexes.py, interfaces_search.py, interfaces_serialization.py, interfaces_storage.py only depend on knowledge_schema, field_helpers, and aitools.embeddings — none of them import anything from the 4 affected consumer files, so importing the full interfaces aggregator from any of them carries zero cycle risk.
• Not a missing-export issue. Every bypassed symbol (TextLocation, AddMessagesResult, IKnowledgeExtractor, IMessage, MessageOrdinal, ChunkFailure) is confirmed present in the relevant submodule's __all__, which interfaces.py merges and re-exports.
• Git history rules out "predates the aggregator." interfaces.py was created 2025-12-31 (add isort dependency and move typeagent to src #139, the src/ layout migration). The bypass lines were added months later (2026-04-26 through 2026-05-16) across three separate commits (two by Guido van Rossum (@gvanrossum), one by another contributor) whose messages (streaming API, batching/pipeline overlap) don't mention import style at all.

Scope — 4 files, 7 bypass sites:

File	Via aggregator	Bypassed (direct from submodule)
conversation_base.py	large from .interfaces import (...) block	TextLocation from interfaces_core
add_messages.py	AddMessagesResult	IKnowledgeExtractor, IMessage, MessageOrdinal, TextLocation from interfaces_core
storage/sqlite/provider.py	ConversationMetadata, STATUS_INGESTED (+3 more once #305 merges)	ChunkFailure from interfaces_storage
podcast_ingest.py	Datetime	AddMessagesResult from interfaces_core

Small irony: AddMessagesResult goes through the aggregator in add_messages.py but is bypassed in podcast_ingest.py — same symbol, two different paths depending on file.

Fix: merge each bypassed import into the existing (or a new) from .interfaces import (...) block. Mechanical, low-risk, comparable in size to #304. PR incoming.

[bmerkle]

PR up: #306. All findings from this effort now have open PRs: #299 (guidelines), #300 (kplib), #301/#302/#303 (Phase 2), #304 (mcp/server.py), #305 (interfaces qualified→direct), #306 (aggregator bypass). All validated independently with full make.

[bmerkle]

robgruen and/or Guido van Rossum (@gvanrossum) can you review the PR #300 to PR #306 ?
this is a refactoring on importstyle step by step across the modules.
I created separted PRs to isolate changes and keep PRs clean and as small as possible.

once we have fixed the import the next step is to rename the modules. Please see issue #307 for that.
i will wait for feedback on #307 before continuning there. The refeactorings will have no functional change.

Once done that, i would consider some additional cleanup and then release a new version (0.6.0)

[gvanrossum]

I don't have time, sorry. robgruen can you spare some?