Merged
Conversation
…stic using pydantic_ai
…t-embedding-3-small)
reduce lookup to cache the ordinals_of_subset Total Complexity reduces from O(n*k) to O(n+k)
e.g. didn't match &api-version=... in multi-parameter URLs. Fixed to [?&,].
There was a mutable default argument in collections.py — SemanticRefAccumulator.__init__ used set() as default parameter, shared across all instances. The code has been rewritten to still track which search terms produced hits in the accumulator but in a way to avoid mutable default arguments. Now: __init__ always creates a fresh set() — no parameter Add a with_term_matches factory that creates a new accumulator with a copy of term matches (makes the copy-vs-share explicit) Update group_matches_by_type to use a copy instead of sharing the same set object Update get_matches_in_scope and WhereSemanticRefExpr to use the factory 2nd change: hit_count=1 for non-exact matches in collections.py MatchAccumulator.add — inflated hit counts for related-only matches. Fixed to hit_count=0. added comments to the else branches added additional tests
… the object — including inherited methods, dunder methods (__init__, __eq__, …), and class-level descriptors. The fix switches to vars(self), which returns only the instance's __dict__ — i.e. the actual dataclass field values. added testcases removed unused method from class
— produced dataclass repr strings instead of actual facet values. Off-by-one in get_enclosing_date_range_for_text_range — used exclusive end ordinal directly, potentially indexing past the last message. Fixed to use message_ordinal - 1. added testcases
…ding a full chunk), the merged result would begin with a spurious separator like "\n\n". The fix adds a guard added testcases
The coverage package is a dev/test dependency. "user" role correctly but defaulted everything else to "assistant" — including "system" prompts. MCP's SamplingMessage only accepts "user" or "assistant" roles. A "system" prompt section from TypeChat would silently be sent as "assistant", which changes LLM behavior. added testcases
dunder methods (__init__, __eq__, __hash__, …), and descriptors.
The fix switches to vars(self), which returns only the instance's __dict__ — the actual dataclass field values.
Combined with the not key.startswith("_") and is not None filters, the repr is now clean.
added testcases
…buteError. The fix introduces a local ordinal = 0 counter incremented on each iteration, which works regardless of the message implementation. added message.timestamp null guard fix related to the changes in SemanticRefAccumulator use with_term_matches classmethod factory, which copies the search-term provenance set, so mutations to the filtered accumulator's set don't affect the source. added testcases
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request contains bugfixes and improvements for the upcoming 0.4.0 release. The most significant change is the migration from direct OpenAI SDK usage to a provider-agnostic architecture using pydantic_ai, which enables support for 25+ AI providers (OpenAI, Anthropic, Google, Azure, etc.) through a unified interface. Additionally, the PR fixes numerous bugs across the codebase including string formatting errors, logic bugs, and API design issues.
Changes:
- Introduced provider-agnostic model adapters using pydantic_ai for chat and embedding models
- Fixed multiple critical bugs including provenance copying, facet serialization, and timestamp handling
- Added comprehensive test coverage for new functionality and bug fixes
- Moved pydantic-ai-slim from dev dependencies to runtime dependencies
Reviewed changes
Copilot reviewed 65 out of 66 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| uv.lock | Moved pydantic-ai-slim[openai] from dev to runtime dependencies |
| pyproject.toml | Updated dependency specification for pydantic-ai-slim |
| src/typeagent/aitools/model_adapters.py | New module providing provider-agnostic chat and embedding model adapters |
| src/typeagent/aitools/embeddings.py | Refactored to protocol-based interfaces (IEmbedder, IEmbeddingModel) |
| src/typeagent/aitools/vectorbase.py | Updated to support dynamic embedding size discovery and improved error handling |
| src/typeagent/aitools/utils.py | Fixed regex for parsing Azure endpoints with ampersand-separated query params |
| src/typeagent/knowpro/query.py | Fixed manual ordinal counting and timestamp None guards |
| src/typeagent/knowpro/searchlang.py | Removed duplicate methods, fixed repr, added missing action_group append |
| src/typeagent/knowpro/search.py | Fixed repr to use vars() instead of dir() |
| src/typeagent/knowpro/collections.py | Fixed hit_count initialization for non-exact matches and provenance copying |
| src/typeagent/knowpro/answers.py | Fixed facet value stringification and off-by-one error in date range lookup |
| src/typeagent/knowpro/convknowledge.py | Removed deprecated create_typechat_model function |
| src/typeagent/knowpro/convsettings.py | Updated to use new model adapter interfaces |
| src/typeagent/knowpro/conversation_base.py | Updated to use new chat model creation pattern |
| src/typeagent/knowpro/fuzzyindex.py | Fixed deserialization logic |
| src/typeagent/knowpro/knowledge.py | Removed max_retries parameter |
| src/typeagent/knowpro/serialization.py | Added embedding size metadata to file headers |
| src/typeagent/knowpro/interfaces_storage.py | Removed embedding_size field from ConversationMetadata |
| src/typeagent/knowpro/interfaces_search.py | Removed duplicate all definition |
| src/typeagent/emails/email_import.py | Fixed separator prepending in chunk merging |
| src/typeagent/emails/email_memory.py | Updated to use new chat model creation |
| src/typeagent/mcp/server.py | Added coverage import guard and match statement default case |
| src/typeagent/storage/sqlite/provider.py | Removed embedding_size consistency checks, now checks size consistency between indexes |
| src/typeagent/storage/sqlite/messageindex.py | Added empty chunks guard |
| src/typeagent/transcripts/transcript.py | Fixed f-string formatting and reads embedding size from file metadata |
| src/typeagent/podcasts/podcast.py | Same f-string and metadata reading fixes as transcripts |
| tools/query.py | Updated imports to use new model adapters |
| tools/ingest_vtt.py | Updated to use new embedding model creation |
| tests/test_*.py | Comprehensive test updates to use new interfaces and added extensive new test coverage |
| AGENTS.md | Added deprecation guideline |
Comments suppressed due to low confidence (1)
src/typeagent/knowpro/searchlang.py:626
- The duplicate method
add_search_term_to_groupadd_entity_name_to_groupwith the typo in its name has been removed. This method name appears to be a copy-paste error combining two method names, and its implementation is identical toadd_entity_name_to_groupabove it.
def add_property_term_to_group(
self,
property_name: str,
property_value: str,
term_group: SearchTermGroup,
exact_match_value=False,
) -> None:
if not self.is_searchable_string(property_name):
return
if not self.is_searchable_string(property_value):
return
if self.is_noise_term(property_value):
return
# Dedupe any terms already added to the group earlier.
if not self.dedupe or not self.entity_terms_added.has(
Comment on lines
+118
to
+126
| async def get_embedding_nocache(self, input: str) -> NormalizedEmbedding: | ||
| result = await self._embedder.embed_documents([input]) | ||
| embedding: NDArray[np.float32] = np.array( | ||
| result.embeddings[0], dtype=np.float32 | ||
| ) | ||
| norm = float(np.linalg.norm(embedding)) | ||
| if norm > 0: | ||
| embedding = (embedding / norm).astype(np.float32) | ||
| return embedding |
There was a problem hiding this comment.
The get_embedding_nocache method doesn't validate that the input string is non-empty before calling the embedding API. According to the test test_get_embedding_nocache_empty_input, empty strings should raise a ValueError with "Empty input text". Consider adding validation: if not input: raise ValueError("Empty input text").
gvanrossum
reviewed
Feb 27, 2026
src/typeagent/knowpro/collections.py
Outdated
|
|
||
| class SemanticRefAccumulator(MatchAccumulator[SemanticRefOrdinal]): | ||
| def __init__(self, search_term_matches: set[str] = set()): | ||
| """Accumulates scored semantic reference matches. |
Collaborator
There was a problem hiding this comment.
It will take me a bit more time to review this particular commit more carefully. It seems you're on to something, but I think I don't recall how it originally worked or how it's supposed to work.
Contributor
Author
There was a problem hiding this comment.
I told Claude Opus to have a look at the usage (before i made changes) and it came up with the following:
search_term_matches tracks which search terms produced hits in the accumulator, and it's deliberately propagated to derived accumulators. Here's the pattern:
- Recording: add_term_matches / add_term_matches_if_new call self.search_term_matches.add(search_term.text) after adding scored refs — this records which terms contributed results.
- Propagation to grouped subsets: group_matches_by_type (line 336) does group.search_term_matches = self.search_term_matches — intentionally shares the same set object so all type-grouped sub-accumulators reflect the parent's term matches.
- Propagation to filtered subsets: get_matches_in_scope (line 346) passes self.search_term_matches to the new accumulator's constructor — the filtered result inherits the parent's term match info.
- Merging on union/intersect: add_union and intersect call .update() to merge term matches from both sides.
- Consumption: In query.py, search_term_matches is read at query.py:864 to populate SemanticRefSearchResult.term_matches, which tells the caller which search terms actually matched something.
So this is a bookkeeping/provenance mechanism, not a cache. It answers "which of my search terms actually produced results?" — used downstream when building search results.
Removed redundant import statements for pydantic.dataclasses and typechat.
…ntics - Removed the incorrect - 1 from range.end.message_ordinal. Now fetches the timestamp of the message at the exclusive end ordinal (the first message after the range). Added a bounds check: if the ordinal is past the end of the collection, end is left as None. - fixed test cases - added test for the edge case where the range end is past the last message
- as Python 3.12 does not have aenumerate in its stdlib, added a small help into utils.py
gvanrossum
reviewed
Mar 2, 2026
gvanrossum
reviewed
Mar 2, 2026
Collaborator
gvanrossum
left a comment
gvanrossum
left a comment
There was a problem hiding this comment.
Okay, except for the SemanticRefAccumulator business discussed offline.
- clone() method added - fix for hit_count = 0 aaplied again - renamed is_in_range to contains_range
gvanrossum
reviewed
Mar 3, 2026
Collaborator
gvanrossum
left a comment
gvanrossum
left a comment
There was a problem hiding this comment.
This is all that I have left.
- removed clone
gvanrossum
approved these changes
Mar 3, 2026
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.
based on #200 I have added some bugfixes for the upcoming release 0-4-0
commits from me are all Commits on Feb 27, 2026
each commit covers one potential improvement or bug, also new testcases are added in this step
changelog should explain the reason