Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughPayment-error handling was added to the chat creation flow: client JS now classifies payment errors and triggers the paywall UI, core util logic uses structured payment-error detection, the Rust router signature now accepts a Changes
Possibly related PRs
🚥 Pre-merge checks | ❌ 2❌ Failed checks (2 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
rust/cloud-storage/chat/src/inbound/http/router.rs (2)
160-166:⚠️ Potential issue | 🟠 MajorAdd
errto tracing instrumentation and skip_model_accessparameter.
create_chat_handlerreturnsResultand should include theerrattribute in itstracing::instrument. The_model_accessextractor parameter should also be skipped from spans.Suggested fix
-#[tracing::instrument(skip(state, user, req), fields(user_id = %user.macro_user_id))] +#[tracing::instrument(skip(state, user, _model_access, req), err, fields(user_id = %user.macro_user_id))] pub async fn create_chat_handler<S: ChatService, Svc: EntityAccessService>(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/cloud-storage/chat/src/inbound/http/router.rs` around lines 160 - 166, The tracing span for create_chat_handler is missing the err attribute and isn't skipping the `_model_access` extractor; update the #[tracing::instrument(...)] on create_chat_handler to include err and include `_model_access` in the skip list (so skip state, user, _model_access, req) while keeping fields(user_id = %user.macro_user_id) so errors from the Result-returning function are recorded and the extractor is not captured in the span.
148-157:⚠️ Potential issue | 🟠 MajorAdd missing 402 response to OpenAPI and fix tracing instrumentation.
The
ChatModelAccessextractor returnsStatusCode::PAYMENT_REQUIRED(402) when users lack AI model permissions, but this status is not documented in the@utoipa::pathmacro. Additionally, thecreate_chat_handlerreturnsResultbut thetracing::instrumentattribute is missing theerrflag, and_model_accessshould be skipped to prevent logging extraction internals.Suggested fixes
#[utoipa::path( post, path = "/chats", tag = "chats", operation_id = "create_chat", responses( (status = 200, body = StringIDResponse), (status = 401, body = String), + (status = 402, body = String), (status = 500, body = String), ) )] /// Create a new chat. -#[tracing::instrument(skip(state, user, req), fields(user_id = %user.macro_user_id))] +#[tracing::instrument(skip(state, user, req, _model_access), err, fields(user_id = %user.macro_user_id))] pub async fn create_chat_handler<S: ChatService, Svc: EntityAccessService>(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/cloud-storage/chat/src/inbound/http/router.rs` around lines 148 - 157, Update the utoipa::path macro for the POST /chats endpoint to include a (status = 402, body = String) response to document ChatModelAccess returning StatusCode::PAYMENT_REQUIRED, and adjust the tracing instrumentation on create_chat_handler: add err to the tracing::instrument attribute and add skip(_model_access) so the extractor internals are not logged; locate the macro near the create_chat_handler function and the ChatModelAccess extractor to apply these changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@js/app/packages/app/component/SoupChatInput.tsx`:
- Around line 74-79: SoupChatInput currently calls
cognitionApiServiceClient.createChat directly and duplicates payment-error
handling; move that network call into the queries layer as a TanStack Query
mutation (e.g., add a createChat mutation and exported hook like
useCreateChatMutation in the queries package) and have the central createChat
handler in core/util/create.ts perform payment-error logic. In SoupChatInput
replace the direct call to cognitionApiServiceClient.createChat and local
usePaywallState handling with the new mutation hook
(useCreateChatMutation().mutateAsync or mutate) and consume errors/results from
that hook, removing duplicate payment handling in the component so all
service-client network calls and payment-error behaviors are centralized.
---
Outside diff comments:
In `@rust/cloud-storage/chat/src/inbound/http/router.rs`:
- Around line 160-166: The tracing span for create_chat_handler is missing the
err attribute and isn't skipping the `_model_access` extractor; update the
#[tracing::instrument(...)] on create_chat_handler to include err and include
`_model_access` in the skip list (so skip state, user, _model_access, req) while
keeping fields(user_id = %user.macro_user_id) so errors from the
Result-returning function are recorded and the extractor is not captured in the
span.
- Around line 148-157: Update the utoipa::path macro for the POST /chats
endpoint to include a (status = 402, body = String) response to document
ChatModelAccess returning StatusCode::PAYMENT_REQUIRED, and adjust the tracing
instrumentation on create_chat_handler: add err to the tracing::instrument
attribute and add skip(_model_access) so the extractor internals are not logged;
locate the macro near the create_chat_handler function and the ChatModelAccess
extractor to apply these changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 630fe19a-252f-4649-a760-cdc6fe96dff3
📒 Files selected for processing (3)
js/app/packages/app/component/SoupChatInput.tsxjs/app/packages/core/util/create.tsrust/cloud-storage/chat/src/inbound/http/router.rs
| const response = await cognitionApiServiceClient.createChat({}); | ||
| if (isErr(response)) { | ||
| if (isPaymentError(response)) { | ||
| const { showPaywall } = usePaywallState(); | ||
| showPaywall(PaywallKey.CHAT_LIMIT); | ||
| } |
There was a problem hiding this comment.
Move chat creation to a query-layer mutation; avoid direct client calls in component.
This component still calls cognitionApiServiceClient.createChat directly and now duplicates payment-error handling already present in js/app/packages/core/util/create.ts (Line 239-254). Centralize this in the TanStack Query layer and consume that mutation here to prevent logic drift.
As per coding guidelines, "All network calls to service clients MUST go through TanStack Query in the queries package. Do NOT call service clients directly from components or other packages."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@js/app/packages/app/component/SoupChatInput.tsx` around lines 74 - 79,
SoupChatInput currently calls cognitionApiServiceClient.createChat directly and
duplicates payment-error handling; move that network call into the queries layer
as a TanStack Query mutation (e.g., add a createChat mutation and exported hook
like useCreateChatMutation in the queries package) and have the central
createChat handler in core/util/create.ts perform payment-error logic. In
SoupChatInput replace the direct call to cognitionApiServiceClient.createChat
and local usePaywallState handling with the new mutation hook
(useCreateChatMutation().mutateAsync or mutate) and consume errors/results from
that hook, removing duplicate payment handling in the component so all
service-client network calls and payment-error behaviors are centralized.
ehayes2000
force-pushed
the
paywallington
branch
from
1c07899 to
a34bb1a
Compare
ehayes2000
force-pushed
the
paywallington
branch
from
a34bb1a to
3386727
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
js/app/packages/app/component/SoupChatInput.tsx (1)
93-103:⚠️ Potential issue | 🟠 MajorBackground send also calls service client directly.
cognitionApiServiceClient.sendStreamChatMessageis called directly in the component. This should also be moved to the queries layer. Consider creating a mutation or using a centralized utility that wraps this call.As per coding guidelines, "All network calls to service clients MUST go through TanStack Query in the
queriespackage. Do NOT call service clients directly from components or other packages."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/app/packages/app/component/SoupChatInput.tsx` around lines 93 - 103, The component is calling cognitionApiServiceClient.sendStreamChatMessage directly (inside the backgroundSend branch) which violates the rule that all service-client network calls must go through TanStack Query in the queries package; create a mutation (e.g., sendStreamChatMessageMutation) inside the queries package that wraps cognitionApiServiceClient.sendStreamChatMessage, export hooks (useSendStreamChatMessage or similar), then in SoupChatInput replace the direct call with the mutation's mutate/mutateAsync call (pass content, model, chat_id, attachments, toolset) and move the invalidateAllSoup call to run after the mutation succeeds (or in onSettled/onSuccess) to preserve behavior and error handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@rust/cloud-storage/chat/src/inbound/http/router.rs`:
- Around line 164-165: The OpenAPI doc for create_chat is missing the 402
response emitted by the ChatModelAccess extractor; update the #[utoipa::path]
attribute on the create_chat handler to include a response with status = 402
(e.g., description "Payment required / model access denied") so the spec matches
the actual behavior of ChatModelAccess rejecting unauthorized model access.
- Around line 164-165: Add a new unit test named
create_chat_payment_required_returns_402 in
rust/cloud-storage/chat/src/inbound/test.rs that exercises the create_chat HTTP
route and asserts a 402 PAYMENT_REQUIRED is returned when the ChatModelAccess
extractor rejects; specifically, invoke the create_chat handler (the same router
used in existing tests) with request inputs that simulate missing model
permissions so ChatModelAccess fails, then assert the response status is 402.
Ensure the test uses the same route construction/fixtures as other inbound tests
so the extractor runs (referencing create_chat and ChatModelAccess) and includes
any necessary headers/body used by create_chat to reach the extractor path.
---
Outside diff comments:
In `@js/app/packages/app/component/SoupChatInput.tsx`:
- Around line 93-103: The component is calling
cognitionApiServiceClient.sendStreamChatMessage directly (inside the
backgroundSend branch) which violates the rule that all service-client network
calls must go through TanStack Query in the queries package; create a mutation
(e.g., sendStreamChatMessageMutation) inside the queries package that wraps
cognitionApiServiceClient.sendStreamChatMessage, export hooks
(useSendStreamChatMessage or similar), then in SoupChatInput replace the direct
call with the mutation's mutate/mutateAsync call (pass content, model, chat_id,
attachments, toolset) and move the invalidateAllSoup call to run after the
mutation succeeds (or in onSettled/onSuccess) to preserve behavior and error
handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 86a242c9-ddf8-44b8-b622-d5f63191e557
📒 Files selected for processing (4)
js/app/packages/app/component/SoupChatInput.tsxjs/app/packages/core/util/create.tsrust/cloud-storage/chat/src/inbound/http/router.rsrust/cloud-storage/chat/src/inbound/test.rs
💤 Files with no reviewable changes (1)
- rust/cloud-storage/chat/src/inbound/test.rs
| // 402 on no perms | ||
| _model_access: ChatModelAccess, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n rust/cloud-storage/chat/src/inbound/http/router.rs | head -180 | tail -40Repository: macro-inc/macro
Length of output: 1509
🏁 Script executed:
rg -A 20 "fn create_chat" rust/cloud-storage/chat/src/inbound/http/router.rsRepository: macro-inc/macro
Length of output: 681
🏁 Script executed:
rg -B 5 "ChatModelAccess" rust/cloud-storage/chat/src/inbound/http/router.rs | head -50Repository: macro-inc/macro
Length of output: 589
🏁 Script executed:
rg -r "impl.*FromRequestParts.*ChatModelAccess|impl.*FromRequest.*ChatModelAccess" --type rustRepository: macro-inc/macro
Length of output: 455
🏁 Script executed:
fd -type f -name "*.rs" | xargs rg "struct ChatModelAccess" -A 30Repository: macro-inc/macro
Length of output: 2960
🏁 Script executed:
cat -n rust/cloud-storage/chat/src/inbound/http/extractors.rs | grep -A 50 "NoModelAccess =>"Repository: macro-inc/macro
Length of output: 1522
Document the 402 response for create_chat in the OpenAPI specification.
The ChatModelAccess extractor on line 165 rejects with 402 (PAYMENT_REQUIRED) when users lack model permissions, but the #[utoipa::path] macro for create_chat currently omits this status code from its responses. Add the missing 402 response to match the actual handler behavior:
#[utoipa::path(
post,
path = "/chats",
tag = "chats",
operation_id = "create_chat",
responses(
(status = 200, body = StringIDResponse),
(status = 401, body = String),
+ (status = 402, body = String),
(status = 500, body = String),
)
)]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rust/cloud-storage/chat/src/inbound/http/router.rs` around lines 164 - 165,
The OpenAPI doc for create_chat is missing the 402 response emitted by the
ChatModelAccess extractor; update the #[utoipa::path] attribute on the
create_chat handler to include a response with status = 402 (e.g., description
"Payment required / model access denied") so the spec matches the actual
behavior of ChatModelAccess rejecting unauthorized model access.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify that create_chat tests exist and include paywall/402 coverage.
fd -a test.rs rust/cloud-storage/chat/src/inbound
rg -n -C3 --type=rust 'create_chat|PAYMENT_REQUIRED|status\s*[:=]\s*402|NoModelAccess|AI features require a paid subscription' rust/cloud-storage/chat/src/inbound/test.rsRepository: macro-inc/macro
Length of output: 121
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Examining router.rs context around lines 164-165 ==="
sed -n '155,175p' rust/cloud-storage/chat/src/inbound/http/router.rs
echo -e "\n=== Checking test.rs file size and structure ==="
wc -l rust/cloud-storage/chat/src/inbound/test.rs
head -50 rust/cloud-storage/chat/src/inbound/test.rs
echo -e "\n=== Search for 402 or PAYMENT_REQUIRED in entire chat inbound module ==="
rg -n 'PAYMENT_REQUIRED|402|NoModelAccess' rust/cloud-storage/chat/src/inbound/ --type=rust
echo -e "\n=== Search for create_chat function definition and tests ==="
rg -n 'fn create_chat|create_chat_test|test.*create_chat' rust/cloud-storage/chat/src/ --type=rust
echo -e "\n=== Check for inline tests in router.rs ==="
rg -n '#\[cfg.*test' rust/cloud-storage/chat/src/inbound/http/router.rs --type=rustRepository: macro-inc/macro
Length of output: 4246
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Full test.rs file to see existing tests ==="
cat rust/cloud-storage/chat/src/inbound/test.rs
echo -e "\n=== Verify ChatModelAccess is an extractor (FromRequestParts) ==="
rg -n 'impl.*FromRequestParts.*ChatModelAccess|struct ChatModelAccess' rust/cloud-storage/chat/src/inbound/http/extractors.rs -A5Repository: macro-inc/macro
Length of output: 15631
Add a test for the 402 rejection path in create_chat.
The ChatModelAccess extractor properly rejects with PAYMENT_REQUIRED (402) when model access is unavailable, but this path lacks HTTP-level test coverage. Add a create_chat_payment_required_returns_402 test in rust/cloud-storage/chat/src/inbound/test.rs that verifies the endpoint returns 402 when the extractor rejects due to missing model permissions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rust/cloud-storage/chat/src/inbound/http/router.rs` around lines 164 - 165,
Add a new unit test named create_chat_payment_required_returns_402 in
rust/cloud-storage/chat/src/inbound/test.rs that exercises the create_chat HTTP
route and asserts a 402 PAYMENT_REQUIRED is returned when the ChatModelAccess
extractor rejects; specifically, invoke the create_chat handler (the same router
used in existing tests) with request inputs that simulate missing model
permissions so ChatModelAccess fails, then assert the response status is 402.
Ensure the test uses the same route construction/fixtures as other inbound tests
so the extractor runs (referencing create_chat and ChatModelAccess) and includes
any necessary headers/body used by create_chat to reach the extractor path.
1 participant
No description provided.