Skip to content

Fix data-corrupting introspection bugs, harden the pipeline and CI#14

Merged
shenanigansd merged 5 commits into
mainfrom
claude/project-review-r9art4
Jul 8, 2026
Merged

Fix data-corrupting introspection bugs, harden the pipeline and CI#14
shenanigansd merged 5 commits into
mainfrom
claude/project-review-r9art4

Conversation

@shenanigansd

Copy link
Copy Markdown
Member

Full project review (bugs, performance, simplifications, modernizations). Every finding below was verified by running the code before and after; the headline is that the introspection dump — the input to everything downstream — was silently missing or mis-keying ~1,900 public stdlib APIs, corrupting the docs-coverage gap artifact.

tools/stdlib_introspect.py — six data-corrupting bugs

  • 784 re-exports silently dropped: entities defined in private C modules (builtins.open in _io, asyncio.Future in _asyncio, functools.reduce in _functools, …) were parked as pending aliases waiting for a canonical record that never came. Leftover pending names are now promoted to canonical records after the walk.
  • One bad member aborted its whole module's walk: inspect.signature(select.epoll.register) raises AttributeError on Linux while evaluating __text_signature__ defaults, wiping select.poll and everything after it from Linux dumps. Signature extraction now suppresses broadly and walk_module guards per member.
  • Class-attribute references stole canonical identity: http.client.HTTPResponse was recorded as HTTPConnection.response_class (plus its 16 members) — 15 documented classes, ~138 mis-keyed qualnames. Class recursion now checks __qualname__ ownership and aliases references instead.
  • Raw staticmethod/classmethod wrappers from vars() hid 43 signatures and attributed staticmethod's own class docstring to C static methods (str.maketrans). Wrappers are unwrapped before metadata extraction.
  • __future__ dropped: the top-level roster filtered dunder modules as private; __main__ added to the skip list as defense.
  • Instance records claimed their type's docstring (math.pi → float's doc, ~3,940 data records); the headline docstring stat falls from a false 81% to an honest 49%.

Result on 3.13: 14,637 → 16,339 records, every removed qualname accounted for as an alias on a canonical record. Smoke-tested on 3.10–3.13 (the matrix floor).

tools/merge_summary.py

  • removed_in fired at the first presence gap even when the entity exists in later minors — one failed cell mid-matrix produced self-contradictory union records (a real example exists in test data: email.generator.NEWLINE_WITHOUT_FWSP_BYTES). It now derives from the last presence, with mid-matrix holes reported in a new presence_gaps field.
  • Cells stream through the merge one at a time instead of all being held parsed in memory: 178 → 86 MiB peak on a 5-cell matrix (~6× at full 18-cell scale), byte-identical output.
  • Dropped recomputed derived values (never-inserting setdefault, duplicate sorted-version pass, thrice-written sort key).

tools/coverage_diff.py

  • --target-version 3.14.0 (or a target whose introspect cells all failed) silently wrote an empty gap file and a "0 undocumented" report with exit 0. The target is now normalized and validated; an empty union exits non-zero after writing its summaries.
  • http_get re-raised every HTTPError before the retry arm (it subclasses URLError), so one transient 503/429 from docs.python.org killed the nightly run. 5xx/429 now retry with backoff; 404 still fails fast for the dev-inventory fallback.
  • CELL_VERSION regex silently dropped hyphenated version cells (linux-py3.15-dev).
  • functools.cache on the hot cell-id parse (~1.8M calls per run, 18 distinct inputs): byte-identical outputs, measurably faster.

CI / config

  • uv sync --locked in CI — plain sync silently re-locks a stale uv.lock (reproduced empirically), neutralizing the uv-lock prek hook.
  • The nightly coverage job no longer pip installs sphobjinv with floating unpinned transitives (the repo's only unpinned supply-chain surface); it uses the same SHA-pinned setup-uv + uv sync --locked --no-dev as the lint workflow.
  • Introspect/aggregate timeouts raised from 2/1 to 10/5 minutes — the scripts take ~1s; the budget was all runner provisioning, and a timed-out cell silently vanishes from the union.
  • Concurrency group + job timeout on the lint workflow; dead packages/* dependabot glob and the noxfile's dead count accumulator removed.

Verification

  • Assertion battery against the fixed dump (recovered qualnames present, canonical homes correct, signatures/docs restored) plus an old-vs-new diff with zero unexplained losses.
  • Old vs new merge_summary on a real 5-cell matrix: identical reports, identical records except the 7 with genuine presence gaps (the intended fix).
  • Old vs new coverage_diff with cached inventories: byte-identical gap/coverage/markdown outputs; error paths exercised (3.14.0 → hard error, empty union → non-zero exit, 3.13.5 → normalizes to 3.13).
  • Full lint gate green: ruff format/check, mypy --strict, ty, prek (zizmor run offline due to a sandbox network limit — no findings).
  • End-to-end pipeline run: introspect on 4 interpreters → merge → coverage diff against live docs.python.org inventories.

🤖 Generated with Claude Code

https://claude.ai/code/session_01JCViu2VXKo5HSbMxjSUsgW


Generated by Claude Code

claude added 4 commits July 8, 2026 00:58
The dump silently lost or mis-keyed ~1,900 public entities, corrupting the
downstream docs-coverage diff. Verified on 3.10-3.13: 14,637 -> 16,339
records on 3.13 with every loss accounted for as an alias.

- Flush PENDING after the walk: re-exports whose defining module is never
  walked (functools.reduce in _functools, builtins.open in _io, asyncio.Future
  in _asyncio -- 784 entities) were parked as pending aliases and dropped.
  PENDING now stores the entity plus enough context to promote the first
  name to a canonical record.
- Catch Exception (not just ValueError/TypeError) around inspect.signature():
  evaluating __text_signature__ defaults can raise AttributeError
  (select.epoll.register on Linux), and one bad member aborted the walk of
  the whole rest of its module. walk_module also now guards per member and
  reports failures instead of sinking siblings.
- Ownership check in class recursion: class-valued class attributes
  (HTTPConnection.response_class -> HTTPResponse) stole canonical identity
  from 15 documented classes; they are aliased instead of recursed into.
- Unwrap staticmethod/classmethod before metadata extraction: raw wrappers
  from vars() hid 43 signatures and reported staticmethod's own class
  docstring for C static methods (str.maketrans).
- Filter top-level modules with is_private() so the documented __future__
  module is dumped; skip __main__ (importing it returns this script).
- Treat a type docstring reached through instance attribute lookup as no
  doc: 3,940 data records (math.pi, errno.E2BIG) claimed their type's doc.
- Reuse record() for module records instead of a hand-built duplicate dict.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JCViu2VXKo5HSbMxjSUsgW
- version_span() took the FIRST presence gap as removed_in, so one failed
  or partial cell mid-matrix produced a union record claiming removal in a
  minor while its cells list showed later minors (observed live:
  email.generator.NEWLINE_WITHOUT_FWSP_BYTES absent only in the 3.12 cell
  was labeled removed_in=3.12 despite a 3.13 cell). removed_in is now the
  matrix minor after the LAST presence, and mid-matrix holes are reported
  in a new presence_gaps field instead of as bogus removals.
- Stream cells one at a time through aggregate() in the existing
  oldest->newest merge order instead of retaining all 18 parsed cell dicts:
  half the peak memory on a 5-cell matrix (178 -> 86 MiB), ~6x at full
  matrix scale, byte-identical union output.
- Drop recomputed derived values: the never-inserting exclusive.setdefault
  (and its .get fallbacks), the second sorted-version-keys pass in main(),
  and the thrice-written cell sort key (now Cell.sort_key).

Verified against the old implementation on a real 5-cell matrix: identical
console/Markdown reports and identical union records apart from the 7
records with genuine presence gaps (the intended fix).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JCViu2VXKo5HSbMxjSUsgW
- --target-version was used raw and unvalidated: '3.14.0', '3.15-dev', or
  a minor whose introspect cells all failed silently produced an empty gap
  file and a "0 undocumented" report with exit 0. The target is now
  normalized through version_label() and rejected when absent from the
  union, and an empty union exits non-zero after writing the summaries
  (no more official_docs_gap_None.jsonl).
- http_get() re-raised every HTTPError before the retry arm could run
  (HTTPError is a URLError subclass), so one transient 503/429 from
  docs.python.org aborted the whole nightly diff. Server errors and 429
  now retry with the same backoff; 4xx still fails fast for the 404 ->
  dev-inventory fallback.
- CELL_VERSION rejected hyphenated version specs (linux-py3.15-dev),
  silently dropping such cells from every version's surface; loosened to
  let version_label() canonicalize.
- functools.cache on cell_version(): it runs per cell per record per
  version pass (~1.8M calls on a real union) with only ~18 distinct
  inputs. Byte-identical outputs, 1.94s -> 1.35s end-to-end with cached
  inventories.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JCViu2VXKo5HSbMxjSUsgW
- python-ci: uv sync --locked -- plain sync silently re-locks a stale
  uv.lock (verified) and neutralizes the uv-lock prek hook that runs right
  after it. Also add a concurrency group (cancel superseded PR runs) and a
  job timeout.
- stdlib-introspect: raise introspect/aggregate timeouts from 2/1 to 10/5
  minutes. The scripts take ~1s; the budget was nearly all runner
  provisioning, and Windows prerelease setup-python alone can eat minutes
  -- a timed-out cell silently vanishes from the union (if: always()
  aggregation, if-no-files-found: warn).
- stdlib-introspect: the coverage job installed sphobjinv==2.4 via ad-hoc
  pip, floating all transitive dependencies unpinned on a nightly schedule
  (the only unpinned supply-chain surface in the repo) and drifting from
  uv.lock. It now uses the same SHA-pinned setup-uv + uv sync --locked
  --no-dev + uv run as the lint workflow.
- dependabot: drop the packages/* directory glob (no such directory has
  ever existed); use the single-directory form like the other ecosystems.
- noxfile: delete the clean session's dead count accumulator (incremented,
  never read).

zizmor --offline: no findings on either workflow.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JCViu2VXKo5HSbMxjSUsgW
Copilot AI review requested due to automatic review settings July 8, 2026 01:11

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the stdlib-introspection → union-merge → docs-coverage-diff pipeline, fixing several introspection correctness bugs, improving merge/coverage robustness and performance, and tightening CI dependency/install behavior.

Changes:

  • Fixes multiple stdlib introspection edge cases (re-exports, poisoned members, wrapper unwrapping, doc/signature attribution) and promotes pending aliases to canonical records.
  • Streams cell parsing in the union merge and corrects removed_in derivation, adding presence_gaps.
  • Hardens coverage diffing (target validation, retry semantics, hyphenated cell versions) and aligns CI to uv sync --locked.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tools/stdlib_introspect.py Fixes data-corrupting introspection cases; improves alias handling and resilience.
tools/merge_summary.py Streams cell loading; fixes version-span derivation and reports presence gaps.
tools/coverage_diff.py Validates target version, improves retry behavior, supports hyphenated cell versions, caches hot parsing.
noxfile.py Removes dead code in the clean session.
.github/workflows/stdlib-introspect.yaml Raises timeouts and switches nightly deps/install to uv + locked sync.
.github/workflows/python-ci.yaml Adds concurrency/timeout and enforces uv sync --locked in CI.
.github/dependabot.yaml Simplifies uv dependabot configuration to a single root directory.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tools/coverage_diff.py
Comment thread .github/workflows/stdlib-introspect.yaml
Comment thread tools/merge_summary.py
Review follow-up: on the 5xx/429 retry path the HTTPError kept its
response open until GC; close it explicitly before sleeping. Verified
with a mocked urlopen: retried responses are closed and 404 still
fails fast for the dev-inventory fallback.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JCViu2VXKo5HSbMxjSUsgW
Copilot AI review requested due to automatic review settings July 8, 2026 01:16

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

@shenanigansd shenanigansd merged commit 6738047 into main Jul 8, 2026
6 checks passed
@shenanigansd shenanigansd deleted the claude/project-review-r9art4 branch July 8, 2026 01:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants