0026-a-bool-is-never-an-int-at-the-discovery-boundary.md

A bool is never an int at the discovery boundary

bool subclasses int in Python, so isinstance(True, int) is true. The discovery assembler's boundary check — _require(mapping, key, expected, context) and its optional sibling — was a bare isinstance, so every integer field in the module accepted a boolean: an api_version of true passed the check that exists to reject exactly that, and would have been carried into the canonical document and compared numerically downstream as a 1.

Two things kept it small. The exposure is close to nil — the values come from an Extractor this client authenticates against, over the site owner's own credential — and it was the module's convention from the start, not a regression a change introduced. What made it worth closing is the other half: the convention is shared, so tightening it is a one-place change. The question had also been asked before in this repository and answered locally, in wpconfig_block.py, whose define writer already orders its bool check ahead of its int check for this precise reason; discovery's own boundary never learned it.

Decision

One predicate decides what a type check means for the whole module: _has_type(value, expected) is isinstance with a single narrowing — a field declared int refuses True and False, and every other declared type keeps plain isinstance semantics.

Both boundary helpers go through it, and so do the module's two hand-rolled integer checks: the entity counts' own check and the file manifest's tolerant size read. No call site changes, the refusal keeps the same shape every other type mismatch produces, and the message names the type that arrived (must be int, got bool), so a future occurrence is diagnosable from the diagnostic alone.

A field declared bool is deliberately unaffected. The narrowing is an int field rejects a bool, never a boolean is not a valid value; the module declares no boolean field today, and the guard on that half exists so that the first one is not broken by this decision.

Rejected alternatives

• Leave the convention and record why not. Defensible on exposure alone, and rejected on cost: it buys a note the next reader has to re-derive, against a predicate that is one function and one line at each of four sites. The question had already been reached twice — once in wpconfig_block.py, once here — and the point of settling it is that there is no third time.
• Narrow at the call site that matters. Spelling isinstance(value, int) and not isinstance(value, bool) for api_version alone answers the reported symptom and leaves every other integer field loose, and the next integer field added would start loose again. The knowledge belongs where the type check is decided, not where a field is declared.
• Adopt a schema validator (pydantic) for the input document. It would restate the whole document's shape to buy one narrowing, and it puts a dependency into a helper whose PEP 723 dependency list is deliberately empty — the assembler runs anywhere uv can start a bare interpreter.
• Tighten every accidental coercion at once — a float for an int, a numeric string, None as an absent value. Each is a separate judgement about what the wire is allowed to say, and folding them into a bool narrowing would hide those judgements inside a change nobody reviewed for them.

Consequences

• Every integer field in the assembler tightens together: the required fields, the optional ones, the entity counts, and the manifest's size. A boolean size in a manifest entry now weighs nothing in a subdirectory total instead of the one byte int-ness lent it, which is the tolerant path's documented intent — a non-integer size counts as zero — rather than a new behaviour.
• The narrowing is invisible to any well-formed input. No document that an Extractor produces changes shape, and no run behaves differently; what changes is which malformed inputs are refused.
• The sibling helpers are deliberately untouched. baseline_diff.py and classify.py carry the same bare-isinstance convention for their own integer fields, and poll_extraction.py reads its progress counters the same way. They are a separate module boundary with their own inputs, and this decision does not silently reach into them; if the same narrowing is wanted there, it is its own change with its own tests. (Superseded for the two boundary helpers by the amendment below, which is that change; poll_extraction.py stays untouched, and now for a stated reason.)

Amendment (2026-08-21): the rule is the transfer engine's, not the discovery assembler's

The two sibling helpers this ADR left alone are now closed the same way, so the rule is stated once for every boundary helper in the transfer engine rather than for the assembler that happened to reach it first (issue #70). What made it worth doing is the inconsistency rather than the exposure, which is as small as it was above: one helper documented the hazard (wpconfig_block.py), one refused it (discovery.py), and two still accepted it. The question had by then been asked three times in this repository, and the point of settling it is that there is no fourth.

• skills/clone/scripts/baseline_diff.py gains its own _has_type, and both of its boundary helpers — the required-field check and its optional sibling — route through it. Its numeric mtime reader cannot: its accepted type is the tuple (int, float), which a predicate keyed on expected is int cannot speak for, so it carries its own explicit bool refusal ordered ahead of the numeric check — the same ordering wpconfig_block.py already uses and documents. This is the one live consequence in the set: mtime is half the size+mtime quick-check and float(True) is 1.0, so a boolean mtime was read as one second past the epoch and could call a file changed when it was not, or unchanged when it was.
• skills/mkwp/scripts/classify.py gains its own _has_type, and its inner-record field check routes through it, tightening a subdirectory's size_bytes at both scans that read one — the uploads outlier heuristic and the non-standard-directory sweep. Its other readers (_list, _string_list) hard-code their accepted type and take no expected, so there is nothing in them for the predicate to decide.

The predicate is copied into each module, not shared. Every helper is a self-contained PEP 723 script with an empty dependency list, and the standalone-distribution guard pins the dependency direction: clone and pull may reach into mkwp's scripts, never the reverse (issues #50/#51/#52). A predicate module the classifier imported from clone would invert that and break mkwp's portability. The rule lives here, in this ADR, rather than in a shared import — which is why it is written once and cited from three modules.

poll_extraction.py's progress counters stay as they are, now decided rather than merely deferred. Its inline isinstance(x, int) reads on chunks_done, tables_done and files_done are progress arithmetic, not a boundary refusal: a boolean there is weighed as 1 and shows a wrong number on a progress line. Refusing it would turn a cosmetic glitch into a failed poll on a multi-hour extraction, which is a far worse trade than the one this ADR makes everywhere else.

A TypeIs annotation was considered and rejected, so the predicate returns a plain bool and mypy gets no narrowing from it. typing.TypeIs is PEP 742 / Python 3.13+ while these scripts declare requires-python = ">=3.12" with an empty dependency list, so typing_extensions is unavailable too and issue #71 settled that the floor does not move. It would not have helped even so: TypeIs[T] over a predicate that takes the expected type as a runtime parameter does not narrow at the call site, and it would be unsound for this one — answering False for a bool where an int is expected narrows a case out of the negative branch that genuinely reaches it.

What the checker gets instead is a thin accessor, not a second isinstance. Replacing isinstance(size, int) with the predicate cost the assembler's file-manifest size read its static narrowing, so size reached the child tuple as Any where an int is declared — correct at runtime, since the alternative branch supplies 0, and rejected by mypy --strict (issue #71). discovery.py therefore reads that one tolerated integer through _boundary_int(value, fallback), which consults the predicate and typing.casts the value the predicate has just vouched for. The cast asserts nothing the predicate has not already decided, and the decision stays in the one place this ADR put it — which a narrowing isinstance written beside the call would not. It is needed only where a tolerated value flows on into a typed structure; the boundary helpers raise on the negative branch, so nothing flows and nothing needs it.

The title and filename keep their original wording even though the scope no longer matches them, because CHANGELOG.md cites this file by name and a rename would break the link for the sake of a slug. The scope that governs is the one stated here.