Skip to content

Bounded list limits and NDJSON streaming exports - #14

Open
NathanCummings wants to merge 8 commits into
mainfrom
feat/adr-0020-bounded-streaming
Open

NathanCummings wants to merge 8 commits into
mainfrom
feat/adr-0020-bounded-streaming

Conversation

@NathanCummings

Copy link
Copy Markdown
Member

Carried over from GitLab MR !2, which was in conflict with main there. Eight commits: request limits are clamped to a configured bound, read-access policy is centralised in _resolve_access, and datasets, shots and collections gain /export endpoints that stream NDJSON.

Known from the GitLab review: the /export generators emit application/jsonl and ignore response_model options such as exclude_none, since FastAPI does not apply them to streamed responses. That is unresolved here and is the main thing to settle before merging.

The branch predates the recent changes to main and will need rebasing; the conflicts are the ones GitLab reported.

🤖 Generated with Claude Code

https://claude.ai/code/session_013E6ygEUq3gzUrTzPJxcwtH

NathanCummings and others added 8 commits May 8, 2026 17:25
Pull the access-policy logic in DatasetService, ShotService, and
CollectionService into a private `_resolve_access(obj, user) -> str | None`
method that returns None on success or a denial reason on failure.

`check_read_access` now raises ForbiddenError(reason) on a non-None
result; the new `is_accessible(obj, user) -> bool` returns
`_resolve_access(...) is None`. Both surface the same canonical logic
without duplication and without exception-driven control flow in the
predicate's hot path — required by ADR-0020's streaming export, which
calls `is_accessible` once per yielded row and cannot afford an
exception after response headers have been sent.

Also collapses each list endpoint's `_filter_accessible_*` helper into
a one-line list comprehension over the predicate.

Adds tests/services/test_access_predicates.py covering predicate
purity (returns bool, never raises) across all access tiers and a
broken-inheritance fallback case.
Add app/api/streaming.py exposing serialize_ndjson() and ndjson_response()
for the bulk-export endpoints (ADR-0020). One JSON object per line,
trailing newline, application/x-ndjson media type. Pydantic's
model_dump_json(exclude_none=True) drives the per-row serialisation.
Lives in app/api/ rather than app/core/ because it imports
fastapi.responses.StreamingResponse — keeps the web-framework dependency
out of app/core/ where everything else is web-framework-agnostic.

Add stream() / stream_by_device_name() to DatasetService, ShotService,
and CollectionService. Each:
  - Uses SQLAlchemy 2.0's stmt.execution_options(yield_per=...) to keep
    the API process's memory bounded regardless of corpus size.
  - Eager-loads the access-level inheritance chain via selectinload()
    so the per-row read-model conversion does not trigger N+1 queries
    inside the stream. Dataset.distributions, Dataset.shot.device, and
    Collection.shot.device are batched per chunk.
  - Calls is_accessible() for per-row filtering — pure predicate, never
    raises, so a per-row failure cannot abort an in-flight HTTP response
    after headers have been sent.

Add CollectionService.to_flat_read_model() — produces an export-shaped
CollectionRead with effective_access_level resolved but datasets and
child_collections set to None. Lives on the service so the export
router stays free of auth/model imports; mirrors how DatasetService
and ShotService already drive their export read-model conversion.

Adds tests/api/test_streaming.py covering NDJSON shape, exclude_none
behaviour, lazy iteration, and the content-type contract.

No HTTP surface change in this commit — the service stream() methods
are unused until the /export endpoints land in the next commit.
Wire the streaming machinery into HTTP endpoints, completing ADR-0020.
Each handler is a thin three-call chain:
service.stream(...) → service.to_*read_model(...) → ndjson_response(rows).

  - GET /api/v1/datasets/export — optional device_name and shot_id query
    filters mirror the path-based list endpoints.
  - GET /api/v1/devices/{device_name}/shots/export
  - GET /api/v1/collections/export — emits a flat CollectionRead via
    CollectionService.to_flat_read_model() (datasets and child_collections
    set to None) so per-row payloads stay bounded; clients pull membership
    separately by Collection ID.

Auth posture mirrors the corresponding list endpoint: open to anonymous
callers, with per-row access filtering silently dropping records the
caller cannot see. Empty result sets return 200 OK with an empty body.
Invalid tokens still 401 during dependency resolution before the
handler runs; once StreamingResponse headers are sent, the per-row
predicate stays raise-free so a per-row failure cannot abort the
in-flight response.

Adds tests/api/v1/test_export_streaming.py covering NDJSON shape,
content-type, anonymous + admin filtering, device/shot scope filters,
empty-corpus 200 OK, and a broken-inheritance fallback case where an
orphaned dataset is silently dropped from an anonymous caller's stream
without raising.

ADR-0020 status: Proposed → Accepted.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants