Document architecture baseline and add guardrails (DOI-USGS#349)

* chore(architecture): document baseline and add guardrails

* docs(architecture): remove duplicated policy

Author: thodson-usgs

CONTRIBUTING.md

@@ -22,6 +22,7 @@ however writing code is not the only way to contribute.
       - [Style](#style)
       - [Docstrings](#docstrings)
       - [Quotes](#quotes)
+    - [Updating Package Version](#updating-package-version)
   - [Documentation](#documentation)
     - [Contributing to the Documentation](#contributing-to-the-documentation)
     - [Adding Examples to the Documentation](#adding-examples-to-the-documentation)
@@ -79,30 +80,51 @@ Please do not combine multiple feature enhancements into a single pull request.
 Before you submit a pull request, check that it meets these guidelines:
 
 1. If the pull request adds or modifies package functionality, unit tests
-   should be written to test the new functionality
-2. If the pull request adds or modifies functionality, the documentation should
-   be updated. To do so, either add or modify a functions docstring which will
-   automatically become part of the API documentation
-3. The pull request should work for Python 3.9 and later - refer to the
-   [python-package.yml file](https://github.com/DOI-USGS/dataretrieval-python/blob/main/.github/workflows/python-package.yml)
-   for the latest versions of Python being tested by the continuous integration
-   pipelines. This will be checked automatically by the CI pipelines once the
-   pull request is opened.
+   should be written to test the new functionality.
+2. If the pull request adds or modifies functionality, update the documentation
+   or function docstrings that describe it.
+3. The pull request should work for Python 3.10 and later. Refer to the
+   [Python package workflow](https://github.com/DOI-USGS/dataretrieval-python/blob/main/.github/workflows/python-package.yml)
+   for the versions and operating systems currently tested by CI.
+4. Build-related changes should preserve the installed-wheel smoke test; tests
+   run from a source checkout are not sufficient to prove package contents.
+5. Architecturally significant changes should update the
+   [architecture documentation](docs/source/architecture/index.rst), add or
+   supersede an ADR, and adjust the corresponding fitness function.
 
 ### Coding Standards and Style
 
-Note that coding standards and style as described below are strong suggestions,
-the `dataretrieval` project does not strictly lint or enforce style guidelines
-via any automated processes or pipelines.
+The continuous integration and pre-commit configurations enforce formatting,
+linting, and strict type checking. Run the relevant checks before opening a PR:
+
+```bash
+ruff check .
+ruff format --check .
+mypy
+coverage run -m pytest tests/
+coverage report -m
+```
+
+For documentation changes, install `.[doc,nldi]` and run `make html` from
+`docs/`. The broader `make docs` target also runs doctests and network-dependent
+link checking.
 
 #### Style
 
-* Attempt to write code following the [PEP8 style guidelines](https://peps.python.org/pep-0008/) as much as possible
-* The public interface should emphasize functions over classes; however, classes
-  can and should be used internally and in tests
-* Functions for downloading data from a specific web portal must be grouped
-  within their own submodule
-  * For example, all NWIS functions are located at `dataretrieval.nwis`
+* Follow the [PEP8 style guidelines](https://peps.python.org/pep-0008/).
+* The public interface should emphasize functions over classes; classes can and
+  should be used internally and in tests.
+* Group public download functions by data portal. For example, modern Water
+  Data functions belong in `dataretrieval.waterdata`; legacy NWIS functions
+  remain quarantined in `dataretrieval.nwis` during deprecation.
+* Preserve the dependency direction documented in
+  [`docs/source/architecture`](docs/source/architecture/index.rst): public
+  facades depend on service/protocol adapters, which depend on stable shared
+  policy and infrastructure. Shared OGC code must not import service adapters,
+  and modern modules must not depend on legacy NWIS.
+* Treat underscore-prefixed helpers as implementation details. Existing
+  cross-package uses are documented variances, not extension points for new
+  code.
 
 #### Docstrings
 * Docstrings should follow the [numpy standard](https://numpydoc.readthedocs.io/en/v1.5.0/format.html):
@@ -165,6 +187,14 @@ via any automated processes or pipelines.
         return re.search(r"(?i)(arr|avast|yohoho)!", message) is not None
     ```
 
+### Updating Package Version
+
+The package version is derived automatically from Git tags by
+`setuptools_scm` (see `[tool.setuptools_scm]` in `pyproject.toml`), so there is
+no version string to edit by hand. To cut a release, tag the commit (for
+example, `git tag v1.2.3`) and push the tag; both the installed package version
+and the documentation's `version` and `release` values follow from it.
+
 ---
 
 ## Documentation

NEWS.md

@@ -1,3 +1,5 @@
+**08/02/2026:** Fixed source-distribution and wheel package discovery so the `dataretrieval.ogc` and `dataretrieval.waterdata` subpackages are included in installed artifacts. CI now builds and installs the wheel outside the source checkout before importing the core service modules. Added an architecture baseline, initial decision records, and executable dependency-direction guardrails for the existing modular-monolith boundaries.
+
 **06/23/2026:** **Breaking change (1.2.0):** the minimum supported Python is now **3.10** (`requires-python = ">=3.10"`). 3.9 support was already effectively broken — the `waterdata` module's dependencies (`anyio`, the test stack) require 3.10+, and the `waterdata` test modules already skipped on <3.10. `anyio` is now declared as a direct dependency (it is imported directly by `waterdata`), and the CI/ruff/mypy targets move to 3.10. Also fully removed the deprecated `variable_info` metadata property: the `NWIS_Metadata` override only warned and returned `None` (it relied on the defunct `get_pmcodes`), and the `BaseMetadata` abstract is gone too since nothing implemented it — accessing `.variable_info` now raises `AttributeError`. `site_info` is unaffected.
 
 **06/23/2026:** **Breaking change (1.2.0):** removed the `nadp` module and the deprecated `samples` module ahead of the 1.2.0 release. `nadp` was deprecated on 05/01/2026 — NADP is not a USGS data source, so retrieve NADP data directly from https://nadp.slh.wisc.edu/. The `samples.get_usgs_samples` shim (a deprecated forward to the modern getter) is gone; use `waterdata.get_samples()` instead. `import dataretrieval.nadp` / `import dataretrieval.samples` now raise `ModuleNotFoundError`.

docs/source/architecture/decisions/0001-modular-monolith.rst

@@ -0,0 +1,49 @@
+ADR 0001: Retain a modular monolith with service adapters
+=========================================================
+
+Status
+------
+
+Accepted
+
+Context
+-------
+
+``dataretrieval`` is one Python library serving users who often combine several
+hydrologic data sources in one process or notebook. The upstream services have
+different protocols and schemas, but the package has one release lifecycle and
+maintainer group. Distributed deployment or separately versioned service
+packages would add operational and compatibility cost without improving the
+library's primary use cases.
+
+Decision
+--------
+
+Maintain one installable distribution organized as a modular monolith. Expose
+functions grouped by data portal. Keep service- and protocol-specific adapters
+independent behind those facades, and share infrastructure only where its
+contract is genuinely API-neutral.
+
+Treat the OGC subsystem as a protocol component used by Water Data and NGWMN,
+not as a universal service framework. Do not force NLDI, StreamStats, WQP, or
+Water Use into OGC-shaped return values or paging semantics.
+
+Consequences
+------------
+
+- Users install and version one coherent package.
+- Cross-service API compatibility can be tested in one pipeline.
+- Component boundaries are source-level rather than deployment boundaries and
+  therefore require import checks and review discipline.
+- Shared infrastructure must remain small enough that it does not become a god
+  module.
+- A new service should begin as its own adapter and earn shared abstractions
+  through demonstrated duplication rather than up-front generalization.
+
+Compliance
+----------
+
+``tests/architecture_test.py`` prevents shared OGC infrastructure from
+importing service adapters and prevents modern modules from depending on legacy
+NWIS. The installed-wheel CI job verifies that the whole monolith ships as one
+usable artifact.

docs/source/architecture/decisions/0002-sync-api-async-internals.rst

@@ -0,0 +1,47 @@
+ADR 0002: Keep synchronous public APIs over async internals
+===========================================================
+
+Status
+------
+
+Accepted
+
+Context
+-------
+
+The established public API consists of synchronous functions used heavily in
+scripts, notebooks, pandas workflows, and teaching examples. OGC pagination and
+chunk fan-out benefit from asynchronous I/O, but exposing only async functions
+would be a broad breaking change and would complicate common notebook use.
+
+Calling ``asyncio.run`` directly is also unsafe when a caller already has a
+running event loop, as Jupyter commonly does.
+
+Decision
+--------
+
+Keep public service getters synchronous. Async-capable implementations may run
+inside a short-lived anyio blocking portal and use ``httpx.AsyncClient`` for
+pagination and bounded fan-out. Internal async functions are implementation
+details, not a second public API promise.
+
+Ambient per-call policy must propagate into the worker context. A resumable OGC
+call captures the context needed to rebuild its remaining requests after the
+original getter has returned.
+
+Consequences
+------------
+
+- Existing scripts and notebooks retain simple blocking call sites.
+- Concurrent network waits improve large paginated downloads.
+- Each top-level async-backed call pays worker-thread and portal startup cost.
+- Cancellation, context propagation, and client ownership need explicit tests.
+- A future public async API, if justified, should be additive and share the same
+  lower-level contracts rather than duplicate behavior.
+
+Compliance
+----------
+
+Tests exercise calls inside an already running event loop, ambient context
+capture on resume, cancellation precedence, bounded in-flight work, and shared
+client ownership.

docs/source/architecture/decisions/0003-dependency-direction.rst

@@ -0,0 +1,52 @@
+ADR 0003: Direct dependencies toward stable policy
+==================================================
+
+Status
+------
+
+Accepted
+
+Context
+-------
+
+Service modules change when upstream APIs change. Error policy, generic data
+structures, and transport contracts should be more stable. If shared
+infrastructure imports service details, unrelated upstream changes propagate
+inward and reuse becomes unsafe. Several current private imports also act as
+unintended cross-package contracts.
+
+Decision
+--------
+
+Dependencies point from public facades to service/protocol adapters and then to
+stable shared policy and third-party infrastructure. In particular:
+
+- ``dataretrieval.exceptions`` is a runtime-dependency-light leaf.
+- ``dataretrieval.ogc`` must not import Water Data, NGWMN, Water Use, or NWIS.
+- Modern modules must not import deprecated NWIS.
+- New non-OGC services must not obtain generic transport behavior by importing
+  private OGC implementation symbols.
+
+Underscore-prefixed symbols remain implementation details even when existing
+internal modules currently use them.
+
+Consequences
+------------
+
+- Stable policy can be reused without pulling in service schemas.
+- Static import checks can detect architectural erosion early.
+- Moving existing private seams requires compatibility-aware sequencing.
+- Some duplication is preferable to a premature abstraction that couples
+  unlike upstream protocols.
+
+Compliance
+----------
+
+``tests/architecture_test.py`` parses runtime imports and enforces the rules
+that hold today. Its allowlist is the authoritative inventory of exact temporary
+cross-boundary imports; this ADR owns the direction and rationale rather than a
+second copy of that mutable inventory.
+
+The allowlist should shrink as private seams move. Any growth requires explicit
+architecture review, and a change to the dependency policy requires this ADR to
+be superseded.

docs/source/architecture/decisions/0004-error-retry-resume.rst

@@ -0,0 +1,52 @@
+ADR 0004: Use typed failures and bounded recovery
+=================================================
+
+Status
+------
+
+Accepted
+
+Context
+-------
+
+Remote hydrologic services fail through HTTP statuses, rate limits, timeouts,
+invalid payloads, and mid-pagination interruptions. Returning partial data after
+an undetected page failure is worse than raising. At the same time, retrying
+without limits can stall callers, multiply quota usage, or hide persistent
+faults.
+
+Decision
+--------
+
+All request failures exposed by public service modules derive from
+``DataRetrievalError`` and provide uniform ``status_code``, ``retry_after``, and
+``retryable`` attributes. Status mapping lives in one policy function.
+
+Where automatic recovery is supported, retries are bounded, use exponential
+backoff with full jitter, honor only bounded ``Retry-After`` delays, and preserve
+cancellation. OGC fan-out retains completed subrequests and raises a typed
+``ChunkInterrupted`` with a handle that resumes only missing work. Fatal or
+unknown failures are not disguised as resumable transients.
+
+This decision does not assert that every upstream API supports pagination,
+chunking, or resume. Those capabilities remain explicit per service until a
+shared API-neutral transport contract is introduced.
+
+Consequences
+------------
+
+- Callers can catch one stable base error and still branch on useful fields.
+- Mid-pagination failure cannot silently look like a complete dataset.
+- Retry can increase latency and request quota, so policy and defaults are part
+  of observable behavior.
+- Partial OGC state requires careful serialization and finalization tests.
+- Expanding retry to another service requires service-specific idempotency and
+  failure-contract tests.
+
+Compliance
+----------
+
+Tests cover status-to-type mapping, uniform fields, transport wrapping,
+pagination failure, retry exhaustion and jitter bounds, ``Retry-After`` limits,
+resume equivalence, partial-state stability, pickling, and cancellation
+precedence.

docs/source/architecture/decisions/0005-legacy-nwis.rst

@@ -0,0 +1,48 @@
+ADR 0005: Quarantine and retire legacy NWIS
+===========================================
+
+Status
+------
+
+Accepted
+
+Context
+-------
+
+The legacy ``dataretrieval.nwis`` facade overlaps modern Water Data APIs and
+contains functions backed by retired services. Existing users still require a
+migration window, while new code should not acquire dependencies on behavior
+scheduled for removal.
+
+Decision
+--------
+
+Keep ``dataretrieval.nwis`` importable during its published deprecation window
+and remove it on or after 2027-05-06 through the project's compatibility and
+release process.
+
+Add no new NWIS capabilities. Permit compatibility, security, and correctness
+fixes only. Every active deprecated function should warn once per user call and
+name a supported replacement where one exists. Modern modules must never import
+NWIS.
+
+Do not treat WQP's legacy result profiles as part of NWIS retirement; they have
+separate upstream constraints and migration behavior.
+
+Consequences
+------------
+
+- Existing users have a predictable migration period.
+- Maintainers avoid investing in a second implementation of modern retrieval
+  behavior.
+- Legacy integration tests may require special handling as upstream endpoints
+  disappear.
+- Removal still requires release notes, replacement checks, and an intentional
+  compatibility boundary.
+
+Compliance
+----------
+
+Deprecation tests verify one warning per public call and validate named Water
+Data replacements. ``tests/architecture_test.py`` prevents modern package
+modules from importing ``dataretrieval.nwis``.

docs/source/architecture/decisions/index.rst

@@ -0,0 +1,25 @@
+Architecture Decision Records
+=============================
+
+Architecture Decision Records (ADRs) capture why a significant structural or
+cross-cutting choice was made, the trade-offs it accepts, and how compliance is
+checked. They complement code and API documentation rather than repeating
+implementation details.
+
+Statuses are ``Proposed``, ``Accepted``, ``Superseded``, or ``Rejected``. An
+accepted decision is not edited to reverse its meaning; a later ADR supersedes
+it and links back to the old record. Keep records concise and commit them with
+the change that makes the decision effective.
+
+Use :doc:`template` when proposing a decision. Number accepted and proposed
+records sequentially.
+
+.. toctree::
+   :maxdepth: 1
+
+   0001-modular-monolith
+   0002-sync-api-async-internals
+   0003-dependency-direction
+   0004-error-retry-resume
+   0005-legacy-nwis
+   template