Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.81%. Comparing base (7635d45) to head (0ffc2ed).
⚠️ Report is 4 commits behind head on master.

@@           Coverage Diff           @@
##           master    #1827   +/-   ##
=======================================
  Coverage   99.81%   99.81%           
=======================================
  Files          33       33           
  Lines        3799     3839   +40     
  Branches      543      553   +10     
=======================================
+ Hits         3792     3832   +40     
  Misses          5        5           
  Partials        2        2           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Merging this PR will not alter performance

✅ 24 untouched benchmarks

Comparing nsec_address_denial (0ffc2ed) with master (ca9d027)

Previous review — superseded by a newer review below.

. A cached denial fails the request without ever sending a query

ServiceInfo is a single shot throw away

@bdraco: > . A cached denial fails the request without ever sending a query

Agreed, dropping that one. Flags reset at top of _load_from_cache, so each request re-derives from cache. Stale window bounded by NSEC TTL. Address guard in _is_denied covers cached-address case.

Previous review — superseded by a newer review below.

PR Review — fix: fail service resolution fast when nsec denies the needed address types

Both outstanding suggestions from the last round are now covered by tests, and no production code changed since — approving with two cosmetic nits.

Specific things done well:

• test_legacy_inverted_nsec_at_own_name_does_not_fail_request pins exactly the invariant that keeps the pre-0.150.4 inverted-bitmap shape safe: server == name + bitmap [AAAA] sets _ipv4_denied backwards, and the test asserts both the wrong flag and that _is_denied stays False. That makes the "needs both families" property explicit, so a future relaxation of _is_denied fails loudly instead of silently breaking legacy v4-only responders.

• test_denied_flag_is_ignored_when_address_is_held reaches the not self._ipvX_addresses guard by the one route that actually gets there — a plain ServiceInfo with a cached SRV + A + host NSEC [TXT], incomplete only because TXT is missing. Both flags go True, an address is held, and _is_denied is False. That is the load-bearing safety net and it is now covered.

• I re-verified test_own_nsec_response_does_not_complete_service against the new server-NSEC path in _load_from_cache: our own [A] NSEC sets ipv6_denied only, so _is_denied stays False and the request keeps querying for the missing TXT. No regression from the added cache lookup.

• _read_bitmap (src/zeroconf/_protocol/incoming.py:411) raises on truncated or oversized windows rather than returning a short list, so a malformed NSEC cannot produce an empty rdtypes that would fast-fail a resolvable host.

• NSEC deferral in async_update_records also improves the fix: correct nsec bitmap semantics and resolve txt denials #1825 TXT path: a same-batch real TXT now always wins over the denial, regardless of wire order.

• Nit: @cython.locals on info.pxd:85 names update (the local is updated) and cache (no such local) — pre-existing, but the line is edited here and the fix restores the intended bint typing.

• Nit: the new legacy-NSEC test docstring has a multi-line body that re-explains the bug, against the repo's documented test-docstring rule.

• Previously raised and dismissed by @bdraco ("ServiceInfo is a single shot throw away"), not re-raised: a cached denial fails the request without putting a query on the wire.

🟢 Suggestions

The cython.locals list on this line declares update=bint and cache=DNSCache, but async_update_records has neither local — the accumulator is named updated (src/zeroconf/_services/info.py:576) and there is no cache binding in that method at all.

Why it matters: the intended effect of update=bint was to give the accumulator a C-level bint so updated |= self._process_record_threadsafe(...) compiles to a C or instead of a Python bitwise-or on bool objects. With the name misspelled, Cython silently declares two unused C locals and updated stays a Python object — the annotation is doing nothing on a path the PR description explicitly calls out as perf-sensitive.

This predates the PR, but the line is being edited here anyway (to add record and nsec_records), so renaming update -> updated and dropping the dead cache entry is a one-token fix that finally delivers the typing that was intended.

    @cython.locals(record_update=RecordUpdate, record=DNSRecord, update=bint, cache=DNSCache, nsec_records=list)
    cpdef void async_update_records(self, object zc, double now, cython.list records)

CLAUDE.md is explicit about this shape: "Test docstrings retelling the production-side story. A test docstring should name what the test pins, in one sentence — not re-explain the bug, the fix, or the surrounding flow."

The summary line already names what is pinned; the two-line body re-explains the legacy inverted-bitmap behaviour and the _is_denied invariant, which is PR-description material. Every other new test in this batch is a single line and reads well.

Dropping the body (or folding the essential half-clause into the summary) brings it in line with the rest of the file.

    """A pre-0.150.4 inverted NSEC from a service with server == name never denies the request.

    Legacy responders listed the missing address types, so the snapshot records
    the wrong family; a single wrong flag must never make _is_denied true.
    """

Checklist

• Protocol behaviour change cites the governing RFC
• Cython .pxd updated in the same commit as the signature/layout change
• Backward compatibility with pre-0.150.4 NSEC semantics
• New state has a defined lifecycle (init, reset, invalidation)
• Record ordering within and across update batches handled
• No premature failure for resolvable services
• Test coverage for the new branches
• Hot dispatch path unchanged for non-NSEC records — suggestion ＃1
• Malformed / empty NSEC bitmap cannot cause a spurious fast-fail
• Diff matches the PR description, no scope creep
• Test style matches documented repo conventions — suggestion ＃2

Automated review by Kōan (Claude) HEAD=ac3afd2 4 min 55s

Addressed both nits in 0ffc2ed.

For the cython.locals one, the suggested updated=bint does not compile; the body already annotates updated: bool = False, so Cython errors with 'updated' redeclared. That annotation already produces a C bint (int __pyx_v_updated in the generated C), so I dropped the dead update and cache entries instead.

The test docstring is now a single line.