ci: add strict-no-cover to detect unnecessary coverage pragmas by maxisbey · Pull Request #1897 · modelcontextprotocol/python-sdk · GitHub

Add strict-no-cover from pydantic to the CI pipeline. This tool identifies # pragma: no cover comments on lines that are actually covered by tests, helping keep coverage pragmas accurate and preventing them from accumulating as dead annotations.

Motivation

Over time, # pragma: no cover annotations accumulate on lines that are actually exercised by tests — the original reason for the pragma may no longer apply (e.g., code was refactored, tests were added). These stale pragmas hide real coverage data and make it harder to identify genuinely uncovered code.

Changes

CI integration (e66af17)

Add strict-no-cover as a dev dependency (installed from git)
Add CI step to run strict-no-cover after pytest coverage (Linux only — the tool has a Windows path-handling bug)
Pin coverage[toml] to >=7.10.7,<=7.13 because 7.13.1 introduced a regression in how source paths are reported in .coverage databases, breaking strict-no-cover
Remove relative_files = true from [tool.coverage.run] as it interferes with strict-no-cover path matching
Add pragma: lax no cover and pragma: no branch to exclude_lines in coverage config

Pragma cleanup (f0c0b6e)

Remove # pragma: no cover from ~150 lines that are actually covered by tests
Add # pragma: lax no cover for platform-specific code (Windows skipif markers, sys.platform branches, OS-specific exception handlers)
Add # pragma: no branch for branches where the exit path is never taken (async for loops, anyio.fail_after blocks, finally clauses)

Pragma guide

Pragma	Meaning	Use when
# pragma: no cover	Line must never be covered	Strict — CI fails if covered
# pragma: lax no cover	Line may or may not be covered	Platform/version-specific code, flaky coverage
# pragma: no branch	Branch exit not taken	async for, with fail_after(), loops that always break

How it works

After coverage run -m pytest and coverage combine, the CI runs strict-no-cover which:

Reads the .coverage database to find which lines were actually executed
Scans source files for # pragma: no cover annotations
Reports any lines that have the pragma but ARE covered (meaning the pragma is unnecessary)
Exits non-zero if any are found, failing the CI check

# pragma: lax no cover lines are excluded from this check, making them safe for code that is covered in some CI configurations but not others.

@Kludex without this all the windows pipelines fail to even run, seems like a strict-no-cover bug? Example failure from previous CI run: https://github.com/modelcontextprotocol/python-sdk/actions/runs/21070443439/job/60598385217

Is this working already? All the pragmas are actually being used?

Is this working already? All the pragmas are actually being used?

not sure, will test

@Kludex Found two issues that prevented strict-no-cover from actually detecting anything:

1. coverage >= 7.13 breaks strict-no-cover

Coverage 7.13.0 changed analysis_from_file_reporter to filter executed lines with & statements. Since statements excludes lines matching exclude_lines patterns, executed_lines in the JSON report can never contain excluded lines. strict-no-cover checks excluded_lines ∩ executed_lines, so the intersection is always empty — the tool silently passes regardless of unnecessary pragmas.

Fix: pin coverage[toml]>=7.10.7,<7.13.

2. relative_files = true breaks strict-no-cover

strict-no-cover creates a temp rcfile containing only [tool.coverage.report] and passes it via --rcfile=<temp> to coverage json. This overrides the entire project config, including [tool.coverage.run] settings like relative_files = true. Without that setting, coverage cannot match the relative paths stored in .coverage to the source files, resulting in empty executed_lines for all files.

Fix: remove relative_files = true from [tool.coverage.run].

I also added a test # pragma: no cover on a covered line (src/mcp/shared/exceptions.py:18) to verify the CI step actually fails. Once confirmed, that pragma should be removed.

AI Disclaimer

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

Why are those needed?

same... all those in this test actually

It seems like on 3.11 there's a bug with the tg.cancel_scope.cancel() causing lines after it to not be marked as covered. I'll get claude to write up some details

These are needed because of a coverage + anyio bug on Python 3.11 specifically.

What happens: when tg.cancel_scope.cancel() fires, anyio internally raises Cancelled to unwind the scope. On Python 3.11, this breaks coverage's sys.settrace-based line tracking — lines after the async with block exits are executed but not recorded by coverage. On 3.12+, coverage uses sys.monitoring (PEP 669) instead, which is not affected.

Confirmed locally — the lines ARE executed (inserting assert False at line 1306 fails the test), but coverage does not record them on 3.11.

Minimal repro (Python 3.11, coverage 7.12):

@pytest.mark.anyio
async def test_after_cancel():
    async with anyio.create_task_group() as tg:
        tg.start_soon(worker)
        tg.cancel_scope.cancel()
    x = 1       # MISS — not tracked by coverage
    assert x == 1

@pytest.mark.anyio
async def test_after_normal_exit():
    async with anyio.create_task_group() as tg:
        tg.start_soon(worker)
    x = 2       # HIT — tracked normally
    assert x == 2

On 3.13, both tests show all lines as HIT. The only difference is cancel_scope.cancel().

AI Disclaimer

Add strict-no-cover from pydantic to CI pipeline. This tool identifies `# pragma: no cover` comments on lines that are actually covered by tests, helping keep coverage pragmas accurate. Changes: - Add strict-no-cover as dev dependency (installed from git) - Add `pragma: lax no cover` to coverage exclude_lines for partial coverage - Add CI step to run strict-no-cover after coverage report (Linux only due to a Windows bug in the tool) - Pin coverage to <=7.13 (7.13.1 introduced a regression that breaks strict-no-cover by changing how source paths are reported in .coverage databases) - Remove relative_files=true from [tool.coverage.run] as it interferes with strict-no-cover path matching

Remove `# pragma: no cover` from lines that are actually covered by tests, as detected by strict-no-cover. For lines with partial coverage across platforms or Python versions, use the appropriate pragma: - `# pragma: lax no cover` for platform-specific code (Windows skip markers, sys.platform branches, OS-specific exception handlers) - `# pragma: no branch` for branches where the exit path is never taken (async for loops, anyio.fail_after blocks, finally clauses)

- Move pragma to except line in streamable_http.py per Kludex suggestion - Remove unnecessary lax no cover from test_streamable_http.py assertions (these lines are consistently covered across all CI configurations)

The session ID and protocol version are always set after initialization, so assert their presence rather than silently skipping the headers.

This handler may be triggered in some CI configurations, so use lax to avoid strict-no-cover failures while still excluding from coverage.

Move assertions inside ClientSession blocks where possible. Lines between the two connection phases (clearing state, setting up headers) are not tracked by coverage on Python 3.11 due to a sys.settrace interaction with cancel scopes — mark them with pragma: lax no cover.

Lines 1307-1309 (post-cancel assertions) are not reached on Python 3.14 due to cancel scope behavior. The ClientSession async with on line 1327 has an untaken exit branch. Mark accordingly with lax no cover and no branch pragmas.

maxisbey force-pushed the add-strict-no-cover branch 2 times, most recently from 78d6633 to edefce3 Compare January 17, 2026 09:37

maxisbey requested a review from Kludex January 17, 2026 09:37

maxisbey commented Jan 17, 2026

View reviewed changes

maxisbey mentioned this pull request Jan 22, 2026

chore: remove relative_files from coverage config #1931

Closed

maxisbey closed this Jan 22, 2026

maxisbey reopened this Jan 22, 2026

maxisbey force-pushed the add-strict-no-cover branch 2 times, most recently from 464a9ff to e942316 Compare January 22, 2026 17:48

Kludex previously approved these changes Jan 22, 2026

View reviewed changes

Comment thread src/mcp/client/streamable_http.py Outdated

Kludex dismissed their stale review via 8db3e6f January 22, 2026 21:36

Kludex previously approved these changes Jan 22, 2026

View reviewed changes

maxisbey dismissed Kludex’s stale review via 6af4639 January 23, 2026 11:32

maxisbey force-pushed the add-strict-no-cover branch 3 times, most recently from f0c0b6e to ab6650c Compare January 23, 2026 12:31

Kludex reviewed Jan 23, 2026

View reviewed changes

maxisbey added 6 commits January 23, 2026 17:52

fix: address review feedback on pragma placement

55cf282

- Move pragma to except line in streamable_http.py per Kludex suggestion - Remove unnecessary lax no cover from test_streamable_http.py assertions (these lines are consistently covered across all CI configurations)

test: use assertions instead of conditional guards in resumption test

2a30761

The session ID and protocol version are always set after initialization, so assert their presence rather than silently skipping the headers.

fix: use lax no cover for GET stream exception handler

a508913

This handler may be triggered in some CI configurations, so use lax to avoid strict-no-cover failures while still excluding from coverage.

Kludex previously approved these changes Jan 23, 2026

View reviewed changes

maxisbey dismissed Kludex’s stale review via 101fddd January 23, 2026 17:55

maxisbey force-pushed the add-strict-no-cover branch from 287e9f1 to 101fddd Compare January 23, 2026 17:55

maxisbey added 3 commits January 23, 2026 17:58

fix: restore uv.lock from main

0fd3003

fix: update uv.lock with strict-no-cover dependency

ae6943c

Kludex approved these changes Jan 23, 2026

View reviewed changes

Kludex merged commit a7ddfda into main Jan 23, 2026
30 checks passed

Kludex deleted the add-strict-no-cover branch January 23, 2026 20:00

maxisbey mentioned this pull request Mar 4, 2026

tests: remove lax-no-cover pragmas by moving assertions before cancellation #2206

Merged

maxisbey mentioned this pull request Jun 2, 2026

[v1.x] Support Python 3.14 #2769

Merged

6 tasks

                   # Now resume the session with the same mcp-session-id and protocol version
-                  headers: dict[str, Any] = {}  # pragma: no cover
-                  if captured_session_id:  # pragma: no cover
+                  headers: dict[str, Any] = {}  # pragma: lax no cover

ci: add strict-no-cover to detect unnecessary coverage pragmas by maxisbey · Pull Request #1897 · modelcontextprotocol/python-sdk · GitHub

Uh oh!

ci: add strict-no-cover to detect unnecessary coverage pragmas - #1897

ci: add strict-no-cover to detect unnecessary coverage pragmas#1897
Kludex merged 9 commits into
mainfrom
add-strict-no-cover

maxisbey commented Jan 16, 2026 •
edited
Loading

Uh oh!

Uh oh!

maxisbey Jan 17, 2026

Uh oh!

Kludex commented Jan 17, 2026

Uh oh!

maxisbey commented Jan 22, 2026

Uh oh!

maxisbey commented Jan 22, 2026

Uh oh!

claude Bot commented Jan 22, 2026

Uh oh!

Uh oh!

Kludex Jan 23, 2026

Uh oh!

Kludex Jan 23, 2026

Uh oh!

maxisbey Jan 23, 2026

Uh oh!

maxisbey Jan 23, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

                         uv run --frozen --no-sync coverage report
+                    - name: Check for unnecessary no cover pragmas
+                      if: runner.os != 'Windows'

		assert isinstance(captured_notifications[0], types.LoggingMessageNotification) # pragma: lax no cover
		assert captured_notifications[0].params.data == "Second notification after lock" # pragma: lax no cover

ci: add strict-no-cover to detect unnecessary coverage pragmas by maxisbey · Pull Request #1897 · modelcontextprotocol/python-sdk · GitHub

Uh oh!

ci: add strict-no-cover to detect unnecessary coverage pragmas - #1897

ci: add strict-no-cover to detect unnecessary coverage pragmas#1897Kludex merged 9 commits intomainmodelcontextprotocol/python-sdk:mainfrom add-strict-no-covermodelcontextprotocol/python-sdk:add-strict-no-coverCopy head branch name to clipboard

Conversation

maxisbey commented Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Changes

CI integration (e66af17)

Pragma cleanup (f0c0b6e)

Pragma guide

How it works

Uh oh!

maxisbey Jan 17, 2026

Choose a reason for hiding this comment

Uh oh!

Kludex commented Jan 17, 2026

Uh oh!

maxisbey commented Jan 22, 2026

Uh oh!

maxisbey commented Jan 22, 2026

Uh oh!

claude Bot commented Jan 22, 2026

Code review

Uh oh!

Uh oh!

Kludex Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Kludex Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

maxisbey Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

maxisbey Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ci: add strict-no-cover to detect unnecessary coverage pragmas#1897
Kludex merged 9 commits into
mainfrom
add-strict-no-cover

maxisbey commented Jan 16, 2026 •
edited
Loading