refactor: police the fan-out abstractions - #364
Merged
thodson-usgs merged 10 commits intoAug 9, 2026
Conversation
Chunking is how you divide the data structurally; fan-out is how you distribute the work operationally. The two are orthogonal, and only the first is protocol knowledge -- dividing a query needs the byte budget, the CQL2 grammar, and which parameters are list-valued, while distributing the pieces needs none of it. The package had not drawn that line. ChunkPlan (division) and ChunkedCall (distribution) sat side by side in dataretrieval.ogc as siblings. Unable to reach an OGC-internal executor, wateruse._fan_out re-implemented the semaphore, the gather, and the failure-precedence rule, with a comment naming ChunkedCall._run as the original -- one subtle rule, two copies, synchronized by prose. The duplicate lacked resume (a 429 partway through discarded every completed location), reported no progress, and ignored API_USGS_CONCURRENT. Move execution down; leave planning up. transport.fanout.FanOut drives any FanOutPlan -- a Protocol of the three members the executor already used (total, canonical_url, iter_sub_args). It is structural because its two implementations share an interface and no implementation: ChunkPlan derives sub-requests from a byte budget, a Water Use plan lists locations the caller already named separately. Water Use sheds ~75 lines and gains resume, progress, and the shared concurrency setting. Concurrency is now one general knob with per-service defaults, and an explicitly set API_USGS_CONCURRENT outranks a service default -- never the reverse, or the setting would be a lie. The interruption taxonomy moves to the dataretrieval.interruptions leaf, since adapters need it whether or not they went through transport. Its base is renamed FanOutInterrupted, because Water Use raises it without chunking anything; ChunkInterrupted stays as a permanent alias of the same class object, so `except ChunkInterrupted` keeps working. _deterministic_failure moves to that leaf too, and transport.retry imports it back. Whether a failure is worth retrying and whether it can be resumed are one judgement about what the exception means, not two -- and the leaf is where meaning lives. Leaving it in transport would have forced the leaf to import transport to ask. BREAKING CHANGE: a Water Use fan-out interrupted by 5xx/429 now raises ServiceInterrupted/QuotaExhausted rather than ServiceUnavailable/ RateLimited. Both remain DataRetrievalError, so broad handlers are unaffected, but a narrow `except ServiceUnavailable` must widen. This is convergence with the OGC getters, and it is what makes the failure resumable. wateruse.MAX_CONCURRENT_REQUESTS is removed in favor of API_USGS_CONCURRENT / wateruse.DEFAULT_CONCURRENT_REQUESTS. Supersedes the ADR 0006 clause assigning resumable ChunkedCall state to OGC; see ADR 0008. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three abstractions in the fan-out commit were carrying less than they cost.
FanOutPlan declared total, canonical_url and iter_sub_args(). The executor
never looks inside what a plan yields -- _pending enumerates and hands the item
straight to the adapter's fetch -- so the item type belongs to the adapter, not
to a dict[str, Any] wide enough to hide that ChunkPlan yields kwargs while
Water Use yielded {"request": r} and unboxed it two lines later. And total plus
iter_sub_args() are len and iter under private names, which is why a fitness
test had to assert the two agreed.
So the protocol is now __len__ and __iter__, generic in the item type. A plain
list is a plan, which deletes _LocationPlan and the boxing; ChunkPlan keeps
total/iter_sub_args as its domain vocabulary and delegates the dunders to them,
so the counts cannot disagree. canonical_url moves to a FanOut argument -- it
is stamped on the combined response, not a property of how work divides.
Conformance is now mypy's job rather than a runtime test's: passing a generator
fails at the call site naming the missing __len__, instead of an AttributeError
mid-fan-out.
Four walks of the exception chain had three traversal policies between them --
_deterministic_failure over cause and context with a cycle guard, and
_classify_chunk_error, FanOut._normalize_failure and the status hunt in
FanOutInterrupted.__init__ over cause alone, two of them unguarded. Every one
asks "first exception in this chain satisfying P", so they share one guarded
_walk_causes generator. The two unguarded walks no longer spin on a chain that
points back at itself; a regression test closes that cycle deliberately.
_resolve_concurrency hand-rolled the read-cast-validate sequence
_read_env_number already performs for API_USGS_RETRIES and
API_USGS_STALL_TIMEOUT, in a different error voice. It delegates now, handling
only the "unbounded" keyword that is genuinely its own; the shared parser gained
minimum= and hint= to say what this knob needs.
pyscn: transport.fanout drops from 9 dependency concerns to 8 (fan-out 7 -> 6),
exceptions loses an afferent edge (fan-in 10 -> 9), transport package cohesion
rises 0.30 -> 0.32, total dependencies 145 -> 144, LCOM 95 -> 100. Composite
80 -> 81. The Architecture sub-score reads 85 -> 81, which is an artifact worth
recording: the two findings that changed are on utils and waterdata.utils, whose
descriptions are byte-identical before and after -- same concerns, same fan-in
and fan-out. Untouched by this commit, they were re-ranked from warning to error
because tightening fanout moved the distribution around them. Same behaviour ADR
0008's predecessor documented when extracting BaseMetadata scored worse, and the
same reason the tool stays advisory.
779 tests pass, pre-commit clean, mypy clean across 53 files.
Five changes, each findable without a metric, plus the honest accounting for two that were not worth making. A next-page href is untrusted data, and validating one existed three times with three behaviors. ogc/engine.py raised a bare RuntimeError on a cross-host link, did not resolve a relative href (it handed the unresolved reference back as the pagination cursor, so the follow-up request failed instead of paging), and fell open by returning the raw href when httpx.URL() rejected it. ratings resolved, required an exact host, stripped userinfo. wateruse resolved, accepted a host alias set, rewrote to the canonical host, dropped the port too. One policy now lives in transport/links.py and the walks pass in what genuinely differs -- the acceptable hosts and whether an accepted host is rewritten. Both engine bugs are fixed by the merge. Its fall-open branch was reachable only from tests whose mock response had no real URL, so those mocks got one rather than the security check keeping a hole to accommodate them. Each walk keeps the exception type it already raised: retyping engine's RuntimeError to DataRetrievalError is a released behavior change to make deliberately, so resolve_next_url takes the type and engine passes RuntimeError. parse_retry_after moved from transport/retry.py, which never called it, to exceptions.py, beside the DataRetrievalError.retry_after field it exists to produce. Its two callers each use it in one expression, both feeding error_for_status. Three stdlib imports went dead in retry.py on the way out -- evidence it shared nothing with its host module. ogc/errors.py, whose job is mapping a status to a typed exception, no longer reaches through the retry policy (and thence interruptions and pandas) to parse a header. utils.py split along its real seam. Half was pandas column munging over codes; half was "GET a USGS URL, map the status, retry it" over exceptions and transport. They shared a filename and nothing else -- nldi, streamstats and nwis took only the HTTP half, waterdata.samples only the munging. The query path is now _querying.py, private for the same reason _ambient and _response_metadata are: the names are public even though the module is not, so utils.query and utils.to_str stay the documented paths (__module__ preserved, so the API reference page is unchanged). Also: waterdata profile validation moved into types.py, next to the three tables it exists to validate against, which was waterdata/utils.py's only reason to import types; nwis.get_dv and get_iv now share one body, which encodes the non-obvious contract that the caller-facing args are aliases that lose to explicit waterservices kwargs; nldi's repeated "fetch, honor as_json" tail became _query_features; and utils.py lost four compatibility aliases for names that never existed at that path (git show v1.2.0 confirms) and were referenced nowhere. One of the five candidates for removal, HTTPX_ASYNC_DEFAULTS, turned out not to be dead -- a test imports it -- so it was repointed at transport.http rather than deleted blind. pyscn: composite 81 -> 82. Dependencies 75 -> 80 (max depth 9 -> 8, total dependencies 144 -> 149 across two new modules), architecture 81 -> 82 (weighted violations 39 -> 37; dataretrieval.utils drops out of ERROR), duplication holds at 70 with clone fragments 26 -> 25 and 9.25% -> 8.90% -- real, but the 75 band starts near 7.3%. Complexity, dead code, coupling and cohesion unchanged at 95/100/100/100. Two candidates were measured and declined. Splitting waterdata/utils.py three ways (policy/args/retrieval) scores +0.65 architecture, but its weighted violations are unchanged -- one 5-point error traded for five 1-point warnings -- so the entire gain is denominator drift from the extra modules, and the new warnings exist because "args" and "retrieval" have real names where "utils" was filtered out of every consumer's responsibility set. Only the part that stands without the number was taken. Extracting the cause-walking predicates out of interruptions.py into a leaf earns zero dependency points (interruptions gains the edge retry loses) and costs a new module plus a layers edit on the module that owns the public resume contract; it is worth doing when interruptions is being refactored for its own reasons, not for this. Pruning the dead utils aliases is scored slightly negative (architecture compliance 80.60 -> 80.50: utils goes from 12 responsibilities to 11 but stays an ERROR while TotalRules falls, so the ratio drops). Kept anyway -- a compatibility promise for a name that never shipped is debt, not surface. Tests 779 passed / 12 deselected throughout, mypy clean (53 -> 55 files with the two new modules), xenon, complexipy, import-linter and pre-commit green.
Six changes, each defensible without a metric, plus one that measured worse and
was reverted.
The "a GeoJSON feature may omit geometry" workaround existed twice, verbatim,
in ogc/shaping.py and waterdata/stats.py -- and each copy carried a comment
pointing at the other. When NGWMN or the statistics service changes what it
omits, that was two places to find. It is now _geo_feature_frame in
ogc/shaping.py. The secondary win is the one worth having: stats.py kept its
own guarded `import geopandas as gpd` only because it called from_features
directly, and paid for it with seven lines of comment explaining that an
empty-page test patches shaping.gpd while the populated branch uses stats.gpd.
Routing through the shared builder makes that asymmetry disappear instead of
documenting it.
ogc/requests.py stated the OGC items path and the CQL2 media type once per
construction function. Both are facts the upstream API owns; neither had a test
that would catch changing only one. They are now _items_url and
_cql2_post_request. Likewise nldi.py spelled the navigation URL grammar in both
get_flowlines and _get_features_request; _navigation_request holds it once. The
callers keep their own query-parameter insertion, because the order distance,
trimStart, stopComid is pinned by URL-matching tests and is a caller concern
rather than part of the grammar. streamstats.py was the last adapter writing
its service host inline (twice) instead of naming it once, like nldi, ngwmn,
wateruse and waterdata already do.
_read_env_number moved out of transport/retry.py into a new transport/env.py.
Its own docstring calls it "the single parser behind every API_USGS_* numeric
knob" -- a package-wide concern -- yet it lived inside the bounded-retry policy
module, so transport/fanout.py imported that module to parse API_USGS_CONCURRENT,
a setting retry knows nothing about. `import math` and `import os` went dead in
retry.py on the way out, which is the usual evidence the helper shared nothing
with its host.
ruff's TC001 is now selected, moving twelve first-party annotation-only imports
into TYPE_CHECKING blocks. This one only makes the code agree with a policy the
project already wrote down twice: .importlinter sets
exclude_type_checking_imports = True ("Contracts describe what runs") and
architecture_test's import visitor deliberately skips TYPE_CHECKING blocks.
exceptions.py already used the idiom and had a test pinning it. Enforcing it
with a rule rather than hand-picking imports is what keeps it honest -- ruff
chose the twelve sites, not me. TC002/TC003 stay unselected: third-party and
stdlib import cost is a different concern with no policy behind it here, and it
would have been 46 more sites of unrelated churn.
waterdata/utils.py now consumes the OGC facade alone. prepare_request_args'
no_normalize parameter replaced the engine's own set rather than extending it,
so the adapter had to reach into ogc.dates for _DATE_RANGE_PARAMS and union it
back -- a live foot-gun, since any adapter passing no_normalize without
remembering that union silently breaks date-range handling. The parameter is
now extra_no_normalize and adds. The single-caller _finalize_ogc wrapper is
gone; get_cql injects extra_id_cols/dialect at its own call site, still reading
both constants from waterdata.utils, so each still has one definition. Both
architecture ratchets were tightened, not loosened.
Finally, the OGC package no longer defaults every base_url to the Water Data
host. ogc/engine.py's docstring says it is "deliberately free of any
Water-Data-specific constants so a sibling package (e.g. NGWMN) can drive it",
but ogc/policy.py set BASE_URL = WATERDATA_BASE_URL and that value was the
default for the ambient base URL, get_ogc_data, both shaping helpers and the
schema fetch -- generic in structure, Water-Data-bound in policy, laundered
through the credentials leaf so test_credential_policy_has_one_definition did
not notice. It was also a live latent bug: get_queryables called
_check_ogc_requests with no base_url and got the Water Data host by accident,
so the same function reached from NGWMN would have queried the wrong service.
The ambient now defaults to empty, each adapter names its own endpoint, and a
path that forgets fails loudly on a malformed URL instead of silently hitting
someone else's API. get_cql needed the same treatment -- it built its request
through the ambient and had been relying on that default too.
Measured, dataretrieval only, before -> after:
composite 82 -> 82 (unchanged; duplication 70 and dependencies 80
dominate and neither moved)
architecture 82 -> 86 (compliance 0.8230 -> 0.8557)
weighted viol. 37 -> 29, total 33 -> 29, rules 209 -> 201
severity {error 1, warning 32} -> {warning 29}
dependencies 149 -> 140 edges, 55 -> 56 modules, depth 8 unchanged
ogc cohesion 0.4848 -> 0.5313
The package's last ERROR-severity module is gone. None of the six moved
duplication: every shared block is under pyscn's 10-line clone floor, which is
the honest reason to make them anyway.
Reverted: moving _raise_for_non_200 and _error_body out of ogc/errors.py into
exceptions.py. The design case is real -- three of that module's five importers
are not OGC (an RDB endpoint, the Aquarius Samples REST service, and the
statistics service), and it is generic USGS status-to-typed-error mapping. But
it measured worse: architecture 86 -> 85 and ogc package cohesion 0.5313 ->
0.4839, back under the threshold, because it removes two ogc-internal edges
while removing only one external one. One violation traded for another plus
denominator loss. Worth doing on its own someday, not bundled here.
Not attempted: collapsing the per-collection getter families, and adding ABCs
to exceptions.py/codes. Both remain declined for the reasons already recorded.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DSMyMqQ4uuQ9TAbJLnJ6Yi
Six changes, each defensible without a metric, plus one that was implemented,
measured worse, and is recorded here so it is not re-derived.
The one worth the commit on its own is that transport/pagination.paginate has
no retry -- retry lives in FanOut, which wraps each sub-request in retry_async.
So waterdata.get_cql (via ogc.engine.fetch_ogc_request) and get_stats_por /
get_stats_date_range (via waterdata.stats.get_data) were the only two retrieval
paths in the package where a mid-page-walk 429 or 503 failed the whole call
outright, while every typed getter and Water Use rode it out. Both now run as a
one-item FanOut, and transport/sync.py -- a 25-line module that existed only
because those two bypassed the executor -- is deleted. This is a behavior
change and NEWS.md says so: those getters gain retry (and therefore spend quota
on transients) and now raise the resumable ServiceInterrupted / QuotaExhausted
instead of ServiceUnavailable / RateLimited / NetworkError. A failure retrying
cannot fix -- bad scheme, a hostname that does not resolve -- still surfaces as
NetworkError immediately, unchanged. The stats mid-pagination test was reworked
onto the new contract rather than dropped: it now asserts the pagination
wrapper is the cause and that the interruption carries a resumable handle.
FanOut also owns the progress line now. Every driver previously had to remember
a separate `with progress_context(...)` block or the shared executor would run
and print nothing -- and wateruse.py carried an in-code comment recording that
this had already been forgotten once. A foot-gun with a comment explaining that
someone stepped on it is a misplaced responsibility. The executor that emits
the events opens the reporter in resume(), so progress_context is now opened in
exactly one place in the package. It opens in the *calling* context, so an
outer reporter (a nested getter, or the caller's own block) is still the one
found and reused; the snapshot's reporter is still overridden, because a
reporter captured at construction belongs to a context that has since closed
it. The side effect is that a `.call.resume()` fired long after an interruption
now reports progress instead of running mute.
The OGC queryables document is parsed by the OGC layer. queryables_frame moved
from waterdata/reference.py into ogc/schema.py, leaving get_queryables as the
documented Water-Data-facing wrapper with its docstring intact. Reading a
queryables JSON Schema is protocol knowledge about OGC API - Features, not
knowledge about USGS Water Data; with it inside one service adapter, NGWMN
could not offer the same getter without copying the loop, and the previous
round found a live bug in exactly this function for exactly that reason.
Three smaller relocations, none of which move the number:
utils.py's own docstring says "do not add new service-specific behavior here",
and _attach_datetime_columns is explicitly service-specific -- its docstring
names "USGS Samples and Water Quality Portal CSV responses" and it detects two
WQX/WQP column-naming dialects. It and its two helpers are now the _wqx leaf,
importing pandas and codes.tz and nothing else, placed in .importlinter's stack
above the pure leaves (it reads the code tables) and below every adapter that
shapes a response with it.
Five Water Data endpoints were spelled in three modules, each independently
deriving from credentials.WATERDATA_BASE_URL. waterdata/endpoints.py holds them
once, and stats.py and ratings.py -- neither of which does OGC retrieval -- no
longer import the credentials leaf just to name a URL. Deliberately a new
module rather than waterdata/utils.py: the utils variant was measured and it
pushes MaxDepth 8 -> 9.
ogc/engine.py imports each symbol from the module that defines it: active_client
from transport.fanout rather than through chunking's test-facing compat alias,
and _dialect / _ogc_base_url from ogc.context rather than through requests.py,
which only re-exports them. The file previously imported three siblings' worth
of per-call ambients from two different places.
Measured, dataretrieval only, before -> after:
composite 82 -> 82 (unchanged; duplication 70 and dependencies 80
dominate and neither moved)
architecture 86 -> 87 (compliance 0.85572 -> 0.87129)
weighted viol. 29 -> 26, rules 201 -> 202
severity {warning 29} -> {warning 26}; no ERROR appeared
cleared progress, transport.sync, waterdata.reference
dependencies 140 edges / 56 modules -> 140 / 57, depth 8 unchanged
waterdata coh. 0.33 -> 0.39 (still under the 0.50 threshold, so the
package-cohesion violation does not clear)
Honest accounting: only three of the six moved a violation. The _wqx and
endpoints moves are worth ~0.13 of a point each and that is pure denominator
drift from the added module and edge -- they are here on design merit or not at
all. The engine import hygiene is worth exactly zero and is listed so the next
round does not mistake it for a lever.
Not applied: lifting _next_req_url / _paginate / _ogc_parse_response /
_walk_pages out of ogc/engine.py into a new ogc/pages.py. The design case is
real (engine mixes "walk the pages of one OGC request" with "orchestrate a
chunked getter call"), but it was implemented and measured and it loses twice
over: the extracted module inherits every edge it was supposed to relieve and
is flagged in its own right (weighted 29 -> 30), and it inserts a hop into the
longest import chain (MaxDepth 8 -> 9), which would also cost the dependencies
point an earlier round spent a commit winning. Architecture 86 -> 85.
progress._use_reporter held the identical set/reset-token dance that _ambient.Ambient.__call__ exists to own -- Ambient's docstring says it bundles the var and its token dance "so an ambient value needs a single declaration instead of a var + setter-function pair", and progress had exactly that pair. _active becomes an Ambient, current() reads it, progress_context scopes through it, and _use_reporter goes away with the contextvars import. Reset-then-close ordering on exit is unchanged, as is the nested-call path that yields the existing reporter. Found by scanning below pyscn's clone floor (min_nodes 8 rather than 20), where two eight-line helpers are invisible to it.
FanOut._run asked wrap_failure() about every failed sub-request in order to let a non-transient sibling surface raw, but kept only the first transient. Each of those calls constructs a FanOutInterrupted, and its __init__ snapshots call.partial_frame.copy() -- a full concat and dedup over every completed sub-request's frame. A batch of sub-requests failing together on the same 429 or 5xx is the routine case, so N failures meant N combined-frame snapshots with N-1 discarded. Split the cheap question from the expensive one: classify each failure with _classify_chunk_error in the loop, then build the single interruption after precedence settles. Failure precedence is unchanged -- cancellation still propagates, an unrecognized failure still wins over a transient sibling, and the first transient is still the one raised.
The duplication sub-score is dominated by five accepted getter families, so it barely moves when real duplication is added -- cutting a genuine clone took 26 fragments to 25 and left the score at 70. A sixth group appearing would be invisible in the number, so record the accepted five and let the weekly sweep diff against that instead, turning a new group into an event. Keyed by the set of function names in each group rather than line numbers, so editing above a getter does not churn the file. Two measurements are recorded with it so neither is re-derived: at similarity_threshold 0.55 (default 0.65) the package still reports exactly these five groups, so nothing latent sits below the bar; and stripping docstrings makes duplication far worse (60 -> 0, 11.3%% -> 30.0%% of fragments), because each docstring is one large unique node holding similarity under the threshold -- suppressing them is not available. The sub-threshold deprecated-NWIS family is recorded separately and excluded from the observed totals, since pyscn does not report it at default settings. CONTRIBUTING gains the rule these findings keep pointing at: check whether a leaf already generalizes a mechanism before writing a small helper. Three mechanisms have now been re-implemented beside an existing one, and nothing automated catches it -- the copies sit under the clone detector's size floor and neither couple nor complicate anything, so CBO and LCOM stay at 100 while the drift accumulates.
thodson-usgs
force-pushed
the
refactor/fanout-abstractions
branch
from
August 9, 2026 18:26
a930466 to
2d3d7aa
Compare
Corrects the direction of one claim and breaks up two dense passages. The delegation was described backwards: ChunkPlan does not delegate its total / iter_sub_args to the dunders, it keeps those as its own vocabulary and defines __len__ / __iter__ to delegate to them. Stated the wrong way round it reads as though the domain names were the derived ones, which inverts the reason the split works. The rest is readability. The paragraph arguing for standard protocols carried four claims across one eleven-line sentence chain; it is now three paragraphs, one claim each. The Compliance section stated three assertions as a single sentence with three trailing 'that' clauses, which is where a reader loses the thread; the three are now enumerated and the adapter-test note stands on its own.
Normalizes the one em dash this branch introduced to the `--` spelling the file otherwise uses (30 occurrences to 11 across docs/, so `--` is the convention), and unpicks the transport entry, where a forty-word sentence carried both what `fanout` owns and why it is the sole async entry point. Those are two claims and now two sentences, with the ragged wrap after them squared up. Scoped to prose this branch wrote. The rest of docs/ went through a readability pass in DOI-USGS#354; re-editing it here would be churn against deliberate choices, and the ten em dashes elsewhere are left as that pass left them.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Important
Depends on #355. This branch starts at #355's head (
d7b98eba). GitHub cannot use a fork-owned branch as an upstream PR's base ref, so the diff includes #355 until that merges; afterward it collapses to the seven commits below.Follow-up to #355: several abstractions there were carrying less than they cost, plus the structural work that followed. Seven single-concern commits.
1.
FanOutPlanstatedlenanditerunder private namesIt declared
total,canonical_url, anditer_sub_args(). Two problems: the executor never inspects what a plan yields —_pendingenumerates and hands the item straight to the adapter'sfetch— so the item type belongs to the adapter, not to adict[str, Any]wide enough to hide thatChunkPlanyields kwargs while_LocationPlanyielded{"request": r}and unboxed it two lines later. Andtotal/iter_sub_args()arelen/iterrenamed, so they can disagree — which is why a fitness function had to assert they agree.The protocol is now
__len__+__iter__, generic in the item type. A plainlistis a plan, so_LocationPlanand the boxing are gone.ChunkPlankeepstotal/iter_sub_argsas its domain vocabulary and delegates the dunders to them.canonical_urlmoves to aFanOutargument — it is stamped on the combined response, not a property of how work divides.Conformance moves from a runtime test to the type checker. The test it replaces existed, by its own docstring, because "a missing
canonical_urlwould surface as anAttributeErrormid-fan-out":2. Four exception-chain walks, three traversal policies
_deterministic_failure__cause__+__context___classify_chunk_error__cause__FanOut._normalize_failure__cause__FanOutInterrupted.__init__status hunt__cause__All four ask "first exception in this chain satisfying P", so they share one guarded
_walk_causesgenerator.follow_contextis a real axis, not two questions in one signature:_deterministic_failureneeds implicit chaining to reach asocket.gaierror, the others only follow explicitraise ... from. Behaviour was probed directly rather than inferred: permanent DNS via__context__stillTrue,EAI_AGAINstillFalse, a wrapped 429 still(QuotaExhausted, 12.0),status_codestill 429 from both the cause chain and the subclass default. The two unguarded walks no longer spin on a self-referencing chain; a regression test closes that cycle deliberately.3–4. One home per policy, one executor per retrieval path
Shared policies consolidated (
parse_retry_aftertoexceptions,_read_env_numbertotransport/env.py, three divergent next-page-link checks totransport/links.py::resolve_next_url), legacy query plumbing split into_querying.py, and every retrieval path routed through the same executor.5.
progressscopes through theAmbientleafprogress._use_reporterheld the identical set/reset-token dance_ambient.Ambient.__call__exists to own. Found by scanning below pyscn's clone floor, where two eight-line helpers are invisible to it.6. One interruption per fan-out, not one per failure
FanOut._runaskedwrap_failure()about every failed sub-request to let a non-transient sibling surface raw, but kept only the first transient. Each call constructs aFanOutInterrupted, whose__init__snapshotspartial_frame.copy()— a full concat over every completed sub-request. A batch failing together on one 429 is routine, so N failures meant N snapshots with N−1 discarded. Classification is now separated from construction; failure precedence is unchanged.7. Clone baseline + the leaf-reuse rule
The duplication sub-score is dominated by five accepted getter families, so it barely moves when real duplication is added — cutting a genuine clone took 26 fragments to 25 and left the score at 70.
.pyscn-known-clones.jsonrecords the accepted five, keyed by function-name sets so edits don't churn it, and turns a sixth group into a visible event.Two measurements are recorded so neither is re-derived: at
similarity_threshold0.55 (default 0.65) the package still reports exactly these five groups, so nothing latent sits below the bar; and stripping docstrings makes duplication far worse (60 → 0, 11.3% → 30.0% of fragments) — each docstring is one large unique node holding similarity under the threshold, so suppressing them is not available.CONTRIBUTING gains the rule these findings keep pointing at: check whether a leaf already generalizes a mechanism before writing a small helper. Three have now been re-implemented beside an existing one, and nothing automated catches it — the copies sit under the clone detector's size floor and neither couple nor complicate anything, so CBO and LCOM stay at 100 while the drift accumulates.
Health
pyscn composite 80 → 82. Dependencies 75 → 80 (max depth 9 → 8), Architecture 85 → 87 with zero error-severity violations remaining, Duplication 70 with 5 known groups and 0 new, Complexity 95, Dead Code / CBO / LCOM 100.
Two candidates were attempted and rejected on measurement, both recorded rather than quietly dropped:
ogc.engine's page walk intoogc/paging.py. Worked as designed — engine's fan-out fell 10 → 8, tests and mypy green — but the new module arrived with fan-out 6 of its own, total dependencies rose 141 → 145, weighted violations 26 → 27, compliance 87.19% → 87.02%. Reverted. engine's fan-out measured something true but not something wrong: it is the orchestrator.waterdata/utilssplit, whose entire gain was denominator drift from adding modules.Verification
mypy: clean across 57 source filespre-commit run --all-files: 14 hooks passxenon,complexipy,lint-imports: pass🤖 Generated with Claude Code
https://claude.ai/code/session_01DSMyMqQ4uuQ9TAbJLnJ6Yi