Repository navigation
0.8.0: codebase simplification, verification fixes, and CI - #3
Merged
Merged
Conversation
Upstream compatibility review of USAspending API releases April-June 2026 found no breaking changes; 0.7.2 already handles the spending_by_award Pydantic migration, the program_activities rename, and the F001-F010 assistance type codes. - Add AwardsSearch.object_classes() for spending_by_award and spending_by_award_count; SubAwardsSearch.object_classes() raises ValidationError (awards-only per the API); downloads.search() emits a UserWarning because /download/search/ silently drops the key - Revalidate all README examples against live data: fix two broken chains (formatted_address, subaward place_of_performance), refresh stale outputs, add client.spending rollup and len(query) examples - Add tests/test_readme_examples.py: executes runnable README blocks verbatim against the live API (pytest -m integration), with a network-free extraction guard in the default suite; share the live client fixture via conftest.py - Correct api-docs-links.md subawards entry and add missing entries for /api/v2/idvs/awards/ and the TAS filter-tree endpoints Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Establishes the guards the DRY refactor depends on. The existing suite has blind spots that would let later phases stay green while regressing, so this lands first. Golden-master integration suite (tests/test_golden_master_integration.py): fetches fixed live records, reads their public properties, and snapshots them to tests/fixtures/golden/. It records values at the altitude the library owns: scalars exactly (parsed dates, coerced Decimals, resolved codes), containers by type and emptiness only, `raw` not at all. Volatile figures are recorded as a type tag taken from the live object, so a date-becomes-str regression is caught while the figure itself may move. Network-backed Agency aggregates are recorded as such rather than read, with one focused test covering them; this keeps the suite to one request per anchor. Verified by mutation: breaking Contract.naics_code fails two snapshots with an exact diff. Characterization tests (tests/test_characterization.py): only behavior not already pinned elsewhere. Covers the Award __class__ upgrade on lazy load (previously untested, and the mechanism parent_award depends on), Loan's deliberate cfda_number override, the unguarded AwardAccount repr, and SpendingSearch.count ignoring default_result_limit. The module docstring names the existing tests that already pin the rest, including the two files Phase 4 must update. Also records a live defect: recipient-ID normalization is implemented twice with different algorithms (the model takes the first token, the query prefers 'R'), so the same raw ID resolves to different entities depending on the path. Public-API name-set snapshot guards against signature drift. A real-YAML test covers special_cases.yaml, which every existing formatter test mocks. Snapshots fail rather than self-heal when missing, so an absent baseline cannot look like a pass; recording one is explicit via a regen environment variable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014d1wFpykqretUik9rQpALH
Award-type knowledge was projected six different ways across three layers, each
hand-maintained: AWARD_TYPE_GROUPS held the codes, get_award_group derived a
singular name via rstrip("s"), award_factory mapped that name to a model class,
awards_search kept a group-to-API-count-key dict plus two if/elif chains (one
selecting a model's SEARCH_FIELDS, one setting the result category),
award_resource kept the inverse of the count-key dict to pick a search method,
and award.py dispatched download types to three DownloadResource wrappers.
Changing a category meant editing all of them in lockstep.
AWARD_CATEGORIES now holds each category once, with the fields every layer
needs, and the consumers read from it. All the dispatch dicts and if/elif chains
are gone. Derived constants (AWARD_TYPE_GROUPS, AWARD_TYPE_DESCRIPTIONS, the
per-category code frozensets, ALL_AWARD_CODES) are computed from the table, and
verified byte-identical to the previous hardcoded values.
Two couplings replace tables rather than adding one: each group name is also its
AwardsSearch convenience-method name, and each download type is also its
DownloadResource method name, so award.py resolves the download method by name
instead of branching. Award.download_type validates against registry-derived
DOWNLOAD_TYPES first, because DownloadResource has other public methods and an
unvalidated string would otherwise reach one. tests/models/
test_award_category_registry.py asserts every coupling, so a category that
forgets one fails immediately instead of at the call site.
model_for_name() in award_factory is now the single place binding a registry
model name to its class, replacing two identical maps (one of which was rebuilt
on every create_award call) and letting queries/ stop importing the concrete
award models entirely. It is lru_cached, removing ~1.2 us of repeated deferred
imports per award.
Also: AwardCategory.codes is a cached_property rather than rebuilding a
frozenset per access, and get_award_group resolves through indexes instead of
scanning all 33 descriptions. categories_for_codes 1.45 -> 0.43 us,
get_award_group worst case 1.98 -> 0.22 us. Both matter because
_transform_result runs per result row.
Golden-master integration snapshots are unchanged, confirming the consolidation
is behavior-preserving against live API data.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014d1wFpykqretUik9rQpALH
Every chainable filter method on SearchQueryBuilder ended in the same three
lines: clone, append the filter object, return the clone. That was repeated 21
times, so each method carried the mechanics of immutability alongside its own
argument validation.
_with_filter() owns the clone-and-append, and the 21 methods reduce to a single
expression returning it. Net 20 fewer lines despite adding the helper, and the
next filter method cannot get the clone protocol subtly wrong.
Also fixes a typing bug in those same signatures. They were annotated
`def f(self: T, ...) -> T`, but T is the *element* type of BaseQuery[T] -- an
Award, a Transaction. That types `search().keywords("x")` as returning an Award
rather than the builder, so chained calls resolved against the wrong type in
editors. Introduces SQB, a TypeVar bound to the builder, and applies it to all
28 self annotations and 34 return annotations, plus the six signatures that took
a bare self. T keeps its correct meaning in QueryBuilder (__iter__, first, all,
_transform_result, __getitem__).
Behavior-preserving: unit suite and golden-master integration snapshots
unchanged.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014d1wFpykqretUik9rQpALH
_clone() was implemented 16 times in two incompatible styles. Seven subclasses called super()._clone() and patched their own fields, which is fine. The other five could not, because their __init__ requires arguments (a toptier code, an award ID, a warn flag), so each rebuilt the instance and then hand-copied the five base pagination fields. AwardingAgenciesSearch and FundingAgenciesSearch were 14-line verbatim copies of each other. _clone() now lives once on BaseQuery as "construct, then copy state", with two overridable pieces: _new_instance() for construction and _copy_base_state_into() for state. Subclasses with demanding constructors override the former in two lines instead of duplicating the latter. The copy chain follows the class hierarchy, so QueryBuilder adds _filter_objects and ClientSideQueryBuilder adds _predicate_filters on top of the base pagination fields, and a new base field now reaches every subclass automatically. The two deprecated agency searches lose their _clone entirely, keeping only a _new_instance that passes warn=False. That flag was the sole reason those bodies existed, and dropping them naively would have emitted a DeprecationWarning on every chained filter call for two public classes; the Phase 0 characterization test covers it. Also removes a redundant _filter_objects copy in TransactionsSearch._clone that _copy_base_state_into already performed. _cached_count is deliberately still not copied, and is documented as such: a fresh instance starts with none, and a clone's filters may differ. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014d1wFpykqretUik9rQpALH
Fixes a real bug first. _cached_count meant two different things: count()
implementations stored the figure the API reported, while the base
_get_cached_count() stored that figure after limit()/max_pages() capping. Both
used the same field, so indexing a limited query cached the capped value and the
next count() returned it. Concretely, on a query with limit(1) over 2 accounts,
query[0] followed by count() returned 1 instead of 2. The two caches are now
distinct fields, and a regression test covers the sequence; it fails on the
previous code.
With that untangled, the per-endpoint count() bodies collapse onto three shared
mechanisms on QueryBuilder, each covering one thing the API actually offers:
_count_via_endpoint - a dedicated count endpoint (transactions, subawards)
_count_via_page_metadata - a total in page_metadata (sub-agencies, accounts)
_count_by_iteration - walking results, for endpoints with no count at all
(funding, IDV children, unscoped subawards)
Each now supplies an endpoint or a metadata key rather than repeating the
request, the caching and the logging. Two function-local log_query_execution
imports go away, and every count path caches consistently instead of only
AwardAccountsQuery doing so.
Iteration is documented as a correct mechanism rather than a fallback to be
optimized away: several endpoints report no count, and for those it is the only
option. AwardsSearch keeps its own implementation because it selects a category
from a counts-by-type response, and AgenciesSearch because its autocomplete
response is not paginated. SpendingSearch keeps its limit-aware loop, which
stops early rather than walking every page.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014d1wFpykqretUik9rQpALH
Corrects the previous commit. 2c added a count cache, which contradicted a contract the suite documents explicitly: "The public count() method always calls the API. Caching is internal via _get_cached_count(), used by __getitem__." Ten of eleven builders honored that; AwardAccountsQuery was the deviation, and 2c generalized the deviation instead of the rule. Caching also reintroduced the very bug 2c set out to fix, by another door. _count_by_iteration counted via __iter__, which substitutes config.default_result_limit (default 10,000) when no explicit limit is set, so count() returned a silently truncated total and then cached it. Demonstrated: with 25 records and the default limit at 10, count() returned 10. Removing the cache fixes both at once and removes state rather than adding it. The original 2c bug cannot recur either: count() always re-reads from the API, so a capped figure has nothing to leak through. The regression test from 2c still passes, and new tests pin that the default fetch limit never reaches a count while an explicit limit() still does. count() is now a single template method owning the logging, with _compute_raw_count() as the seam each builder implements. That deletes nine copies of the same debug line, five hand-rolled result-log lines, and the guard that was duplicated across the three mechanisms. _count_via_paging replaces _count_by_iteration and sums raw response rows instead of iterating models, so counting no longer builds a model per row only to discard it. SpendingSearch's 56-line hand-rolled pagination loop was already exactly this, and collapses onto it. Also from review: - _count_via_endpoint takes method and json, so the two POST-based counts (awards, recipients) can use it, and it no longer logs filters for endpoints that scope themselves by URL. - _copy_base_state_into is gone. Introducing _new_instance in 2b removed its reason to exist, and no leaf class ever used it; every level now uses the one super()._clone()-and-patch idiom. - AwardsSearch: the two filter methods the 2a sweep missed now use _with_filter, and the duplicated award-type requirement plus its double filter aggregation collapse into _require_award_type_filters. - All six _new_instance overrides use self.__class__, so a future subclass cannot silently clone to its parent type. - SubAgencyQuery drops a function-local log import and a hardcoded class name. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014d1wFpykqretUik9rQpALH
Four builders each carried their own copy of "scope this query to one award": the _award_id attribute, its clone copy, an identical award_id() method, and an identical required-award guard in _build_payload. Three separately implemented sorting, each with a SORT_FIELD_MAP plus a hardcoded list of valid API field names that duplicated that map's own values -- and the copies had already drifted in form, two hardcoding the list and one deriving it. Both concerns are now mixins in queries/mixins.py, each owning one cohesive slice of state, carrying it across clones, and exposing the method that sets it. Neither reads the other's state, so composing them needs no understanding of the combination. Valid sort fields derive from SORT_FIELD_MAP, which is what kept drifting; verified the derived sets are identical to the deleted lists. FundingSearch and AwardAccountsQuery use both; IDVChildAwardsSearch uses only SortableQuery, since its award ID is a constructor invariant rather than an optional filter; TransactionsSearch uses only AwardScopedQuery, since it validates sorts against a frozenset with no friendly-name map. SubAwardsSearch stays out: its award scope is one optional filter among many. 825 lines across the four builders become 707 including the new module. Defaults are class attributes rather than cooperative __init__ assignments. A mixin __init__ would have replaced the concrete builder's constructor signature with (*args, **kwargs) in help() and editor hovers, which review caught; the values are immutable, so class attributes are safe and leave signatures intact. Self-typed TypeVars keep a chained call resolving to the concrete builder rather than widening to the mixin. Also fixes a defect review found in TransactionsSearch: its count endpoint interpolated _award_id directly, so an unset award requested /awards/count/transaction/None/ instead of raising. It now uses the same guard _build_payload already used. Added a loud failure for a class that declares no default sort field, rather than silently sending an empty sort. Verified by execution, not inspection: every attribute of all four builders survives the new clone chain, MRO resolves as intended for the two-mixin cases, and the public API surface snapshot is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014d1wFpykqretUik9rQpALH
Four cleanups that finish the query-layer scaffolding work. QueryBuilder._execute_query now reads an _http_method class attribute and sends its payload as query parameters for GET, or a JSON body otherwise. SubAgencyQuery had copied the whole method verbatim to change one word; the copy's real cost was re-typing the query logging, the payload debug line and the page-metadata logging, all of which drift independently. It now declares the verb and inherits the rest. The method is typed Literal["GET", "POST"], since that is the actual domain. validate_toptier_code replaces three byte-identical validators in agency_query, agency_award_summary and sub_agency_query. Both error messages are preserved exactly, as tests match on each. Two inputs improve incidentally: None and 0 now report "toptier_code is required" rather than interpolating "None" into the malformed-code message, which is what two of the three copies already did. AgencyAwardSummary no longer subclasses SingleResourceBase. That base is for "fetch one model by id" endpoints; this class returns an aggregate dictionary, so it satisfied the base's find_by_id contract only by raising NotImplementedError, and it used nothing else from the base. Its _endpoint and _construct_endpoint members existed only to fulfil the base's shape, so with one call site and no subclass the URL is now built inline and verified through the request the client actually makes. Its test class asserted the NotImplementedError, so that test is replaced by one stating the intent: it is not a resource finder. Seven annotation-only `from ..client import USASpendingClient` imports move under TYPE_CHECKING, matching the ~30 modules that already guard it. These worked only because the modules are themselves imported lazily from inside resource methods, so client.py finished importing first; the guard removes a latent cycle. Also removed a pair of orphaned comments in spending_search describing imports that no longer exist, and relocated a third to the line it actually explains. Deferred to their own commit, and recorded in the plan: the four agency_type validators produce three different tested messages, so consolidating them is a behavior change best paired with adopting the AgencyType/AgencyTier enums in parse_agency_spec; and AgenciesSearch._execute_query still hardcodes POST and skips query logging, which wants a response-reshaping seam rather than a full override. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014d1wFpykqretUik9rQpALH
FederalAccountsQuery and TASCodesQuery read sibling levels of the same /references/filter_tree/ hierarchy, which returns a whole level unpaginated. Both had independently arrived at the same design -- fetch once, cache, filter in memory -- and were ~90% identical: the same cache handling, the same code/codes/description filters, the same clone-shares-results semantics, and the same "no scope yet means no results" rule expressed as two different inline comments. FilterTreeQuery owns all of that. A subclass supplies the level it reads (_scope), how a response row becomes a model (_build_model), how to reconstruct itself (_new_instance), and the filters specific to its level. 412 lines become 400 with the duplication gone, and the shared behavior is stated once. _scope() returns the hierarchy path, which is one value with three consumers: it formats the endpoint, it decides whether the query is answerable, and it names the scope in repr. Returning an empty value is a legitimate state, not an error, because a model that does not yet know its agency code constructs exactly that; the rule now lives on the abstract method where a subclass author reads it. Two silent failure modes are now loud, which review flagged as the highest value-per-line change here. _new_instance is abstract: a subclass that omitted it used to construct fine and then fail on its first filter call with a confusing error about an unexpected keyword argument. ENDPOINT has no default: a subclass that omitted it used to GET the API root. Both now fail at the point of the mistake, and tests cover each. Also from review: repr coerces its scope values, since _fetch tolerates any falsy scope and a repr that raises is worse than useless in a log line; the class-level field lists are tuples rather than mutable lists on an ABC; the base exposes an _endpoint property so this reads like every other query in the package; and the two subclasses lose loggers that no longer log anything. CLAUDE.md's query-builder section documented a hierarchy and an abstract-method list that several commits in this phase have since changed, so it now describes the current seams, including the mixins and the count mechanisms. Kept deliberately separate: each subclass keeps its own fiscal_year (different arity, different semantics, different cost) and its own description alias, whose entire value is the public name and docstring. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014d1wFpykqretUik9rQpALH
Contract and IDV are both procurement instruments and had independently grown the same members. An AST comparison of their bodies, ignoring docstrings, found seven byte-identical: piid, base_exercised_options, base_and_all_options, psc_code, psc_hierarchy, naics_hierarchy and latest_transaction_contract_data. Those move to a ProcurementAward base. IDV also carried a place_of_performance that was character-for-character the one it already inherited from Award, including the all-null guard written for IDVs specifically. The same comparison against Grant showed five of Loan's overrides were exact copies: fain, uri, cfda_info, primary_cfda_info and sai_number. Removed. total_subsidy_cost is kept because it genuinely differs, preferring the flat "Subsidy Cost" key award search returns. Four classification accessors stay per-subclass because they are not identical: Contract resolves naics_code, naics_description and psc_description through more fallback sources than IDV does, and the two define contract_award_type differently. Unifying them would change reported values, so the difference is preserved and each docstring now states which sources it actually consults, verified against the bodies. Notably this did not hold for the sixth Loan override. Review checked the claim that cfda_number "genuinely differs" and found the justification inverted: Grant's version is a strict superset whose last tier is Loan's entire body, so the override can only ever find less. On a detail response, which nests CFDA data under cfda_info and leaves the flat key null, it returns None where the inherited version returns the number -- confirmed at 'None' vs '84.268' against the loan fixture, and visible in the recorded golden master. The override is kept, because removing it changes behavior, but the comment now says that plainly instead of asserting the opposite. The existing test reads `if loan.cfda_number: ... else: assert ... is None`, so it passes either way; the fix and the tightened test are recorded together in the plan. 555 lines across the three subclasses become 452 including the new base. Public surfaces are unchanged, since dir() walks the MRO, and the golden-master snapshots are unchanged, confirming behavior is preserved against live data. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014d1wFpykqretUik9rQpALH
Award's four agency accessors were two verbatim pairs differing only by a "funding"/"awarding" literal that _load_agency_data already threads. They now delegate to _build_agency() and _build_subtier_agency(), leaving four one-line cached_propertys. Private builders sit above their public consumers, matching _load_agency_data and _derived_award_identifier in the same file. Agency's six per-category *_obligations properties each repeated the same two-line body over _get_award_summary(); they now call the existing public get_obligations(award_type_codes=...), whose defaults were verified character-identical to the deleted code. total_obligations is unchanged: it reads raw data first and only falls back, so it is a different shape. Behavior-preserving. cached_property still collapses repeat access to one request; _load_agency_data runs twice per side but is pure dict work, since LazyRecord guards the single detail fetch with _details_fetched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014d1wFpykqretUik9rQpALH
Agency.__init__ took a third parameter, stored it as self._subtier_data, and nothing in src/ or tests/ ever read it. The docstring claimed it backed a "subtier_agency property" that does not exist, and the tests said so outright: "passed to constructor but not exposed as a property". Subtier data has its own model, SubTierAgency. Only one of the seven Agency() call sites passed it, and that value was discarded, so removing the parameter also erases an asymmetry in the two builders added in Phase 3b: _build_agency passed subtier_agency through raw while _build_subtier_agency enriched it with office_agency_name first. Now only the latter touches it. Deleted test_agency_with_subtier_data: it existed to exercise the parameter, and its three assertions were a strict subset of test_agency_with_toptier_data_only, which builds from identical data. Agency is publicly exported, so this narrows a public constructor. Recorded under Unreleased in CHANGELOG.md; passing a third argument was already a no-op, so the resulting object is identical either way. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014d1wFpykqretUik9rQpALH
Nine sites hand-rolled an integer coercion the library already owns. to_int was already the convention at 11 pre-existing sites, including award_account.py's byte-identical funding_agency_id getter, so these were the outliers; git history confirms adoption rather than drift, since the hand-rolled form landed in c817e94 before to_int arrived in 66a9fd4. funding.py x5, subaward.py x1 int(v) if v is not None else None treasury_account_symbol.py x2 int(v) if v else None award.py subaward_count int(...) -> to_int(...) or 0, matching the neighbouring to_decimal(...) or Decimal("0.00") utils/retry.py 9 lines that were literally to_int's body Behavior change, narrow but real: a non-numeric value in a numeric API field now yields None instead of raising ValueError out of a property. The old code could not even repr() the object. This matches the library's existing split, where user input raises ValidationError and API-supplied values coerce leniently. The two TAS sites are exactly equivalent, since bpoa/epoa come from \d{4} regex groups. retry.py is verified identical across every input shape, including HTTP-date and "0". Also gave to_int and to_float the `if x is None: return None` guard to_decimal already had. to_int(None) reached int(None) purely to catch the TypeError, which is the common path for optional fields: subaward.prime_award_internal_id is present-but-null in every fixture row, where it measured 2.3x slower. to_int and to_float had zero direct tests while backing 20 call sites, so tests/utils/test_formatter.py gains parametrized suites for both. Skipped the _money/_date/_int helper layer the plan floated. The reason is not default variance, which is only observably 2 across the 38 coercion sites, but that get_value and _lazy_get differ in whether they can fire a network request, and Award uses both deliberately in one class. A shared seam would make award.py:458's local dict read and award.py:300's HTTP fetch look identical. Revisit after Phase 4 sets one missing-money policy, since funding.py and grant.py currently disagree for the same field name. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014d1wFpykqretUik9rQpALH
Two defects, each proven against the old code before the fix. Transaction was declared @DataClass with no fields. The generated __eq__ compared empty tuples, so every Transaction was equal to every other one regardless of its data, and __hash__ was set to None, leaving instances unhashable. Nothing in src/ or tests/ relied on either, and no other model defines __eq__, so removing the decorator restores identity semantics. This also turned Transaction.__init__ into a pure pass-through: the explicit __init__ was previously load-bearing, since @DataClass would otherwise have generated __init__(self), so it is now deleted along with three sibling pass-throughs in Spending, StateSpending and DistrictSpending. FederalAccount.count passed self.tas_codes.count() as a default ARGUMENT. Python evaluates arguments eagerly, so every access fired a TAS-code request and discarded the result whenever the response already carried the count. It now checks for the key first, and is a cached_property so the fallback costs one request per instance instead of one per access, since tas_codes hands back a fresh query whose own result cache is thrown away. While in transaction.py: routed its 12 raw.get reads through get_value, the accessor every sibling model uses; dropped a default="" that never applied, since smart_sentence_case(None) already returns ""; and stopped re-coercing amt, whose operands are already coerced by their own getters. to_decimal was verified idempotent and the trailing `or None` is load-bearing, since Decimal("0.00") is falsy. transaction.py was also the only file under models/ without `from __future__ import annotations`, and the only one using Optional[X] in annotations, which is why ruff's own UP rules never fired on it. Deliberately NOT done: the plan's third item, moving Spending and SubTierAgency onto ClientAwareModel. The premise was that they forgo weakref safety by hand-storing the client, but no property on either class issues a request, so the switch bought nothing and broke two working behaviors. SubTierAgency.offices reshapes local data only, yet raised DetachedInstanceError once the client was collected, where it previously returned the offices. And reattach(recursive=True) recurses only into LazyRecord, which neither class is, so Award's cached subtier children were silently never reattached. Both were reproduced, and the one-line get_value cleanups in spending.py are kept. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014d1wFpykqretUik9rQpALH
code, amount, total_outlays and spending_level were duplicated verbatim between Spending and RecipientSpending, which cannot share a base because RecipientSpending must be a Recipient. A SpendingMixin composed ahead of each model base removes the copy: Spending(SpendingMixin, BaseModel) and RecipientSpending(SpendingMixin, Recipient). The mixin deliberately omits name, id and category. Recipient.name applies title casing, and a mixin accessor ahead of it in the MRO would silently replace it with the raw API value. Verified by mutation: injecting name into the mixin fails four existing fixture tests with 'THE BOEING COMPANY' where 'The Boeing Company' is expected. Enumerated every attribute the mixin defines against the full MRO of all four consumers: zero conflicts, so name is the only exclusion the design needs. StateSpending's and DistrictSpending's __repr__ were byte-identical to Spending's apart from the class name and an "Unknown ..." label, so __repr__ now reads type(self).__name__ and subclasses set a _UNKNOWN_NAME hook. Output was captured before and after across five input shapes for all three classes and is identical. RecipientSpending keeps its own repr: round_to_millions renders recipient totals as "$13.3 billion", which is a deliberate display choice, not drift. Its dead `or 0` is gone, since round_to_millions(None) returns "$0.00". RecipientSpending.__init__ is kept despite looking like a pass-through: Recipient names its first parameter data_or_id, so deleting it would rename a public keyword argument. Tests: replaced 51 lines of new assertions with one. Mutation testing showed the rest was already covered, and that total_outlays on RecipientSpending was the only real gap, now closed in the existing fixture test. Breaking the key fails both hierarchies. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014d1wFpykqretUik9rQpALH
Award._fetch_details rebinds self.__class__ to an Award subclass once the detail response reveals the type code. Nothing said why, so it read like a workaround. It is deliberate late binding: an award can be built before its type is knowable, from an ID alone or from SubAward.parent_award and Award.parent_award, which construct a bare Award from an identifier carried on the child record. Returning the base class permanently would deny callers every subtype member, and returning a fresh object would leave the caller holding a stale instance, since the upgrade is triggered by attribute access on an award they already hold. The type decision is not duplicated here either; it is delegated to create_award, the same factory the resource layer uses. Removal stays out of scope, and tests/test_characterization.py pins both upgrade paths. Also corrected the SpendingMixin docstring from the previous commit. It claimed name, id and category were all excluded because a mixin ahead of Recipient in the MRO would shadow them. That rationale holds for name only: Recipient defines neither id nor category. The three now carry their actual reasons. Added a warning that anything placed on the mixin outranks the whole host hierarchy, BaseModel included, so declaring the get_value dependency as a stub would shadow the real implementation and break every accessor. Verified: such a stub does resolve ahead of BaseModel.get_value and raises from Spending.code. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014d1wFpykqretUik9rQpALH
Award._derived_award_identifier was a pure function of a string with one caller, so it moves to models/award_identifier.py as parse_award_identifier. The generated-ID formats and the reason the segment counts are exact rather than minimums are now documented in one place, and the parser has its own test module: 31 parametrized cases built from real fixture IDs, covering all four prefixes, the "-NONE-" placeholder, and malformed input. Proved equivalent to the original across 177 inputs by differential test before wiring it in. The old test called the private method and became a delegation test. _load_agency_data carried two near-identical branches, one per agency side, hardcoding eight Title Case column names between them. Detail responses nest the agency under a snake_case key while search results flatten it into columns that differ only by the "Funding"/"Awarding" prefix, so all six keys now derive from agency_type. Verified every derived key byte-identical to the hardcoded one; the flat path is covered for both sides by test_award_agency_integration. award.py: 914 -> 857 lines. NOT done: the plan's second extraction, a models/_field_mapping.py unifying the flat-to-nested reconciliation in _load_agency_data, recipient and period_of_performance. The three do share a nested-then-flat-then-fetch skeleton, but their tier boundaries differ in ways a shared helper would flatten: _load_agency_data tests the nested value for truthiness where the others test isinstance dict, its fetch tier calls _lazy_get where the others call _ensure_details and re-read, period_of_performance constructs its wrapper unconditionally so it returns a wrapper around None where recipient returns None, and recipient attaches a Location after building. A helper taking that many callables reads worse than the explicit code. Nor is award.py's remaining length a reason to split further. Of 857 lines only 297 are code: 344 are docstrings, 57 comments, 159 blank. Across 48 members the median is 9 lines and the largest is 56. The length is documentation volume, not complexity, and scattering a cohesive model across modules to lower a line count would cost discoverability for no reduction in what a reader must understand. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014d1wFpykqretUik9rQpALH
Rename SpendingMixin to SpendingFields. It was the only "Mixin"-suffixed class in the package, against the convention set by queries/mixins.py earlier in this refactor, where AwardScopedQuery and SortableQuery are named for the concern they own. The suffix restates what the class statement already shows; the concern name says what the fields are for. Unexported and referenced in two files, so the rename is contained. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014d1wFpykqretUik9rQpALH
The /idvs/awards/ endpoint reuses funding_agency and awarding_agency for a plain
agency-name string rather than a nested agency record. _load_agency_data
accepted any truthy value under those keys, so it handed a str to the builders
and all four agency accessors raised
AttributeError: 'str' object has no attribute 'get'
for every child award. That is reachable through the documented public path,
iterating idv.child_awards, which the property's own docstring demonstrates.
Reproduced against release/v0.7.3 in a scratch worktree, so it ships today.
Both exit points now enforce the dict-or-None the signature already promised.
Guarding only the nested tier is not enough: _lazy_get will not fetch past a key
that is already present, so it returns the same string from the lazy tier. The
accessors now yield None, and the name is still readable through raw().
Two adjacent fixes from the same review:
Invalid agency_type raised a bare ValueError while every other agency-type check
in the library raises ValidationError, which CLAUDE.md assigns to invalid
parameters. ValidationError subclasses ValueError, so this widens what callers
can catch without breaking `except ValueError`.
The id_key variable held one of two values and was read once, so the id now
derives from agency_type like the other keys. Verified across every fixture that
no payload reaching the flat tier carries a top-level funding_agency_id, so the
result is unchanged; if the API ever adds one, using it beats discarding it. The
comment claiming search results have no funding equivalent is narrowed to
spending_by_award, since child-award records carry both.
Tests: five cases, four of which fail on the pre-fix file with the exact
AttributeError.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014d1wFpykqretUik9rQpALH
The only intentional behavior change in this refactor. Optional getters
fabricated a value when the API reported nothing, so "this award has no such
figure" and "this figure is zero" both produced Decimal("0.00") and the
distinction was unrecoverable. They now return None; a zero the API actually
reports still returns Decimal("0.00").
Award total_obligation, award_amount, covid19_obligations,
covid19_outlays, infrastructure_obligations,
infrastructure_outlays
AwardAccount total_transaction_obligated_amount, obligated_amount
Funding transaction_obligated_amount, gross_outlay_amount
Location zip5, district
The four COVID and infrastructure getters also passed default=0 to _lazy_get, so
both that and the `or Decimal("0.00")` had to go. Confirmed default= affects only
the returned value, never whether a lazy fetch fires: that decision reads the
keys alone.
Four of these already declared Optional while never returning None, so the fix
makes the annotation truthful rather than changing a contract: Funding's two, and
Location.zip5 and Location.district. Funding also contradicted Grant, which
already returned None for the identically named transaction_obligated_amount.
Swept every consumer that would now break. Nine docstring examples formatted
these with :,.2f and would raise TypeError if copied, so they gained the same
`or 0` fallback as the README example. AwardAccount.__repr__ was the one
unguarded money format in src/, and now matches its siblings. No arithmetic,
comparison or sort in src/ consumes these values.
Golden masters regenerated deliberately, the only phase permitted to. Reviewed
property by property: 4 properties x 6 award records, plus
Funding.transaction_obligated_amount on the live funding record. All
Decimal -> None, nothing else moved. total_obligation, award_amount, zip5 and
district were unchanged against live data because the API does report those,
so the change bites exactly where data is genuinely absent.
Award.award_identifier deliberately keeps returning "". It is annotated -> str,
so unlike Location.zip5 there was no Optional annotation to reconcile it with.
The plan asked to normalize it; that rested on a misreading of its signature.
Migration notes in CHANGELOG.md, including that `== 0` on these properties was
previously true for every award lacking the figure and is now false, with
`is None` identifying exactly those.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014d1wFpykqretUik9rQpALH
The completeness sweep for Phase 4 ran BEFORE I widened the change to funding.py, and I never re-swept. That left resources/funding_resource.py:38 formatting record.transaction_obligated_amount with :,.2f after I had made it Optional. Not hypothetical: the regenerated golden shows the live first record is null, so copying that example raised TypeError. Found by review, not by me. Normalized the last two getters in the same defect class. Award.type and Award.type_description declared Optional[str] while returning "" via default="", exactly the mismatch cited to justify changing Location.zip5. Leaving them meant the stated principle had known counterexamples on the most used model in the library. Award.award_type_code and Contract.contract_award_type follow, since both delegate. Award-type dispatch is unaffected, as create_award reads the raw payload rather than these properties; verified the factory still returns Contract for type "A", and no golden master moved, so the API reports both fields for every record in the suite. Also guarded four pre-existing unguarded money formats, in transactions_resource, subawards_resource, subawards_search and subaward. Those properties were already Optional, so those examples were already broken; this phase established the `or 0` convention and they were the visible outliers. Fixed three examples referencing properties that do not exist. client.py used award.amount and award.recipient_name, neither defined on Award, and the IDV child-award examples used child.obligated_amount, which no Award subclass defines. I had added `or 0` to those two without checking the attribute existed, so the guard sat on a line that raised AttributeError first. A sweep over src/ now reports no unguarded money format anywhere. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014d1wFpykqretUik9rQpALH
Recipient-ID normalization was implemented twice with algorithms that
disagreed. Recipient._clean_recipient_id kept the FIRST level of a
"<hash>-['C','R']" suffix; RecipientQuery._clean_recipient_id PREFERRED 'R'. The
same raw ID therefore addressed <hash>-C through a model and <hash>-R through
client.recipients.find_by_id(). Six such IDs appear in the captured fixtures, so
this reached real data rather than being a theoretical duplication.
Both now call one normalize_recipient_id() in utils/validations.py, which keeps
the first level. Choosing between the two was settled by measurement, not
preference: for all six multi-level IDs, the live /recipient/{id}/ endpoint
reports the recipient's spending at the FIRST level and reports zero at the
trailing 'R' in four of six. One example gives $24.6M over 109 transactions at
-C and $0 over 0 transactions at -R. I initially consolidated on 'R', reasoning
that a caller asking for a recipient wants the recipient rather than its
child-relationship view; the data refuted that, since it would have silently
zeroed real totals. The finder path changes as a result, and it changes toward
reporting data instead of zeros. Nothing is lost: a -C record still carries
parent_name, which Recipient.parent reads.
The consolidated helper also takes the model version's defensive handling, which
the query version lacked: non-string passthrough, whitespace strip, trailing
slash strip.
Also documented the _fetch_details contract on LazyRecord. Verified each claim
against the code: Agency asks a query object for a dict and swallows failures,
Recipient issues a direct request and swallows, Award goes through the public
resource, reads .raw off the returned model, and re-raises. Award is the outlier
on both counts and stays as-is, entangled with the __class__ reassignment. The
practical consequence is recorded too: a failed lazy load does not reliably
surface as an exception, since two of three report it as absent data.
Tests split by what they pin: the normalizer's edge cases in
tests/utils/test_validations.py, cross-path agreement in the characterization
suite, and construction behavior in test_recipient.py. The characterization test
that pinned the DIVERGENCE now pins the agreement, which is the property that
was broken.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014d1wFpykqretUik9rQpALH
…nders client.tas.agencies fired an HTTP request on every attribute read: an uncached property doing I/O, the same defect class as FederalAccount.count in Phase 3e. Measured three requests for three accesses. It also reached for client._make_request directly, the only resource that did, and hand-built Agency models inline. All three follow from it not using the abstraction the package already has. TASAgenciesQuery is the root of the same tree FederalAccountsQuery and TASCodesQuery read further down, so it fits FilterTreeQuery exactly: fetch one unpaginated level, cache it, filter in memory. The transport call and the model building move into the base and the _build_model hook, and the resource keeps returning list[Agency] from a cached_property, so no caller changes. Three accesses now cost one request, and len plus three filters also cost one, since the base shares its cache across clones. Two things surfaced while writing it. The default CODE_KEY of "id" matched nothing, because Agency.id is the internal database ID and is None for a filter-tree row while the agency code lives on .code; the base's own docstring warns about exactly this. KEYWORD_FIELDS needed narrowing too, as Agency has no description. Both overrides are covered, and reverting either fails four tests. Being the first scope-less level, it also exposed the base repr assuming every level has a scope, which rendered a doubled space; fixed in the base so the next root-level query inherits it. RecipientsResource.find_by_duns and find_by_uei had byte-identical bodies, differing only in a log label: eight lines each including the parent-level preference loop. Both now delegate to _find_by_keyword, which documents why a parent record wins. NOT done, with numbers. resources/ is 972 lines of which 525 (54%) are docstrings and 213 (22%) is code; the search() and award_id() factory bodies are three statements each. A declarative _search_cls hook cannot remove the docstrings, the imports or the class definitions, and the local imports exist to break cycles, so it would need a string resolver to keep them deferred. That trades roughly a dozen statements for a resolver plus indirection. Likewise the find_all_funding/awarding_agencies_by_name twins: two code lines each under 39-line docstrings, differing by one literal. Consolidating them gains nothing and would need stacklevel=3 to keep DeprecationWarning pointing at the caller. Both are already deprecated. AgencyAwardSummary was already removed from SingleResourceBase in Phase 2, and the client's ten explicit lazy resource properties stay, as planned. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014d1wFpykqretUik9rQpALH
… more entry point Three findings from review of the previous commit, all verified. The rule was positional: it kept whichever level came first. Every observed level list is ['C'] or ['C', 'R'], which is equally consistent with the API just emitting them alphabetically, so "first" carried an unearned assumption. A list arriving as ['R', 'C'] would have picked R and reintroduced exactly the zeroing this commit series fixed. The rule now skips R by membership whenever another level is available, keeps R when it is the only one offered, and deliberately encodes no order among the non-R levels, since nothing observed says whether P or C should win. A regression I introduced: guarding on the level LIST being non-empty rather than on the CHOSEN level meant "abc123-['']" produced a bare trailing dash, where both deleted implementations returned the hash alone. Fixed, and the differential table now shows the consolidated helper matching or beating both predecessors on every input, including two where the model version raised IndexError. SpendingSearch.recipient_id() validated its argument but never normalized it, so it was the one remaining public entry point where a multi-level ID copied out of raw() or off the website produced a filter the API cannot match. Also tightened the parent-level check in _find_by_keyword from a substring test to a suffix test. Safe either way today, since hashes are lowercase hex, but the level is the tail of the ID and saying so costs nothing. Independent live measurement in review also corrected the rationale further: the R record has parent_id, parent_name and parents all null, so the original claim that parentage stayed reachable through Recipient.parent was wrong in the other direction too. Choosing R would have discarded both the spending and the parentage. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014d1wFpykqretUik9rQpALH
Considered adding an optional level argument to find_by_recipient_id and did not,
for two reasons.
The capability already exists. Normalization only fires on the multi-level
"<hash>-['C', 'R']" form, so an ID given with an explicit suffix passes through
untouched: find_by_recipient_id("<hash>-R") already fetches that record. It was
merely undocumented, and now is not.
A level argument would also collide with the suffix the ID already carries.
find_by_recipient_id("<hash>-R", level="C") has no unsurprising answer: either it
overrides the ID the caller typed or it ignores the argument they passed. It
would additionally be a no-op on the majority of calls, since most IDs arrive
already suffixed, and would have to spread to SpendingSearch.recipient_id and
Recipient.__init__ to stay consistent.
The real gap was that the API reports recipient_level on every recipient payload
and the library dropped it, so after normalization a caller could not tell which
of several records they held without parsing the ID string. It is now a property.
Live data exercises both branches: the golden masters gained two records at "C"
and one at "R", as pure additions with no existing value moved.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014d1wFpykqretUik9rQpALH
…y name smart_sentence_case was a one-line delegation to TextFormatter.to_sentence_case with three call sites. Those now call the classmethod directly and the wrapper is gone, so there is one entry point for sentence casing rather than two names for it. contracts_titlecase is renamed titlecase_name across 13 files. The old name said "contracts", which is not what it does: it cases recipient, agency and place names. Its docstring was worse, describing "NASA-relevant" and "NASA-specific rules" for a library whose stated first principle is to be agency-agnostic. The rewritten docstring says what the function is for and why the special cases live in data. A code comment illustrating possessives with NASA is generalized too. The special_cases.yaml entries themselves stay as they are: that file is casing reference data, and the acronyms in it are the forms to leave alone, not agency logic. No compatibility alias for either rename. Neither name is exported from usaspending/__init__.py, neither appears in the README, and every importer is in-repo and updated in this commit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014d1wFpykqretUik9rQpALH
…casing _preserve_special_case scanned the whole 140-entry special-case list for every word, doing up to three .lower() comparisons per entry, so a miss cost ~420 string lowerings. It runs once per word of every recipient, agency and place name the library cases. Measured: the miss path, which is the common one, goes 19,877 ns -> 413 ns, 48x. Casing "THE BOEING COMPANY" goes ~68 us -> 9.4 us, and a 100-row page of recipients reading .name once each goes ~9.3 ms -> 0.94 ms. Behavior preserving, and checked rather than asserted: a differential run over 4,123 distinct words -- every special case in eight forms plus every token in tests/fixtures -- found zero divergences from the scan. Entries are keyed both as written and, for a form ending in a period, without it, so "inc" still finds "Inc.", and earlier entries still win. Building the index introduced a second piece of cached state, and the first run caught it: twelve places in the suite invalidate casing by setting _special_cases_cache = None, and the index went stale behind them, breaking three tests. Rather than make every caller remember a thirteenth thing, the index stores the list object it was built from and rebuilds when that changes, so clearing the cache remains the single way to invalidate casing. A regression test covers it, and reverting the guard fails it. Left Recipient.name a plain property rather than caching it. At 9.4 us a read it is no longer worth per-instance state. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014d1wFpykqretUik9rQpALH
_load_special_cases caught bare Exception and returned an empty list, so a corrupt or wrongly shaped special_cases.yaml silently disabled special casing. That failure is invisible in practice: names are still returned, just mis-cased, so the logged error gets scrolled past. A parse failure or a non-list now raises ConfigurationError. A missing file still degrades quietly, since an installation can legitimately lack it. current_fiscal_year called datetime.now() twice and compared the month from one reading against the year from the other. Harmless almost always, wrong if the two calls straddle the October boundary, which would report a fiscal year the calendar never had. One reading now, verified either side of the boundary. Removed the module-level get_logger forwarder. I claimed earlier it had zero importers; that was wrong, and the suite caught it with an ImportError. tests/test_logging_config.py imported it through a multi-line parenthesized import my grep pattern missed, and had a test class dedicated to it. It is still right to remove: nothing under src/ uses it, it is absent from __all__, the README and the API-surface snapshot, and its own docstring claimed it was "commonly used throughout the library codebase", which was false. Its only consumer was its own tests, which go with it. Declined the plan's exception-constructor collapse, with numbers. exceptions.py is 295 lines of which 237 (80%) are docstrings and 37 are code. The four __init__ bodies total ten statements: four instances of the idiomatic super().__init__(message) and six assignments of DIFFERENT attributes. A **attrs base would trade explicit typed assignments for a setattr loop, saving about two net lines while defeating static analysis of the attributes. Also verified rather than assumed the packaging item: special_cases.yaml already ships in the wheel, since hatchling includes package data under `packages`, and Phase 0's guard derives its path from the module location rather than the repo, so it would fail in an installed context if the file went missing. An explicit force-include entry would be config that looks load-bearing but is not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014d1wFpykqretUik9rQpALH
Correcting the previous commit. It raised ConfigurationError on a corrupt or wrongly shaped special_cases.yaml, which I verified breaks __repr__: casing runs through titlecase_name inside RecipientSpending.__repr__ and SubAward.__repr__, so both raised. A raising __repr__ is the worst place to surface a fault, since it takes out debuggers, logging and error messages, and it did so over a purely cosmetic problem. A mis-cased name is still the right name. It now degrades to no special casing and emits a UserWarning. That keeps the gain the previous commit was after, since the old behavior logged and was invisible in practice, while leaving the failure non-fatal. A warning is visible by default and anyone who wants it fatal can escalate it with -W error::UserWarning. The library already uses UserWarning for exactly this "proceeding, but something is wrong" case in client.downloads.search(). A missing file still only logs, since an installation can legitimately lack it. Also verified the index optimization is intact rather than assuming it: across 200 titlecase_name calls the index is built exactly once, because _load_special_cases returns a stable list object, so the identity check holds. Tests now pin that repr survives a corrupt file, which is the property that was broken. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014d1wFpykqretUik9rQpALH
`reattach(recursive=True)` walked the instance for nested models to rebind, but tested LazyRecord while `reattach` is defined on ClientAwareModel. Holding a client is what makes a model need rebinding, and five classes hold one without loading lazily: AwardAccount, FederalAccount, Funding, SubAward and TreasuryAccountSymbol. They were stepped over silently, kept a weak reference to the outgoing client, and raised DetachedInstanceError once it was collected, including for a model the caller was still holding, such as a TreasuryAccountSymbol taken from account.tas_codes. The walk was doing almost nothing for a real graph: on a 16-account agency with its TAS codes, 273 models, it visited 1. Three things followed. The predicate now tests ClientAwareModel, which is declared in this module, so the deferred LazyRecord import that existed only for that isinstance check is gone. The walker moved out of `reattach` into a module-level function: it captures nothing, and hoisting it also makes it testable. That saves one function object per model, which is 4% of a call happening once per session handoff, so the reason to do it is that it is a plain function, not the speed. The cache discard added earlier is now conditional. A recursive reattach rebinds the models inside a cached level in place, so the level survives: re-reading a fully populated agency after a reattach costs 0 requests where discarding cost 17. A non-recursive one cannot rebind them and still discards. The docstrings no longer claim that is invisible to the caller, because it is only invisible for the cache object: a model already taken out of one stays on the old client either way, so recursive=True repairs the graph while the default just keeps this model working. The README taught both calls as though both were needed; it now shows the recursive one and says what the default leaves behind. The walk also skipped nothing, including `_data`, which is the raw API payload and by construction holds no model, so it descended entire responses node by node. Skipping it halves the agency walk, 468 to 232 us, and takes the README's award example from 33 us to 2. A test pins the boundary that makes it safe, and is explicit that a model placed in a payload would not be rebound. Not taken, recorded as a task: a descriptor that would let a level cache notice a client change at read time, retiring `_REATTACH_INVALIDATES` and the conditional entirely. It works, but it swaps a stdlib decorator for a project-local one that a reader must learn, for two call sites. Two other alternatives were rejected on inspection: discarding every cached_property containing a model over-reaches onto the nested models the flag exists to leave alone, and an explicit children hook would need eight names across five classes, reintroducing exactly the silent-omission failure this commit fixes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014d1wFpykqretUik9rQpALH
`to_date` converts every date the library returns, and did it by working down a list of seven strptime formats until one matched. The first entry is the plain YYYY-MM-DD that accounts for 92 of the 94 date values in the captured API responses, so nearly every call paid a format walk to land on the first format. Reading that shape through `date.fromisoformat` instead takes 0.18 us rather than 2.06 us, an 11.4x cut measured by A/B against the committed function. Where it shows up is per row. Reading `action_date` across a 5000-transaction page falls from 11.0 ms to 1.7 ms. That also makes a measurement in `TransactionsSearch._row_passes` stale, since its guard now saves 2.2 ms rather than the 21 ms its docstring claimed, so this corrects it. The fast path is pinned to the exact YYYY-MM-DD shape rather than handed the string outright. Python 3.11 widened `date.fromisoformat` to accept the basic and week-date forms, `20250829` and `2025-W35-5`, which the 3.9 floor refuses and USAspending never sends; accepting them on a new interpreter and not an old one would make behavior depend on the interpreter. The format list stays the authority on what parses, and the fast path can only short-circuit a success: a failure falls through, so an impossible date still warns by the same path as every other failure. Behavior is unchanged, verified on Python 3.9.6, 3.10.19, 3.11.15 and 3.12.13, plus a brute force over guard-passing strings that found no input the fast path accepts and `%Y-%m-%d` rejects. 17 tests added, written first and confirmed green against the old implementation so they characterize existing behavior rather than the new code. Two of them patch the format list empty to prove the fast path is what fires, which nothing else can observe: the two paths return equal dates, and `date` is an immutable C type that cannot be spied on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014d1wFpykqretUik9rQpALH
`parse_date_string` is annotated `-> date` but tested only `isinstance(value, date)`. datetime subclasses date, so a datetime returned unnarrowed and the first comparison against a real date raised `TypeError: can't compare datetime.datetime to datetime.date`. Two public paths reached it, and they fail differently. `time_period` publishes `datetime.date | str`, so passing a datetime is a declared promise; it crashed inside the FY2008 floor check at query_builder.py:673. `since()` and `until()` crashed at iteration instead, because the bound is only read in the row predicate, which puts the traceback a long way from the mistake. Their hints also said `str` while the runtime had always taken a date, so they now say `str | date` and the tests stop asserting an undocumented accident. The wire payload was never wrong: `TimePeriodFilter.to_dict` serializes with `strftime`, which formats a leaked datetime correctly. The range validation was the only thing in the path that could detect it, which is the argument for narrowing at the parse function rather than trusting downstream to notice. This is the second time this defect has been fixed here. `to_date` had it and was fixed in 50e60de with the same test name, and the fix never reached the sibling. Both sites now cross-reference each other, since the failure mode was not that the two lines were duplicated but that the author of one did not know the other existed. Extracting a shared helper was considered and rejected: each caller would still need its own isinstance branch, so it removes no line that could be got wrong, and it protects nobody who does not already know to reach for it. A type checker was also considered and verified useless here: mypy --strict passes the pre-fix code, because typeshed declares datetime a subclass of date, so `-> date` returning a datetime is legal and isinstance narrows to exactly the wrong static type. Written test-first: all new tests confirmed failing against the old code for the right reason. Three mutants, narrowing removed, order swapped so the date check runs first, and narrowing that returns the datetime, each fail 6 tests including the doctest. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014d1wFpykqretUik9rQpALH
The package has two date parsers with deliberately opposite policies, and
neither honored its own documented contract for a value that is not a date at
all.
`parse_date_string` reads what a caller supplied and documents
`Raises: ValidationError`, but a wrong-typed value escaped as
`TypeError: strptime() argument 1 must be str`. `None` is the one that matters,
since threading an Optional through is the ordinary way to arrive there, and
`time_period(None, "2024-01-01")` produced a traceback from inside strptime
naming nothing useful. `to_date` reads API payloads and, per the convention
`utils/numbers.py` states for the coercer family, answers a missing or
unparseable value with None, but it too could let a TypeError out, making it the
only coercer in the package that could raise on bad input.
Each now does what it says, and each docstring names the other, so the split is
discoverable from either side rather than being a thing you notice twice.
Two cleanups to the error text, which is public API surface. It quoted
`%Y-%m-%d` where the docstring promises `YYYY-MM-DD`, leaking a strftime
pattern the caller never supplied, and the value is now shown with repr so an
empty or whitespace-only value is visible in the message. `format_str` is
deleted: no caller ever passed one, so its only effect was to put that pattern
in the message.
Tests first, 11 new, confirmed failing against the old code with the raw
TypeError. Two of my own cases were wrong and taught something worth keeping:
`[]` and `{}` are falsy, so they take the missing-value guard and answer None
without a warning, which is correct and now documented in the test.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014d1wFpykqretUik9rQpALH
`time_period()` rejects a bound before FY2008 and rejects an inverted range. `since()`/`until()` checked only the string format, so both mistakes were accepted and then failed silently: a pre-FY2008 bound matched every row the API returned, and an inverted range yielded nothing, which is indistinguishable from an award with no transactions in range. Both now raise, which is the one behavior change here and is noted in the CHANGELOG. The two rules now live in `parse_api_date` and `validate_date_range` in filters.py, next to the `MIN_API_DATE` they enforce and reachable by both callers without inverting the utils/queries dependency that `utils/validations.py` documents. `time_period()` loses 17 lines of hand-rolled checks and its error messages are unchanged, which the pre-existing tests confirm. The ordering check has to work in either chaining order, since each filter clones and only the second call can see both bounds. `validate_date_range` therefore takes Optionals and does nothing until both are set, and both `since()` and `until()` call it, so `.until(x).since(y)` is validated like `.since(y).until(x)`. There is a test for each direction, and for the two boundaries the rules must not eat: equal bounds select one day, and 2007-10-01 itself is valid. Tests first, 8 new; 5 failed against the old code and 3 passed, those 3 being the boundary cases that already held. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014d1wFpykqretUik9rQpALH
CPython caches compiled strptime patterns in one process-global dict and clears the whole thing once it holds more than five. The format list held seven, so a value that matched none of them walked past that point and evicted every entry in the process, charging the next `strptime` call anywhere in the application a recompile. One unparseable API field made unrelated code slower. Two changes, neither of which alters what parses. `%z` already reads a `Z` as well as a numeric offset, so the separate `%Y-%m-%dT%H:%M:%SZ` format was doing nothing; removing it leaves six. And a value is now walked against the list only if it could match one: every format begins `%Y-%m-`, and `%Y` takes exactly four digits, so anything the list can read starts with four digits and a dash. Verified by brute force over 161,601 strings, mutations of every documented shape plus random ones, that nothing the formats accept fails that test. The guard sits after the date-only fast path rather than before it. Ahead of it, the string slice it needs cost the dominant shape 33% (0.21 to 0.32 us) to speed up a path no observed API value takes. Behind it, the hot path is unchanged at 0.20 us, an unparseable value costs 0.25 us rather than 29 us, and the offset shapes gain about 15% from the shorter list. The gain here is defensive: every date field across all 50 fixtures is parseable, so nothing in the recorded data takes this path. What is not defensive is the eviction, which reaches code this library does not own, and that is what the new test pins: an unrelated caller's cached pattern now survives a rejected value. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014d1wFpykqretUik9rQpALH
`tests/utils/test_dates.py` had accumulated one test per accepted shape, each
asserting `.year`, `.month` and `.day` separately, so twelve tests and about
ninety lines said what one parametrized table says. The two nearest neighbours,
`test_numbers.py` and `test_validations.py`, already use the shape this adopts:
one `("value", "expected")` accept table and one reject table, with a comment on
the rows that are interesting rather than a test name for each.
The reasoning that lived in those test names and docstrings survives as row
comments, which is where the neighbours put the same kind of note: that
`%Y-%m-%d` takes an unpadded date where `fromisoformat` does not, that only `%z`
reads an offset without a colon, and that `%z` is also what reads a `Z`.
Two tests were deleted outright rather than folded in. `test_backwards_
compatibility` parsed `2025-01-15` and `2025-06-15` and asserted they are those
dates, which the basic case already covers, and its name described a migration
rather than a behavior. `test_real_usaspending_formats` asserted the same shape
as `test_iso_datetime_format` with a different day.
Tests making a claim of their own stay separate: the two fast-path seam tests,
the leading-date-is-not-enough table, the version-portability table, the
impossible-date warning, the shared-cache test, and the date and datetime
passthroughs. Wrong-typed values now live only in the test that also asserts the
warning, so each value has one home.
47 insertions against 146 deletions, and 43 test items rather than 38.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014d1wFpykqretUik9rQpALH
`utils/validations.py::validate_sort_field` had no callers while four query builders hand-rolled the same check with four different messages, and the direction check was hand-rolled four more times with four more. Same for `agency_type`, validated at six sites in four wordings. A dead helper beside several inline copies is the worst arrangement of the two options, since the helper looks like the answer and is not. Consolidating means choosing the wording, so each canonical form is the one already in the majority: a quoted field and "Valid fields are:" (each 3 of 5 sites, matching `AwardsSearch` exactly), the transactions builder's direction message, and the "Invalid agency_type:" prefix that three tests already assert. Only two test assertions changed as a result, both in `test_sub_agency_query.py`, which had the two messages no other site shared. `validate_sort_field`'s `context` argument is now optional. It defaulted to "query", so every message it produced said "for query", which is why the sites with one fixed set of fields could not have used it without padding their errors with a restatement of the method just called. `AwardsSearch` is the case it was built for, where the same field is valid for contracts and not for loans. Two related gaps closed while here. `AwardsSearch.order_by` never validated its direction, so it now does, by the same helper as its siblings. And `AgenciesSearch._endpoint` was an if/elif over two agency types with a third branch raising; the two endpoints differ only by that word, so it validates and interpolates, keeping the guard for a future setter that skips validation. `SortableQuery` keeps its own field check, with a comment saying why: it accepts either a friendly name or the API name it maps to, so the set it tests and the set worth showing a caller are different, and the shared helper shows what it tests. Inverting each of the three helpers' conditions fails 10, 46 and 95 tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014d1wFpykqretUik9rQpALH
The snapshot walked `usaspending.__all__`, which tracked 47 classes and not one resource. Resources are the most user-facing classes in the library, since every entry point runs through one, but they are deliberately not exported: they are reached as client properties, with TYPE_CHECKING stubs for autocomplete. So the one collection strategy the guard had was the one that could not see them, and their public members could be added, renamed or removed with nothing failing. This branch had already done it twice, unguarded: `TASResource` gained `agencies_query`, and `RecipientsResource` gained a private `_find_by_keyword` while its two public finders kept their signatures. Both subpackages that export names of their own are now walked as well, which adds 9 resources and the 13 query classes that `usaspending.__all__` does not re-export, taking the tracked total from 47 to 69. Their `__all__` lists are snapshotted too, in a separate test, since an export can disappear without any class changing. Verified by reintroducing the drift that got through before: adding a public method to `TASResource` now fails with it named in the diff, and deleting `TASResource` from the resources `__all__` fails two of the four checks. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014d1wFpykqretUik9rQpALH
A reviewer built and verified the alternative: a `level_cache` descriptor that checks at read time whether its models still point at the owner's current client, which deletes this list from three files and the entire non-recursive branch of `reattach`, leaving `recursive` to govern one thing again. It measured identical in behavior and cost about 0.7 us on a read of a cached 16-model level. Not taken, for the reason CLAUDE.md names: it swaps a stdlib decorator for a project-local descriptor a future reader has to learn, for two call sites. But `filter_tree_query.py` documents level caching as a pattern, so a third is expected, and at three this list is a registry and the descriptor is right. The trigger and the tradeoff now sit on the attribute itself, since that is what someone adding the third cache will be reading. No behavior change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014d1wFpykqretUik9rQpALH
An award search result reports its recipient and its dates as flat, title-cased keys, where a detail response sends nested objects. `Award` translated those keys into the nested spellings before constructing each model, which meant the mapping lived in the caller rather than in the models that own the fields, and the two had already drifted. The drift was user-visible. `Recipient.uei` and `Recipient.location` read only the nested spellings, so `Recipient(row, client)` for a search row answered None for both, while `award.recipient.uei` for the same row worked, because `Award` had repaired it on the way past. `PeriodOfPerformance` was missing `Base Obligation Date` for the same reason. Each model now reads every spelling of its own fields, which is what makes handing the payload over possible. Each model also declares the flat keys it owns and copies just those. Copying rather than aliasing is load-bearing: `Award` replaces its payload in place when a detail fetch fires, and both models read some fields lazily, so a shared reference would let that fetch change an object already built. Scoping the projection also keeps `raw` describing a recipient rather than an award. Deleted along the way: the post-construction `recipient.location = Location(...)` attach, which worked only because `cached_property` writes through `__dict__`, and a `recipient_hash` lookup that no result row in any of the 18 paginated fixtures carries. Behavior is otherwise unchanged, which the golden-master suite confirms against live data. One nuance: the branch that builds from flat keys now tests every key either model owns rather than three of them, so a payload carrying only, say, a potential end date builds locally where it used to trigger a fetch. No row in any fixture takes that path, and building from data in hand is the better answer where it does. Four mutants, one per pushed-down key plus one that stops the projection filtering, each fail the suite. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014d1wFpykqretUik9rQpALH
Four reviewers went over `5968ab9..HEAD`. Three findings were defects, the rest
were the batch overstating itself.
A bad sort direction reached three different outcomes. `TransactionsSearch`
raised, `RecipientsSearch` sent the typo to the API, and a client-side query
stored it and computed `reverse = direction == "desc"`, which is False, so
`order_by("name", "descending")` silently sorted ascending. The previous commit
routed four builders onto a shared check and missed the base method they all
inherit and the one builder that validated nothing at all, because it surveyed
duplicated code rather than entry points. The check now lives in
`BaseQuery.order_by`, which covers every builder present and future.
`Award.start_date` and `end_date` still translated the flat date keys that the
same batch had just moved into `PeriodOfPerformance`, so the claim that the two
could no longer disagree was false on arrival: the end-date chain read a
spelling nothing else in the package uses, and the two orderings of
`Base Obligation Date` were opposites. Both now delegate. Separately, each
model's `_SEARCH_KEYS` was missing a key its own properties read, which for
`PeriodOfPerformance` meant a payload fetching over the network for an answer it
already held, and for `Recipient` meant dropping the hash a record with no
`recipient_id` needs to lazy-load by. A test now pins the tuple against the
properties in both.
The surface snapshot gained the subpackage mechanism and then left `models` out
of it, so the 19 exports that are not re-exported at the top level stayed
unguarded, `BaseModel`, `ClientAwareModel` and `LazyRecord` among them. Now 75
classes rather than 69.
Two corrections to my own words. `_DATE_FORMATS` carried a comment saying to keep
the list short, implying six was short enough; it is not, and a date-shaped value
that matches nothing still walks all six and evicts the cache. The comment now
says so and a test pins it, since the alternative narrows what parses. And
`_REATTACH_INVALIDATES` was described three times as a list, is a tuple, and was
credited with two entries while being empty.
Smaller: `validate_sort_field` took `Iterable`, which it reads twice, so a
generator would have reported no valid fields at all; it also now returns its
argument like the other seven validators in that module. Deleted a
`_validate_agency_type` that had become a pass-through. Cut roughly 40 lines of
comment and docstring that restated each other or argued to a reviewer.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014d1wFpykqretUik9rQpALH
The efficiency pass on the batch measured what 6c1cff5 claimed. The date-only hot path is unregressed and an unparseable value is 99% cheaper, both as stated, but "the offset shapes gain about 15%" is true only of numeric offsets. A `Z` is now read by the larger `%z` pattern rather than its own literal format, and since six formats still overflow the regex cache that pattern recompiles on every call, so that shape is about 4% slower. Measured 27.96 to 29.15 us against 33.95 to 29.88 us for `+0500`. Not worth reverting: no recorded response carries a `Z` or a numeric offset at all, and restoring the literal format would take the list to seven and make the overflow worse for everything else. Worth writing down, next to the tradeoff it belongs to, so the next reader does not have to re-measure to learn that the shorter list was not free. Also on the record from the same pass, since both are load-bearing for decisions already taken: the guard placement in that commit costs 67% rather than the 33% I reported, in the direction that argues harder for the shipping order; and the projection added by d2a286f is 2.5x faster than the `items()` alternative, because a search row has 26 keys and the projection wants 5 to 7. A whole page of 100 rows got 4% cheaper, since building a recipient no longer eagerly builds its location. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014d1wFpykqretUik9rQpALH
Filters return their internal lists from to_dict() by reference, and
_clone() shares filter objects between a query and its clones. When two
filters serialized under one payload key, _aggregate_filters() stored the
first filter's own list and extend()ed it in place, so every payload build
grew the body and the parent query silently acquired the child's values:
q1 = search().keywords("alpha"); q2 = q1.keywords("beta") left q1 sending
["alpha", "beta", "beta"] after two builds of q2.
Copy list values once on read and merge into lists the aggregator owns.
The four filters that alias internals (keywords, award_type_codes, simple
psc_codes, treasury_account_components) are pinned by parametrized
idempotence and parent/sibling isolation tests, and BaseFilter.to_dict now
states the aliasing contract where filter authors read it. Pre-existing at
0.7.3; found during independent verification.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ClientSideQueryBuilder.__getitem__ applied filters and ordering but not limits, unlike iteration, so tas.agencies.limit(3)[:] answered all 92 agencies where len(), .all() and a loop answered 3, and limit(3)[5] returned a row instead of raising IndexError. Indexing now resolves through all(), the same idiom TransactionsSearch.__getitem__ already uses, and rejects bad key types before doing any work. Pre-existing at 0.7.3 for SubAgencyQuery and friends; this branch made it visible by turning tas.agencies into a query. Tests pin the limit and max_pages caps on both the generic builder and a fetched filter-tree level; every one fails against the old __getitem__. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Which answer count() gave used to depend on how the endpoint reports a total. Builders backed by a count endpoint or page metadata returned the server figure whatever the caller asked for, so limit(3) on an awards search counted 510 while yielding three rows, and the docstrings claiming each behavior contradicted each other in one file. Builders that count by walking pages already stopped at the bound. count() now applies limit() and max_pages() everywhere, so count(), len(query) and len(query.all()) always agree; the server total is what count() says before the query is bounded. The capping lives in one place, BaseQuery._cap(), shared with the in-memory _apply_limits so yielding and reporting cannot drift, and a _yields_nothing() guard answers limit(0) and max_pages(0) without spending a request, where max_pages(0) previously fetched a whole filter-tree level to report nothing. A parametrized matrix drives the same nine cases through all four count mechanisms; the max_pages(0) rows fail against the limit-only guard, and the capping rows fail against the old uncapped count(). The negative index path in TransactionsSearch now reads the memoized count it always agreed with, saving a full re-pagination per repeated q[-1]. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Agency and Recipient swallowed every fetch error and returned None, and the loaded flag latched regardless, so one server error, rate limit, dropped connection or closed client permanently converted a model into a source of Nones with no way to retry. reattach() could not recover such a model: it never issued another request. The fetchers now swallow only the API's definitive no-such-record answers, 400, 404 and 422, which still return None and latch so a missing record costs one request rather than one per read. Everything else propagates and leaves the model unlatched, so the next property access retries. The list names answers rather than failures on purpose: an unrecognized failure must surface, not silently poison the model. The mock client learned to raise RateLimitError for a 429 the way the real retry handler does, and the migration note steers callers to one guarded fetch_all_details() rather than per-read guards, which would pay the whole retry ladder on every read. Pre-existing at 0.7.3; found during independent verification. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
parse_date_string is exported from usaspending.utils, so removing its format_str parameter broke any 0.7.3 caller that passed one. The parameter is back as deprecated-but-working: the default call takes the fast path unchanged and warns about nothing, while a non-default pattern parses with strptime, keeps ValidationError semantics, and emits a DeprecationWarning naming the caller's line. Scheduled for removal. current_fiscal_year was deleted with no replacement anywhere in the package. It returns with the same October 1 convention, now properly public in usaspending.utils rather than reachable only through the removed utils.formatter deep path, which stays gone. Everything else the refactor removed stays removed and documented; these two were the cases with a released contract behind them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Independent verification found five tests that could not fail: two request-count tests deriving their expected figure from the same parsed result they validate, two zero-iteration loops that pass when the query yields nothing, one assert that a non-negative counter is non-negative, and a loan assertion that branches on the property under test. Each now pins against the fixture on disk, with a non-empty guard so an emptied fixture fails loudly rather than passing quietly. TransactionsSearch.order_by was wholly unpinned: deleting its direction validation passed all 1915 tests. It now has rejection tests for bad directions and fields, a literal pin of the seven documented sort fields (so a dropped field fails rather than silently shrinking the parametrization), and a payload round-trip. Award.download()'s dispatch and the recipient parent-preference branch, previously covered by nothing, get unit tests mocked at the HTTP seam with the endpoint literals as an independent oracle. Every fix was proven by reintroducing the defect it guards against and watching the suite fail. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ware The snapshot recorded names only, via dir(), on Python 3.10. That made it false-fail on 3.11+ (exceptions gain add_note) and blind to the one genuine signature break this branch made: parse_date_string losing a parameter. It also excluded utils and download on a rationale their own __all__ contradicts - parse_date_string is deprecating a parameter this cycle and the README documents client.downloads returning a DownloadJob. Members are now kept only when their defining class is the library's own, constructors are recorded when this package wrote them (the builtins rule is not enough there: Enum defines __init__ on 3.11/3.12 only), and every public callable records an annotation-free signature with literal defaults spelled out, so order_by's 'desc' and a dataclass gaining a required field are guarded values. Verified byte-identical snapshots on 3.9, 3.10, 3.11, 3.12 and 3.14, and a compat test pins the deep-import paths the snapshot cannot see. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The golden master compared fields named in VOLATILE_PROPERTIES by type and nullness only, so a wrong-but-well-typed end date or a one-cent shift in every Award money field passed. And its search-built award never exercised the flat search-key path at all: the first property read in sorted(dir()) order triggers a detail fetch that merges the detail response over the row, so the projection classmethods ran zero times during snapshotting. Volatile fields now compare exactly against the raw value in the same response payload, both live and, through shared tables in tests/volatile_fields.py, offline over the recorded fixtures, so a mapping regression fails the unit suite rather than a release run. The flat projection is pinned by offline tests running under a client that refuses requests, including the documented precedence flips and the dropped end-date alias. The mock client grew forbid_requests() for exactly that idiom, and the module docstring records the legitimate search-versus-detail differences so nobody mistakes upstream data for library defects. Live request count is unchanged: 51 before, 51 after, for 14 more tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Three independent reviews agreed the refactor's substance was sound and its paper trail was not. The release section now declares everything the cycle actually changed: sort-direction validation on every builder, the start and end date delegation with all three precedence moves, the paging counts outgrowing config.default_result_limit, the to_int leniency, the filter-aggregation and in-memory indexing fixes, and the removals the Removed section had skipped. The entry that claimed bool(query) still triggers a count, contradicting the entry above it, now tells the truth, and the recipient entry names a method that exists. Release mechanics: version 0.8.0, dated section, lockfile regenerated, pytest-cov in the dev group so the coverage gate is reproducible, and an sdist allowlist plus .gitignore entries for .claude/ and the stray ~/ cache directory, without which a build from this working tree shipped a 1.9 MB sdist carrying another agent's worktree. SubAward.amount's annotation now matches the Decimal it has always returned, and the README's retry-delay default matches config. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
CPython 3.9's list() constructor consults __len__ twice for its size hint where 3.10 and later consult it once, and __len__ on a query costs a count request. Two example tests asserted the 3.10 total, so the suite failed on the 3.9 floor, and the mock README taught the same assertion in a form wrong on every version. Nobody had ever run the suite on 3.9; the new multi-interpreter gate caught it. The tests and the README now read queries with all(), which asks for no hint and costs the same one request everywhere, and the caveat lives on get_request_count's docstring where the next person asserting a total will read it. Suite verified green on 3.9 through 3.13. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Every guarantee this cycle established was verified once, by hand: the suite green on five interpreters, ruff clean, the coverage floor, the interpreter-independent surface snapshot, the live golden master. With no CI they start decaying immediately, which the 3.9 failure nobody had ever seen made concrete. test.yml runs the unit suite on Python 3.9 through 3.14 (the surface snapshot's own docstring reasons about 3.14, so the claim is now tested), plus ruff, the 80 percent coverage floor, and a built-wheel smoke test that asserts behavior rather than importability, since a package-data file dropped from the hand-written sdist allowlist degrades title casing silently. integration.yml runs the ~50-request live suite weekly and on demand rather than gating pushes. setup-uv is pinned per-interpreter so the matrix legs do not share one cache key, and classifiers now claim the 3.13 and 3.14 the matrix tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The 0.8.0 section buried its two most user-facing entries as bare paragraphs mid-file, opened with "internal simplification pass", and ran 527 lines of entry-order-by-accident. It now opens with a breaking changes summary ordered by how likely each is to reach caller code, every entry is a proper bullet ordered by user impact within its section, and the prose is a quarter shorter with every migration fact kept: workarounds, affected properties, the bounds under which the count promises hold, and which constructs never paid the length-hint request. Review corrections folded in: SingleResourceBase was never removed, only AgencyAwardSummary's inheritance of it; "always agree" now says under whose bounds, since config.default_result_limit still caps iteration but not counts; and the measured request figures name the date-bound condition they were measured under. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This comment was marked as off-topic.
This comment was marked as off-topic.
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
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.
The refactor branch (54 commits), plus 11 commits from an independent adversarial verification and its follow-ups:
limit(), thecount()endpoint/paging fork, lazy-load errors latching silentNones, five tests that could not fail, a suite that had never run on Python 3.9.count() == len() == len(all())on every builder; sort direction validated everywhere; targeted 0.7.3 compat restore (parse_date_string(format_str=)deprecated-but-working,current_fiscal_yearrelocated).Verification evidence lives in VERIFICATION_REPORT.md (untracked, local).
🤖 Generated with Claude Code