Code Quality & Cleanup (from TODO.md)

[bmerkle]

In this issue we track possible Code Quality & Cleanup Items from TODO.md

Code Quality & Cleanup

P1 - High Priority

• - [P1, large] Make coding style more uniform (e.g. docstrings)
• - [P1, large] Reduce code size

P2 - Medium Priority

• - [P2, small] Avoid most inline imports: fixed Refactoring inline imports and resolve circular dependencies #113
• - [P2, medium] Break cycles by moving things to their own file if necessary fixed Refactoring inline imports and resolve circular dependencies #113
• - [P2, medium] Unify or align or refactor VectorBase and EmbeddingIndex
• - [P2, medium] Address TODO comments (too numerous)
• - [P2, medium] Address raise NotImplementedError("TODO") (five found) -- implement it

P3 - Low Priority

• - [P3, medium] Change inconsistent module names (Claude uses different naming style)
• - [P3, medium] Rewrite podcast parsing without regexes
• - [P3, medium] Switch from Protocol to ABC
• - [P3, medium] Reduce duplication between ingest_vtt.py and typeagent/transcripts/
• - [P3, small] Rename kplib.py to something ending in _schema.py

[bmerkle]

If you want to try your hand at some items from TODO.md, maybe these make decent starter issues:

Code Quality & Cleanup

P2 - Medium Priority

• [P2, small] Avoid most inline imports
• [P2, medium] Break cycles by moving things to their own file if necessary

I'd start with the former but back out whenever moving an inline import reveals a circular dependency -- for those we often need to do the latter, which is much more subtle and requires a deep understanding of the architecture in order to move the right things into separate files.

I'm not in a hurry with the development infra items (all P3s anyways). Feel free to document the test/build process. The release process I'd rather document myself because I think there are actually some improvements needed for that.

Originally posted by Guido van Rossum (@gvanrossum) in #45

[bmerkle]

For moving inline imports to the top of the file, I'd try one file at a time under typechat. There are some traps where these are needed to avoid circular imports, but many are undoubtedly due to Claude's preference for that style, which is not mine.

For the tests I think you should be able to move all imports out, since there are no circular imports between tests and everything else ought to be importable in any order.

...

I think I understand what you mean about "lost meta information", but it's not typeagent's fault -- it's that email tools (because they predate the Internet) work using conventions that are easy for humans to understand but less so for simplistic software. An LLM would have no problem figuring out who said what, but that's yet another LLM roundtrip.

...

You can use whatever tools you like, including LLMs, but for those I'd like to know about it (and ideally which one and version), and I urge you to review their output carefully before submitting the PR. (You can also make it a Draft PR so I'll know you're still working on it.)

Originally posted by Guido van Rossum (@gvanrossum) in #45

[bmerkle]

I will focus on

P2 - Medium Priority

• [P2, small] Avoid most inline imports
• [P2, medium] Break cycles by moving things to their own file if necessary

[gvanrossum]

Thanks for the reorg!

[bmerkle]

okay, i worked with VSCode and GPT-5-Codex on the refactoring, pushed all modifications to a branch.
all tests are running (except the mcp server where i have some communications problem)

• all inline imports are removed
  (I am embarrassed, I didn't know anything about inline imports. I come from C++ and always imported “at the top.”)

• formatted imports and from imports
  (sorted, arranged etc, via ruff tool) ruff check --select I --fix .

• circular dependecies are removed, a new types.py file was introduced

• searchlib.py improved handling of pydantic classes

• note that the patch set is quite large (99 files) but i looked through to assert that nothing broke.

• unit tests ran fine. (except mcp server)

[shreejaykurhade]

I would like to take up [P3, medium] Change inconsistent module names (Claude uses different naming style).

[bmerkle]

I would like to take up [P3, medium] Change inconsistent module names (Claude uses different naming style).

you can create a PR and i will have a look on it. Please keep PR as small as possible.

[shreejaykurhade]

yess

[gvanrossum]

Hm. I think we must make a plan for what renames we want, rather than letting Claude pick them. Maybe it's the Claude-picked names we want to change! And we must not worry about backward compatibility. And in the code, we must change all the module references -- I don't like doing "import new_module as oldmod" and then keep using "oldmod.thing" in the code. It should be "new_module.thing".

In the end we may decide that it's too much change and we stick with the old module names despite some naming inconsistency.

Shreejay Kurhade (@shreejaykurhade) Please discuss.

[shreejaykurhade]

I am new to this repo. I noticed a few things:

1. Imports of typeagent are made and where previously different. we are changing references of it as using it. I saw this with typeagent imports
2. As the architecture is vast i use claude sonnet 4.6 and codex 5.4 to understand the design and architecture of different modules and parameters.
3. It gets confused and tries to resolve this.
4. goes offrail to solve this and wastes time and precious tokens.(there are naming inconsistencies which it notices and tries to resolve when is runs the tests)
5. I think it will take time if we want to change the old module naming thingy but it will save a lot of time and effort.

I agree.
Jus my thought much easier for peeps like me to understand quicker too.
"Plan" I would love that. to dissect the modules and rename them I would learn more about the repo's implementation too.
I don't think claude should decide the names I think module wise dissection and naming can be done using manually and by claude(checking inconsistencies) and likely drafts can be made to rename the modules. Guido van Rossum (@gvanrossum)

[gvanrossum]

Shreejay Kurhade (@shreejaykurhade) -- Would you like to start with a draft list of modules to rename and propose new names? I can then review the list and we can quickly come to an agreement that way.

Just put the list in a comment. It will cost no tokens.

[shreejaykurhade]

yes, will start

[shreejaykurhade]

Guido van Rossum (@gvanrossum)
I do not see many literal import new_module as oldmod compatibility shims.
But I do see one strong alias smell: repeated knowledge_schema as kplib.
And I do see a lot of direct imports/usages of abbreviated module names like convsettings, convknowledge, secindex, semrefindex, propindex, reltermsindex, and textlocindex, which means any rename plan needs coordinated call-site updates across a lot of files.

The biggest one is that the inconsistency is not only in filenames, it’s also in import style. Some places import modules and use module-qualified names like secindex.build_secondary_indexes(...), while other places import symbols directly, like from .knowledge_schema import ConcreteEntity. That makes the codebase feel uneven even before any rename happens. If you do a rename pass, it would be a good moment to also pick one import style per module family.

below for typeagent folder.

knowledge_schema

Three different styles are in use.

Module alias style: knowledge_schema as kplib

• src/typeagent/emails/email_message.py:10
• src/typeagent/knowpro/conversation_base.py:22
• src/typeagent/knowpro/convknowledge.py:8
• src/typeagent/knowpro/interfaces_core.py:22
• src/typeagent/knowpro/knowledge.py:11
• src/typeagent/knowpro/serialization.py:27
• src/typeagent/knowpro/universal_message.py:11
• src/typeagent/storage/memory/propindex.py:7
• src/typeagent/storage/memory/semrefindex.py:11

Direct symbol import style

• src/typeagent/knowpro/answers.py:33
• src/typeagent/knowpro/query.py:47
• src/typeagent/knowpro/search.py:27

Then module-qualified usage via alias appears throughout those files:

• kplib.ConcreteEntity
• kplib.Action
• kplib.KnowledgeResponse
• kplib.Facet

This is the clearest mixed-style family in the repo.

secindex

Two styles are in use.

Module import + module-qualified calls

• src/typeagent/knowpro/conversation_base.py:20
• src/typeagent/knowpro/factory.py:6
• src/typeagent/podcasts/podcast.py:12
• src/typeagent/storage/memory/semrefindex.py:12
• src/typeagent/transcripts/transcript.py:11

Direct symbol import style does not appear much for secindex, but the inconsistency is that some related helper/index modules are module-qualified while others are imported by symbol.

Examples of module-qualified use:

• src/typeagent/knowpro/conversation_base.py:99
• src/typeagent/podcasts/podcast.py:142
• src/typeagent/storage/memory/semrefindex.py:699

convknowledge

Mostly module import + module-qualified use.

• src/typeagent/knowpro/conversation_base.py:15
• src/typeagent/knowpro/knowledge.py:10
• src/typeagent/storage/memory/semrefindex.py:10

Examples:

• src/typeagent/knowpro/conversation_base.py:216
• src/typeagent/knowpro/knowledge.py:18
• src/typeagent/storage/memory/semrefindex.py:729

This one is internally consistent by itself, but inconsistent relative to families like knowledge_schema, which are often direct-imported.

propindex

Mixed styles.

Module import + module-qualified use

• src/typeagent/knowpro/conversation_base.py:243
• usage at src/typeagent/knowpro/conversation_base.py:245

Direct symbol import style

• src/typeagent/knowpro/query.py:11
• src/typeagent/knowpro/searchlang.py:34
• src/typeagent/knowpro/searchlib.py:11
• src/typeagent/knowpro/secindex.py:5
• src/typeagent/storage/memory/provider.py:24
• src/typeagent/storage/sqlite/propindex.py:49
• src/typeagent/storage/sqlite/propindex.py:80
• src/typeagent/storage/sqlite/provider.py:16
• src/typeagent/storage/sqlite/__init__.py:8

So this family is definitely mixed.

reltermsindex

Mostly direct symbol import style.

• src/typeagent/knowpro/search.py:8
• src/typeagent/knowpro/secindex.py:6
• src/typeagent/storage/memory/provider.py:25
• src/typeagent/storage/sqlite/provider.py:17
• src/typeagent/storage/sqlite/__init__.py:10

No obvious module-qualified style showed up here in src/typeagent, but it sits next to families that do use module-qualified imports.

semrefindex

Mixed styles.

Module import style

• src/typeagent/knowpro/conversation_base.py:24

Direct symbol import style

• src/typeagent/mcp/server.py:33
• src/typeagent/storage/memory/provider.py:26
• src/typeagent/storage/sqlite/provider.py:24
• src/typeagent/storage/sqlite/__init__.py:12

Examples:

• module-qualified usage in src/typeagent/knowpro/conversation_base.py:113
• direct type import in src/typeagent/mcp/server.py:33

textlocindex

Direct symbol import style only in the places I found.

• src/typeagent/storage/memory/messageindex.py:19
• src/typeagent/storage/sqlite/messageindex.py:16

[bmerkle]

thanks for you analysis Shreejay Kurhade (@shreejaykurhade) , I will have a closer look to this topic.
the rename will be a larger refactoring, I will try if we do this via AI prompt or via IDE support.
My experience is that in these large rename operations / refactoring often a classic IDE (like pycharm or others) are faster and more consistent.
Also regarding the PR i am not sure if it is effective if you do this and then we get a large PR to review. We might do this internally as it is easier but will take your suggestions and hints from the analysis and comments into consideration.
I hope this would be ok for you ?

[shreejaykurhade]

Yes, I am fine with it.