Repository navigation
Conversation
deserialize_stream_record built models as model_class(record), leaving meta=None, while the paged path (ServiceBase.make_collection) builds model_class(resource, meta). Code moved from iterate() to stream() then crashed with AttributeError on the first .meta access, and neither stream() docstring mentioned the gap. Build one Meta per streaming read and share it across every model of that stream, as one page's models share their page's Meta. Its response is a headers-and-status snapshot with an empty body: the body is the stream itself and buffering it to fill the snapshot would give up the one-record memory bound that makes streaming worth using. Its pagination.total is the declared MPT-Item-Count, identical in both wire formats, with limit and offset left at zero because a stream has no pages. Deletion stubs stay meta-less — a stub is not a model and guarantees nothing but its id. The envelope's own $meta.pagination.total is still parsed and dropped rather than sourced: the header stays the single authoritative total, so a faulty response cannot overwrite it. Reading the count once up front for both the progress receiver and the Meta moved into a _negotiated_meta helper, keeping stream() within its local-variable budget; the events generator is passed unstarted, so set_total_items still fires before the first record leaves the wire. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Essentials Run ID: 📒 Files selected for processing (3)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
Included review availability: 2 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 3 reviews per hour. 📜 Recent review details🧰 Additional context used📓 Path-based instructions (5)Review documentation changes against `docs/documentation.md` and the linked repository's `standards/documentation.md`.⚙️ CodeRabbit configuration file Files:
For each subsequent commit in this PR, explicitly verify if previous review comments have been resolved⚙️ CodeRabbit configuration file Files:
When repository behavior changes, update the narrowest relevant document under `docs/`📄 CodeRabbit inference engine (docs/contributing.md) Files:
Topic-specific documentation must live in the matching file under `docs/` directory Shared engineering rules must be linked from `mpt-extension-skills` instead of copied into this repository📄 CodeRabbit inference engine (docs/documentation.md) Files:
Put topic-specific documentation under `docs/` directory instead of expanding `README.md` Link shared engineering rules from `mpt-extension-skills` instead of duplicating them locally in documentation📄 CodeRabbit inference engine (AGENTS.md) Files:
🧠 Learnings (1)📚 Learning: 2026-02-02T13:05:41.144ZApplied to files:
🔇 Additional comments (3)
📝 WalkthroughWalkthroughStreaming models now share response and pagination metadata for the complete stream. Synchronous and asynchronous paths create this metadata before record consumption. Deletion stubs remain metadata-free. Tests and documentation cover the new behavior. ChangesStreaming model metadata
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to Streaming models now expose shared response and pagination metadata in synchronous and asynchronous formats while preserving meta-less deletion stubs. The covered behavior is ready to merge with no active current-head risk identified. 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
🤖 AI-generated PR — Please review carefully.
What
Fixes MPT-24908: models yielded by
stream()carriedmeta=None, while models from the paged path carry aMeta.iterate(),fetch_page(),fetch_one()):ServiceBase.make_collectionbuilds each model asmodel_class(resource, meta), so every model carries the page'sMeta.stream(), both wire formats, sync and async):deserialize_stream_recordbuilt models asmodel_class(record), leavingmeta=None.Code migrated from
for m in svc.iterate(): use(m.meta.response.headers)— or any.metaaccess — tosvc.stream()therefore crashed withAttributeError: 'NoneType' object has no attribute ...on the first record. Neitherstream()docstring mentioned the gap, and in envelope format the$metamember that actually arrives on the wire was parsed into aStreamedTotaland then discarded rather than surfaced to the models.How
streaming_meta()inmpt_api_client/http/mixins/streaming_mixin.pybuilds oneMetaper streaming read, shared by every model of that stream — mirroring how one page's models share their page'sMeta.deserialize_stream_recordnow takes it and callsmodel_class(record, meta).meta.responseis a headers-and-status snapshot with an empty body. The body is the stream and is never buffered; buffering it to populate the snapshot would give up the one-record memory bound that makes streaming worth using.content/textare empty andjson()raises; headers andstatus_codeare real. Header names come out lowercased bydict(response.headers)— the paged path does the identical conversion, so the same lookup works on both.meta.pagination.totalis the declaredMPT-Item-Count, identical in both wire formats.limit/offsetstay0, sohas_next()isFalseandtotal_pages()is0: a stream is one response, not a cursor into a sequence of them.meta-less. ADeletionStubis not a model and guarantees nothing but itsid, so.metaon a streamed object still requires the existing stub branch.$meta.pagination.totalis still parsed and dropped rather than sourced — the header remains the single authoritative total, so a faulty response whose two carriers disagree cannot overwrite it.progressreceiver and theMetamoved into a_negotiated_metahelper, keepingstream()within itsWPS210local-variable budget. The events generator is passed unstarted, soset_total_itemsstill fires before the first record leaves the wire; the existing progress-ordering tests cover that.deserialize_stream_recordgained a required third parameter. That is not a public break: it is not exported fromhttp.mixinsorhttp, has no callers outside the mixin and its tests, andstream()has not shipped in a release.Testing
test_streaming_mixin.py: the declared count reachingmeta.pagination(sync, async, and envelope format), the response snapshot's headers/status/empty body, oneMetashared across a stream's models,metastaying out ofto_dict(), anddeserialize_stream_recordattaching the meta — plus the three existingdeserialize_stream_recordcall sites updated for the new parameter.has_next(),total_pages(),ignored,text,json()raising, and the stub'sAttributeError.docs/streaming.md: a new Whatmodel.metaCarries On A Streamed Model section with a paged-vs-streamed comparison table, plus amodel.metarow in thestream()-versus-iterate()table.make checkpasses (ruff format, ruff check, flake8, mypy, uv lock).make testpasses: 2635 tests, 99% overall coverage,streaming_mixin.pyat 100%.🤖 Generated with Claude Code
Closes MPT-24908
Metadata to models from synchronous and asynchronousstream()calls.limitandoffsetvalues.DeletionStubobjects.MPT-Item-Countresponse header.