feat(composition): bound ASGI request-body lifecycle - #443
seonghobae wants to merge 13 commits into
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughASGI 요청 본문을 제한된 바이트 수와 receive 이벤트 수로 읽는 함수를 추가했습니다. 이 함수는 입력과 이벤트를 검증하고, 잘못된 입력, 한도 초과, 클라이언트 연결 끊김을 구분합니다. 테스트는 본문 조립과 한도 경계, 예외 및 취소 전파를 확인합니다. Changes제한된 요청 본문 읽기
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Feature Merge Risk: ⚪ Minimal · up to This PR adds a standalone helper that reads ASGI request bodies within byte and event limits, with focused tests. It does not yet change any serving path, and no concrete defect was identified. It appears safe to merge once the exact-head test run the author already requires passes. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review Please review exact head |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf 'HEAD: '; git rev-parse HEAD
printf 'PR head via gh: '; gh pr view 443 --repo ContextualWisdomLab/Orgmetra --json headRefOid --jq '.headRefOid' || true
printf 'Named files:\n'; fd -t f '^(asgi_request_body\.py|test_asgi_request_body_lifecycle\.py)$'
printf 'Head changed paths:\n'; git diff-tree --no-commit-id --name-only -r f1dda8ed572997fc2f4abdbfbbe6e2c37aa2b036Length of output: 918 🏁 Script executed: #!/bin/bash
set -euo pipefail
ast-grep outline services/product-composition-api/src/orgmetra_product_composition/asgi_request_body.py --items all || true
ast-grep outline services/product-composition-api/tests/test_asgi_request_body_lifecycle.py --items all || true
printf '\nProduction module:\n'
nl -ba services/product-composition-api/src/orgmetra_product_composition/asgi_request_body.py
printf '\nLifecycle tests:\n'
nl -ba services/product-composition-api/tests/test_asgi_request_body_lifecycle.py
printf '\nCoverage configuration references:\n'
rg -n -C2 'branch\s*=|coverage|docstring|edge' services/product-composition-api/pyproject.toml services/product-composition-api/.coveragerc pyproject.toml 2>/dev/null | head -180 || trueLength of output: 13313 🏁 Script executed: #!/bin/bash
set -euo pipefail
ast-grep outline services/product-composition-api/src/orgmetra_product_composition/asgi_transport.py --items all || true
printf '\nTransport exception and projections:\n'
rg -n -C5 'class CompositionTransportError|CompositionClientDisconnectedError|CompositionRequestBodyTooLargeError|read_bounded_http_request_body' services/product-composition-api
printf '\nCoverage configuration:\n'
sed -n '20,58p' services/product-composition-api/pyproject.toml
printf '\nExact head statuses (read-only):\n'
gh api repos/ContextualWisdomLab/Orgmetra/commits/f1dda8ed572997fc2f4abdbfbbe6e2c37aa2b036/status --jq '{state: .state, statuses: [.statuses[] | {context, state, sha, updated_at}]}' || trueLength of output: 21288
The tests exercise every visible production control-flow outcome, including the non-callable You are interacting with an AI system. |
|
@coderabbitai review Please review exact head |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf 'HEAD '; git rev-parse HEAD
printf 'PR head '; gh pr view 443 --repo ContextualWisdomLab/Orgmetra --json headRefOid --jq '.headRefOid'
printf 'Head changed paths:\n'; git diff-tree --no-commit-id --name-only -r bcabe53aa6ebb901fa67b57cb86762b75c90c3c2
printf 'Candidate names:\n'; fd -t f '^(asgi_request_body\.py|test_asgi_request_body_lifecycle\.py|pyproject\.toml)$' | head -60
printf 'Changed commit overview:\n'; git show --format=fuller --stat --no-renames bcabe53aa6ebb901fa67b57cb86762b75c90c3c2 | head -90Length of output: 1998 🏁 Script executed: #!/bin/bash
set -euo pipefail
ast-grep outline services/product-composition-api/src/orgmetra_product_composition/asgi_request_body.py --items all
ast-grep outline services/product-composition-api/tests/test_asgi_request_body_lifecycle.py --items all
printf '\nModule:\n'; nl -ba services/product-composition-api/src/orgmetra_product_composition/asgi_request_body.py
printf '\nTests:\n'; nl -ba services/product-composition-api/tests/test_asgi_request_body_lifecycle.py
printf '\nHead diff:\n'; git show --format= --no-ext-diff bcabe53aa6ebb901fa67b57cb86762b75c90c3c2 -- services/product-composition-api/src/orgmetra_product_composition/asgi_request_body.py
printf '\nCoverage settings:\n'; rg -n -C3 'coverage|branch|docstring|edge|pytest' services/product-composition-api/pyproject.toml | head -150Length of output: 21324 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf 'PR diff paths against base:\n'; git diff --name-only d58dc07034082a18a5de6c9f4df03d40b096a72f bcabe53aa6ebb901fa67b57cb86762b75c90c3c2
printf 'Receive reader references:\n'; rg -n -C3 '\bread_bounded_http_request_body\s*\(|\bCompositionRequestBodyTooManyEventsError\b' services/product-composition-api
printf 'Test file history (compact):\n'; git log -4 --oneline -- services/product-composition-api/tests/test_asgi_request_body_lifecycle.py
printf 'Other adjacent tests and files:\n'; fd -t f 'asgi_.*\.py' services/product-composition-api | head -60
printf 'Exact-head commit status:\n'; gh api repos/ContextualWisdomLab/Orgmetra/commits/bcabe53aa6ebb901fa67b57cb86762b75c90c3c2/status --jq '{state, statuses: [.statuses[] | {context, state, sha, updated_at}]}' || trueLength of output: 13935
The reader accepts a terminal event in the final receive-event slot. If that event has Test gap: I found no obvious untested production control-flow outcome by source inspection. I did not run the tests or coverage, so I cannot certify statement, branch, docstring, or edge coverage for either changed file. The prior review and predecessor execution evidence do not establish acceptance for this head.
|
|
@coderabbitai review Please restart review on exact head |
|
✅ Action performedReview finished.
|
1 participant
Scope
ASGI request-body lifecycle owner stacked on #442 exact
9bbb792ef43fca8b08684f4f55a8bfe65e1120da. Current source head is728f07a8c86f92bbf33586f2a8007da9f31f5e06, open · Draft · mergeable, 13 commits / 2 files. This branch owns bounded request-body receive semantics only: byte accumulation, receive-event work, ASGI receive/event capability shape,http.disconnect, cancellation and server/runtime failure ownership. It does not own authentication, owner HTTP dispatch, success-response semantics, deployment packaging or buyer-path performance. No SQL/migration byte changes; PostgreSQL lineage remains 0018→0025.RED → repair
Earlier commits established exact body-byte and receive-event budgets, malformed-event rejection, non-callable/non-awaitable shape rejection, distinct disconnect handling, and await-side exception/cancellation preservation.
Fresh review of descendant #444 exposed an inconsistency in this owner: the old receive path wrapped every exception raised while invoking a callable as
CompositionRequestBodyError, while #444 correctly concluded that invocation timing alone cannot prove a local capability defect. ASGI definesreceiveas an awaitable callable; a non-callable capability or a normal non-awaitable return is demonstrably malformed, but a callable may itself raise a server/runtime failure before returning an awaitable.Test-first
6b61a5c78efc1000a96f73d96303d9623307eb85requires an ordinary synchronousRuntimeErrorto retain exact identity. Repair728f07a8c86f92bbf33586f2a8007da9f31f5e06removes the broad invocation wrapper and currentizes the public docstring. The boundary now fails closed only for demonstrable capability shape; invocation exceptions, await-side exceptions, and cancellation remain caller/server lifecycle authority.http.disconnectremains the protocol-defined peer lifecycle signal.Descendant adoption — current
#444 adopted this owner repair by ordinary-forward merged #445, then advanced to exact
e1af671deeeec682dffdf809cb8579dcc4d439dawith core response-event/status/header/send validation. Current response-completion leaf #446 exact7954f5bf606584ddb5bbcd29e1b64e49041b9409adds complete non-streaming response prevalidation, no-content/framing/transfer-coding authority and detached validated caller headers before the firstsendawait. No force-push or destructive rebase was used. #444/#446 remain outbound owners; this PR remains inbound receive/body owner.Verification boundary
This source-level RED → repair is not canonical runtime GREEN. Canonical acceptance must use one unchanged exact #446
7954f5bf606584ddb5bbcd29e1b64e49041b9409leaf and execute the inherited composition closure with exact owned statement/branch/docstring/edge coverage. #447 exact47ac3556dfef7d5d27af2af8ae2ba5038b837e7cis the current Draft #260 Foundation discovery/runtime implementation and remains non-GREEN pending normal #64/#305 metadata integration plus protected-base checks/review. #261 owns installed wheel/sdist acceptance. #311 exactdbc2fcf70ba6a6883381e8526cd62c6e19ce159cmust execute the complete inherited 0018→0025 PostgreSQL inventory on the same composition candidate. Source-level typing, mergeability, bot status and predecessor GREEN do not transfer.Proposed architecture #433 is source-current at
ff50182c25ccf45928b1413f947c992f62be2dedbut ADR 0432 remains Proposed.Keep Draft until same-tree package/install/PostgreSQL execution, prerequisite checks and qualifying independent review exist. No self-approval, administrator bypass, gate weakening, synthetic status, feature-local quality workflow, force-push/destructive rebase, mutable sibling source, predecessor-GREEN transfer, premature Ready/merge/release or simple Close is authorized.
Refs #432 #433 #435 #437 #438 #439 #440 #442 #444 #445 #446 #447 #260 #261 #311 #100.