Achieve 100% unit test coverage

[KRRT7]

Goal

Establish a coverage baseline, fix the existing coverage configuration, integrate coverage into CI, and incrementally grow to 100% unit test coverage.

Current state

The repo has a solid test suite (~39 test files) but no visibility into what's actually covered. The pieces for coverage are partially in place but misconfigured:

What exists

• .coveragerc — present but source = test measures the test code, not the source code
• coverage[toml] — already in dev dependencies
• make coverage — exists in the Makefile

What's missing or broken

1. .coveragerc source is wrong — should be source = src/typeagent to measure the actual library, not the tests
2. CI doesn't run coverage — ci.yml runs make test but never make coverage, so there's no visibility into coverage on PRs
3. No coverage floor — nothing prevents coverage from regressing between PRs
4. No per-module breakdown — no way to see which subpackages (knowpro/, aitools/, storage/, mcp/, podcasts/, emails/, transcripts/) need attention

Suggested plan

Phase 0 — Fix infrastructure and establish baseline

1. Fix .coveragerc:

   [run]
   branch = true
   source = src/typeagent
   
   [report]
   sort = cover
   show_missing = true
   fail_under = <baseline>  # set after first run
   
   [html]
   directory = htmlcov

2. Run make coverage locally to get the baseline number, then set fail_under to that value

3. Add coverage to CI — update ci.yml to run coverage on at least one matrix entry (e.g., ubuntu + py3.14) and upload the report as an artifact or to a service like Codecov

4. Add an omit pattern for files that are hard to unit test:

   omit =
       src/typeagent/mcp/server.py  # if it's just an entry point

Phase 1 — Identify gaps

Run coverage and create sub-issues per subpackage based on actual numbers:

• src/typeagent/knowpro/ — core indexing/querying (likely partially covered)
• src/typeagent/aitools/ — AI tooling (may need mocked LLM calls)
• src/typeagent/storage/ — storage providers (partially covered via test_storage_providers_unified.py, test_sqlitestore.py)
• src/typeagent/mcp/ — MCP server (covered by test_mcp_server.py)
• src/typeagent/podcasts/ — podcast processing (covered by test_podcasts.py, test_podcast_incremental.py)
• src/typeagent/emails/ — email import
• src/typeagent/transcripts/ — transcript handling

Phase 2 — Fill gaps incrementally

One PR per subpackage, each must maintain or increase the coverage floor. After each merge, ratchet fail_under up.

Phase 3 — Reach 100%

Mop up remaining branches, edge cases, and error paths.

Principles

• Ratchet, don't regress — fail_under in .coveragerc should be bumped after each coverage PR so the floor only goes up
• Measure source, not tests — coverage should track src/typeagent, not tests/
• Mock external services — AI API calls, Azure auth, and email providers should be mocked for offline unit tests
• Fast and isolated — unit tests should run without network access or API keys (the existing offline/online split is a good pattern to keep)
• Branch coverage — branch = true is already set, keep it — it catches missed conditional paths that statement coverage misses

Quick start

# Fix .coveragerc source, then:
make coverage
# Look at the total % and per-file breakdown
# Set fail_under to the current total

[gvanrossum]

Sorry, not interested for now.

[KRRT7]

Motivating example: the #238 regression

The URL parsing regression from #231 is exactly the kind of bug that test coverage would have caught. Here's the chain of events:

What happened

1. PR Fix parse_azure_endpoint passing query string to AsyncAzureOpenAI #231 fixed parse_azure_endpoint to strip query strings (?api-version=...) before passing the URL to AsyncAzureOpenAI — a valid fix
2. But the tests only verified parse_azure_endpoint in isolation. They confirmed the query string was stripped, but never tested the downstream consumer (create_chat_model → _make_azure_provider → AsyncAzureOpenAI)
3. AsyncAzureOpenAI appends /openai/deployments/{name} to azure_endpoint. When the endpoint from .env already contains that path (which is the default format from Azure Foundry), it produces a doubled path → 404
4. test_online.py (the only integration-ish test) takes a different code path (create_async_openai_client) than the real app (create_chat_model), so it passed fine
5. CI ran make test which only runs offline tests — no coverage, no integration path exercised

Root cause: insufficient test coverage

The real problem wasn't the code change itself — it was the lack of coverage over the actual code paths users hit:

• create_chat_model → _make_azure_provider → AzureProvider had zero test coverage
• model_adapters.py had zero test coverage
• The only Azure endpoint test was test_online.py which tests a completely different code path

Had the coverage been measured and visible, a reviewer would have seen that model_adapters.py and the _make_azure_provider path were untested, and could have asked for tests before merging.

What coverage would have prevented

• A fail_under gate would have flagged that Fix parse_azure_endpoint passing query string to AsyncAzureOpenAI #231 added code to a 0%-covered file without adding tests for the actual consumer
• Per-file coverage in CI would have shown model_adapters.py as a blind spot
• The fix PR regression URL parsing in PR #231 #239 / Fix endpoint parsing #241 added 82+ lines of tests to test_model_adapters.py that didn't exist before — these should have been required from the start

This is the textbook case for why coverage infrastructure matters: the bug wasn't in the function that was tested, it was in the integration path that had no tests at all.

[KRRT7]

Totally understand if this isn't a priority right now. Would you mind keeping the issue open though? That way it's tracked for whenever someone has bandwidth to pick it up — especially given the #238 regression showed there are real blind spots in the test suite that coverage would surface.

[KRRT7]

Worth adding — an integration test covering the actual user code path (create_chat_model → _make_azure_provider → AsyncAzureOpenAI) would have also caught this before it shipped.

The unit tests in #231 confirmed parse_azure_endpoint returned the right string, but nothing verified what happened when that string was passed into AsyncAzureOpenAI. Something like:

def test_create_chat_model_with_foundry_endpoint(monkeypatch):
    """Azure Foundry-style endpoints with /openai/deployments/ path don't produce doubled paths."""
    monkeypatch.setenv(
        "AZURE_OPENAI_ENDPOINT",
        "https://myhost.openai.azure.com/openai/deployments/gpt-4o?api-version=2024-06-01",
    )
    client = create_chat_model(...)
    # Assert the resolved base_url doesn't contain /openai/openai/

would have flagged the issue. Unit tests verify the parts work, integration tests verify they work together — having both gives much stronger confidence when merging.

[gvanrossum]

No. I actively want to discourage people from working on this. It's too distracting.