Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdded an AST-based soup API and OpenAPI schemas, generalized soup request/query types over a filter type, introduced IntoSoupReqAst, updated service/domain/service logic and handlers to be generic, changed cursor extraction to optional Option<CursorWithValAndFilter...>, and removed the old document_storage_service soup handler module. Changes
Possibly related PRs
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
rust/cloud-storage/soup/src/inbound/axum_router/tests.rs (1)
1-923: 🧹 Nitpick | 🔵 TrivialConsider adding test coverage for the new
/soup/astendpoint.The tests thoroughly cover the existing
/soupendpoint withEntityFilters, but there's no test exercising the newpost_soup_ast_handlerat/soup/astwithEntityFilterAstpayloads. Adding at least one integration test would verify the AST endpoint's request parsing and cursor handling work correctly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/cloud-storage/soup/src/inbound/axum_router/tests.rs` around lines 1 - 923, Tests exercise /soup but not the new AST endpoint; add an integration test that sends a POST to /soup/ast using an EntityFilterAst payload and asserts the router invokes the service and handles cursors correctly. Create a test similar to existing ones (use soup_router(SoupRouterState::new(...)) and MockSoup/MockEmail/MockEmailLinkResult), call the post_soup_ast_handler via a Request to "/soup/ast" with a JSON body representing EntityFilterAst, then inspect MockSoup.called to assert the received request contains the expected cursor/filter and that cursor kind handling (Simple vs Frecency) is exercised. Ensure you reference the handler name post_soup_ast_handler (or the /soup/ast route on soup_router), the EntityFilterAst payload shape, and the MockSoup.called assertions to validate behavior.
🤖 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/service-clients/service-storage/openapi.json`:
- Around line 3107-3109: The new OpenAPI operation with operationId
"post_items_soup_ast" under tag "soup::inbound::axum_router" is missing a
summary; add a concise "summary" field (for example "/items/soup") to that POST
operation to improve generated docs and SDK discoverability, placing it
alongside the existing "tags" and "operationId" properties.
- Around line 7054-7056: Fix the typo in the OpenAPI schema description for the
property named "chanf": change the word "taht" to "that" in the description
string so it reads "the filters that should be applied to the channel entity"
(update the "chanf" property's description in the openapi.json).
In `@rust/cloud-storage/document_storage_service/src/api/items/soup.rs`:
- Around line 21-23: Remove the duplicate import of CursorWithVal from
models_pagination: keep a single import line that brings in CursorWithVal and
CursorVal (or separate unique imports) and delete the redundant CursorWithVal
import to eliminate the duplicate symbol import in soup.rs; ensure only one use
models_pagination::CursorWithVal; remains (or consolidate into one use
models_pagination::{CursorWithVal, CursorVal};).
---
Outside diff comments:
In `@rust/cloud-storage/soup/src/inbound/axum_router/tests.rs`:
- Around line 1-923: Tests exercise /soup but not the new AST endpoint; add an
integration test that sends a POST to /soup/ast using an EntityFilterAst payload
and asserts the router invokes the service and handles cursors correctly. Create
a test similar to existing ones (use soup_router(SoupRouterState::new(...)) and
MockSoup/MockEmail/MockEmailLinkResult), call the post_soup_ast_handler via a
Request to "/soup/ast" with a JSON body representing EntityFilterAst, then
inspect MockSoup.called to assert the received request contains the expected
cursor/filter and that cursor kind handling (Simple vs Frecency) is exercised.
Ensure you reference the handler name post_soup_ast_handler (or the /soup/ast
route on soup_router), the EntityFilterAst payload shape, and the
MockSoup.called assertions to validate behavior.
🪄 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: 64afe2ad-d562-4ce6-b182-fdec7d13f573
⛔ Files ignored due to path filters (6)
js/app/packages/service-clients/service-storage/generated/schemas/entityFilterAst.tsis excluded by!**/generated/**js/app/packages/service-clients/service-storage/generated/schemas/index.tsis excluded by!**/generated/**js/app/packages/service-clients/service-storage/generated/schemas/postItemsSoupAstParams.tsis excluded by!**/generated/**js/app/packages/service-clients/service-storage/generated/schemas/postSoupAstRequest.tsis excluded by!**/generated/**js/app/packages/service-clients/service-storage/generated/schemas/postSoupAstRequestAllOf.tsis excluded by!**/generated/**js/app/packages/service-clients/service-storage/generated/zod.tsis excluded by!**/generated/**
📒 Files selected for processing (15)
js/app/packages/service-clients/service-storage/openapi.jsonrust/cloud-storage/channels/src/inbound/axum_router.rsrust/cloud-storage/document_storage_service/src/api/items/soup.rsrust/cloud-storage/document_storage_service/src/api/swagger.rsrust/cloud-storage/email/src/inbound/axum/previews_router.rsrust/cloud-storage/item_filters/src/ast.rsrust/cloud-storage/models_pagination/src/axum.rsrust/cloud-storage/notification/src/inbound/http/mod.rsrust/cloud-storage/notification_service/src/api/user_notification.rsrust/cloud-storage/soup/src/domain/models.rsrust/cloud-storage/soup/src/domain/ports.rsrust/cloud-storage/soup/src/domain/service.rsrust/cloud-storage/soup/src/inbound/axum_router.rsrust/cloud-storage/soup/src/inbound/axum_router/tests.rsrust/cloud-storage/soup/src/inbound/toolset/list_entities.rs
Comment on lines
21
to
23
| use models_pagination::CursorWithVal; | ||
| use models_pagination::CursorVal; | ||
| use models_pagination::CursorWithVal; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="rust/cloud-storage/document_storage_service/src/api/items/soup.rs"
python - <<'PY'
from collections import Counter
import re
from pathlib import Path
p = Path("rust/cloud-storage/document_storage_service/src/api/items/soup.rs")
imports = []
for i, line in enumerate(p.read_text().splitlines(), 1):
m = re.match(r'\s*use\s+([^;]+);', line)
if m:
imports.append((i, m.group(1).strip()))
counts = Counter(path for _, path in imports)
dups = {k:v for k,v in counts.items() if v > 1}
if not dups:
print("No duplicate imports found.")
else:
for imp, count in dups.items():
lines = [str(i) for i, pth in imports if pth == imp]
print(f"{imp} duplicated {count}x on lines {', '.join(lines)}")
PYRepository: macro-inc/macro
Length of output: 121
🌐 Web query:
Rust compiler duplicate identical use imports error E0252
💡 Result:
The Rust compiler error E0252 occurs when attempting to import two items (such as types, functions, or other values) with the same name into the same scope without using aliases. This creates a name conflict in the module's namespace. Example of erroneous code: use foo::baz; use bar::baz; // error[E0252]: the name baz is defined multiple times Official explanation from Rust documentation: "Two items of the same name cannot be imported without rebinding one of the items under a new local name." Solutions: 1. Use an alias (as) for one of the imports: use foo::baz as foo_baz; use bar::baz; Then use foo_baz and baz respectively. 2. Avoid importing one and use its fully qualified path: use bar::baz; let x = foo::baz; // Refer to the other via its parent module This applies to "identical use imports" from different modules or crates sharing the same item name, such as duplicate Poisson types from rand_distr and statrs as seen in community examples. The error is current as of Rust stable (docs unchanged through 2026-03-31).
Citations:
- 1: https://doc.rust-lang.org/error_codes/E0252.html
- 2: https://stackoverflow.com/questions/67510812/how-can-i-import-two-types-with-the-same-name-from-different-modules
- 3: https://doc.rust-lang.org/error_codes/E0255.html
Remove duplicate CursorWithVal import.
Lines 21 and 23 import the same symbol from the same module. While Rust allows identical imports, the redundancy should be cleaned up.
Fix
-use models_pagination::CursorWithVal;
use models_pagination::CursorVal;
use models_pagination::CursorWithVal;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| use models_pagination::CursorWithVal; | |
| use models_pagination::CursorVal; | |
| use models_pagination::CursorWithVal; | |
| use models_pagination::CursorVal; | |
| use models_pagination::CursorWithVal; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rust/cloud-storage/document_storage_service/src/api/items/soup.rs` around
lines 21 - 23, Remove the duplicate import of CursorWithVal from
models_pagination: keep a single import line that brings in CursorWithVal and
CursorVal (or separate unique imports) and delete the redundant CursorWithVal
import to eliminate the duplicate symbol import in soup.rs; ensure only one use
models_pagination::CursorWithVal; remains (or consolidate into one use
models_pagination::{CursorWithVal, CursorVal};).
seanaye
force-pushed
the
seanaye/feat/expose-soup-ast
branch
from
99e8fde to
0e5bdbc
Compare
Contributor
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)
rust/cloud-storage/soup/src/inbound/axum_router.rs (1)
258-286: 🧹 Nitpick | 🔵 TrivialConsider adding
tracing::instrumenttoget_soup_handler.The
post_soup_handlerandpost_soup_ast_handlerboth have#[tracing::instrument(err, skip_all)], butget_soup_handlerdoes not. For consistency and observability, consider adding instrumentation here as well.♻️ Proposed addition
+#[tracing::instrument(err, skip_all)] pub async fn get_soup_handler<T, U>(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/cloud-storage/soup/src/inbound/axum_router.rs` around lines 258 - 286, Add tracing instrumentation to get_soup_handler by annotating the function with #[tracing::instrument(err, skip_all)] to match post_soup_handler and post_soup_ast_handler; place the attribute directly above the pub async fn get_soup_handler<T, U>(...) declaration, ensuring you skip recording all parameters and capture errors only (err) so observability and behavior remain consistent with the other handlers.
♻️ Duplicate comments (2)
js/app/packages/service-clients/service-storage/openapi.json (2)
7054-7056:⚠️ Potential issue | 🟡 MinorFix typo in
EntityFilterAst.chanfdescription.Line 7055 contains
"taht"instead of"that".✏️ Proposed fix
"chanf": { - "description": "the filters taht should be applied to the channel entity" + "description": "the filters that should be applied to the channel entity" },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/app/packages/service-clients/service-storage/openapi.json` around lines 7054 - 7056, The description for EntityFilterAst.chanf has a typo ("taht"); update the description string for the property named chanf in the EntityFilterAst schema so it reads "the filters that should be applied to the channel entity" (fixing "taht" → "that") to correct the typo.
3107-3109:⚠️ Potential issue | 🟡 MinorAdd a
summaryfor the new/items/soup/astoperation.Line 3107 defines a new operation but omits
summary, which hurts generated docs and SDK discoverability.✏️ Proposed fix
"post": { "tags": ["soup::inbound::axum_router"], + "summary": "Gets the items the user has access to", "operationId": "post_items_soup_ast",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/app/packages/service-clients/service-storage/openapi.json` around lines 3107 - 3109, The OpenAPI operation for path "/items/soup/ast" (operationId "post_items_soup_ast") is missing a human-readable summary; add a concise "summary" property to that POST operation (e.g., one-line description of what the endpoint does) so generated docs and SDKs can surface and discover the operation more clearly.
🤖 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/document_storage_service/src/api/swagger.rs`:
- Line 186: The OpenAPI registration omits schema entries for the request and
filter types used by the post_soup_ast_handler endpoint; update the schemas(...)
call that includes PostSoupRequest (used by post_soup_handler) to also include
PostSoupAstRequest and EntityFilterAst so the post_soup_ast_handler path has its
request and filter types documented; locate the schemas(...) array near where
post_soup_ast_handler is registered and add PostSoupAstRequest and
EntityFilterAst alongside the existing PostSoupRequest entry.
In `@rust/cloud-storage/item_filters/src/ast.rs`:
- Around line 94-97: Fix the typo in the doc comment for the field
channel_filter: change "the filters taht should be applied to the channel
entity" to "the filters that should be applied to the channel entity" in the
documentation comment above the pub channel_filter: LiteralTree<ChannelLiteral>
field so the comment reads correctly.
---
Outside diff comments:
In `@rust/cloud-storage/soup/src/inbound/axum_router.rs`:
- Around line 258-286: Add tracing instrumentation to get_soup_handler by
annotating the function with #[tracing::instrument(err, skip_all)] to match
post_soup_handler and post_soup_ast_handler; place the attribute directly above
the pub async fn get_soup_handler<T, U>(...) declaration, ensuring you skip
recording all parameters and capture errors only (err) so observability and
behavior remain consistent with the other handlers.
---
Duplicate comments:
In `@js/app/packages/service-clients/service-storage/openapi.json`:
- Around line 7054-7056: The description for EntityFilterAst.chanf has a typo
("taht"); update the description string for the property named chanf in the
EntityFilterAst schema so it reads "the filters that should be applied to the
channel entity" (fixing "taht" → "that") to correct the typo.
- Around line 3107-3109: The OpenAPI operation for path "/items/soup/ast"
(operationId "post_items_soup_ast") is missing a human-readable summary; add a
concise "summary" property to that POST operation (e.g., one-line description of
what the endpoint does) so generated docs and SDKs can surface and discover the
operation more clearly.
🪄 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: d0353557-6b61-4fb2-9efd-58e4864af749
⛔ Files ignored due to path filters (6)
js/app/packages/service-clients/service-storage/generated/schemas/entityFilterAst.tsis excluded by!**/generated/**js/app/packages/service-clients/service-storage/generated/schemas/index.tsis excluded by!**/generated/**js/app/packages/service-clients/service-storage/generated/schemas/postItemsSoupAstParams.tsis excluded by!**/generated/**js/app/packages/service-clients/service-storage/generated/schemas/postSoupAstRequest.tsis excluded by!**/generated/**js/app/packages/service-clients/service-storage/generated/schemas/postSoupAstRequestAllOf.tsis excluded by!**/generated/**js/app/packages/service-clients/service-storage/generated/zod.tsis excluded by!**/generated/**
📒 Files selected for processing (9)
js/app/packages/service-clients/service-storage/openapi.jsonrust/cloud-storage/document_storage_service/src/api/swagger.rsrust/cloud-storage/item_filters/src/ast.rsrust/cloud-storage/soup/src/domain/models.rsrust/cloud-storage/soup/src/domain/ports.rsrust/cloud-storage/soup/src/domain/service.rsrust/cloud-storage/soup/src/inbound/axum_router.rsrust/cloud-storage/soup/src/inbound/axum_router/tests.rsrust/cloud-storage/soup/src/inbound/toolset/list_entities.rs
seanaye
force-pushed
the
seanaye/feat/expose-soup-ast
branch
from
0e5bdbc to
64a131d
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (3)
rust/cloud-storage/document_storage_service/src/api/items/soup.rs (1)
21-23:⚠️ Potential issue | 🔴 CriticalRemove duplicate
CursorWithValimport (compile blocker).Line 21 and Line 23 import the exact same symbol from the same module, which causes a duplicate-name import error.
Proposed fix
use models_pagination::CursorWithVal; use models_pagination::CursorVal; -use models_pagination::CursorWithVal;#!/bin/bash set -euo pipefail python - <<'PY' from pathlib import Path import re from collections import Counter, defaultdict p = Path("rust/cloud-storage/document_storage_service/src/api/items/soup.rs") lines = p.read_text().splitlines() imports = [] for i, line in enumerate(lines, 1): m = re.match(r'^\s*use\s+([^;]+);', line) if m: imports.append((i, m.group(1).strip())) counts = Counter(path for _, path in imports) dups = {k: v for k, v in counts.items() if v > 1} if not dups: print("No duplicate imports found.") else: by_path = defaultdict(list) for ln, path in imports: by_path[path].append(ln) for path, _ in dups.items(): print(f"duplicate import: {path} at lines {by_path[path]}") PY🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/cloud-storage/document_storage_service/src/api/items/soup.rs` around lines 21 - 23, There is a duplicate import of CursorWithVal from models_pagination causing a compile error; remove the redundant use line so only a single use models_pagination::CursorWithVal; remains (keep the import used by functions in this module), and scan other use statements in soup.rs for similar duplicates to avoid further duplicate-name import errors.js/app/packages/service-clients/service-storage/openapi.json (2)
3107-3109:⚠️ Potential issue | 🟡 MinorAdd a
summaryto the newpost_items_soup_astoperation.Line 3107 defines a new operation without
summary, which reduces generated docs/SDK discoverability compared with/items/soup.✏️ Proposed fix
"post": { "tags": ["soup::inbound::axum_router"], + "summary": "Gets the items the user has access to", "operationId": "post_items_soup_ast",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/app/packages/service-clients/service-storage/openapi.json` around lines 3107 - 3109, The OpenAPI operation with operationId "post_items_soup_ast" (tagged "soup::inbound::axum_router") is missing a summary; add a concise human-readable "summary" field to that POST operation (e.g., one-line description of what the endpoint does) so generated docs/SDKs surface it similarly to the existing /items/soup endpoint and improve discoverability.
7059-7061:⚠️ Potential issue | 🟡 MinorFix typo in
chanfdescription.Line 7060 uses
"taht"instead of"that".✏️ Proposed fix
"chanf": { - "description": "the filters taht should be applied to the channel entity" + "description": "the filters that should be applied to the channel entity" },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/app/packages/service-clients/service-storage/openapi.json` around lines 7059 - 7061, Update the description value for the OpenAPI property "chanf" to correct the misspelling: replace "the filters taht should be applied to the channel entity" with "the filters that should be applied to the channel entity" in the openapi.json entry for "chanf".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@js/app/packages/service-clients/service-storage/openapi.json`:
- Around line 3107-3109: The OpenAPI operation with operationId
"post_items_soup_ast" (tagged "soup::inbound::axum_router") is missing a
summary; add a concise human-readable "summary" field to that POST operation
(e.g., one-line description of what the endpoint does) so generated docs/SDKs
surface it similarly to the existing /items/soup endpoint and improve
discoverability.
- Around line 7059-7061: Update the description value for the OpenAPI property
"chanf" to correct the misspelling: replace "the filters taht should be applied
to the channel entity" with "the filters that should be applied to the channel
entity" in the openapi.json entry for "chanf".
In `@rust/cloud-storage/document_storage_service/src/api/items/soup.rs`:
- Around line 21-23: There is a duplicate import of CursorWithVal from
models_pagination causing a compile error; remove the redundant use line so only
a single use models_pagination::CursorWithVal; remains (keep the import used by
functions in this module), and scan other use statements in soup.rs for similar
duplicates to avoid further duplicate-name import errors.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f15a0cf7-db43-4bdd-b0ee-bcb6cca453f2
⛔ Files ignored due to path filters (6)
js/app/packages/service-clients/service-storage/generated/schemas/entityFilterAst.tsis excluded by!**/generated/**js/app/packages/service-clients/service-storage/generated/schemas/index.tsis excluded by!**/generated/**js/app/packages/service-clients/service-storage/generated/schemas/postItemsSoupAstParams.tsis excluded by!**/generated/**js/app/packages/service-clients/service-storage/generated/schemas/postSoupAstRequest.tsis excluded by!**/generated/**js/app/packages/service-clients/service-storage/generated/schemas/postSoupAstRequestAllOf.tsis excluded by!**/generated/**js/app/packages/service-clients/service-storage/generated/zod.tsis excluded by!**/generated/**
📒 Files selected for processing (15)
js/app/packages/service-clients/service-storage/openapi.jsonrust/cloud-storage/channels/src/inbound/axum_router.rsrust/cloud-storage/document_storage_service/src/api/items/soup.rsrust/cloud-storage/document_storage_service/src/api/swagger.rsrust/cloud-storage/email/src/inbound/axum/previews_router.rsrust/cloud-storage/item_filters/src/ast.rsrust/cloud-storage/models_pagination/src/axum.rsrust/cloud-storage/notification/src/inbound/http/mod.rsrust/cloud-storage/notification_service/src/api/user_notification.rsrust/cloud-storage/soup/src/domain/models.rsrust/cloud-storage/soup/src/domain/ports.rsrust/cloud-storage/soup/src/domain/service.rsrust/cloud-storage/soup/src/inbound/axum_router.rsrust/cloud-storage/soup/src/inbound/axum_router/tests.rsrust/cloud-storage/soup/src/inbound/toolset/list_entities.rs
seanaye
force-pushed
the
seanaye/feat/expose-soup-ast
branch
from
64a131d to
5ee822f
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
rust/cloud-storage/document_storage_service/src/api/items/soup.rs (1)
21-23:⚠️ Potential issue | 🔴 CriticalRemove the second
CursorWithValimport.Lines 21 and 23 import the same symbol into the same module scope. Rust treats that as a duplicate definition, so this file will not compile until one of them is removed.
Suggested fix
-use models_pagination::CursorWithVal; use models_pagination::CursorVal; use models_pagination::CursorWithVal;Run this read-only check to confirm the duplication:
#!/bin/bash set -euo pipefail FILE="rust/cloud-storage/document_storage_service/src/api/items/soup.rs" rg -n '^use models_pagination::CursorWithVal;$' "$FILE"Expected result: two matches at Lines 21 and 23.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/cloud-storage/document_storage_service/src/api/items/soup.rs` around lines 21 - 23, The file has a duplicate import of the same symbol CursorWithVal (two identical use models_pagination::CursorWithVal; lines); remove the redundant import so CursorWithVal is only imported once and ensure the remaining imports (e.g., CursorVal and any other models_pagination uses) remain untouched; locate the duplicate use statement for CursorWithVal in the soup.rs imports and delete one occurrence.js/app/packages/service-clients/service-storage/openapi.json (2)
7059-7061:⚠️ Potential issue | 🟡 MinorFix typo in
EntityFilterAstchannel description.Line 7060 uses
tahtinstead ofthat.✏️ Proposed fix
"chanf": { - "description": "the filters taht should be applied to the channel entity" + "description": "the filters that should be applied to the channel entity" },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/app/packages/service-clients/service-storage/openapi.json` around lines 7059 - 7061, In the EntityFilterAst JSON schema update the description for the "chanf" property to correct the typo by replacing "taht" with "that" so it reads "the filters that should be applied to the channel entity"; locate the "chanf" entry in EntityFilterAst within the OpenAPI JSON and update its description string accordingly.
3107-3109:⚠️ Potential issue | 🟡 MinorAdd a
summaryfor the new/items/soup/astoperation.Line 3107 introduces a new operation without
summary, which hurts docs/SDK discoverability consistency with nearby soup endpoints.✏️ Proposed fix
"post": { "tags": ["soup::inbound::axum_router"], + "summary": "Gets the items the user has access to", "operationId": "post_items_soup_ast",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/app/packages/service-clients/service-storage/openapi.json` around lines 3107 - 3109, The new OpenAPI operation with operationId "post_items_soup_ast" for the path "/items/soup/ast" is missing a summary; add a concise "summary" property (matching the style of nearby soup endpoints) to the POST operation object to improve docs/SDK discoverability and consistency with other soup endpoints.
🤖 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/service-clients/service-storage/openapi.json`:
- Around line 8903-8921: The OpenAPI schema PostSoupAstRequest currently
flattens EntityFilterAst and Params but must match the Rust handler contract
that expects a body object with explicit fields "filters", "sort", and "limit";
update the PostSoupAstRequest definition (instead of referencing EntityFilterAst
+ Params) to include properties filters, sort, and limit with types/structures
that mirror the handler's expected shapes, preserve any existing emailView
property, and remove the flattened inheritance so generated clients produce
valid payloads compatible with the rust/cloud-storage/soup inbound handler.
In `@rust/cloud-storage/soup/src/domain/models.rs`:
- Around line 218-237: The into_ast implementation for IntoSoupReqAst (for
SoupRequest<EntityFilterAst>) currently wraps every EntityFilterAst in Some
(cursor.map(Some)), causing empty AST filters like `{}` to be treated as
effective filters; change the mapping in into_ast so that when converting the
cursor (and any other EntityFilterAst Option fields if present) you collapse
empty filters to None by checking the EntityFilterAst value (e.g., via an
is_empty-style check on EntityFilterAst) and only return Some(filter) when it is
non-empty, otherwise return None; update the cursor mapping inside into_ast
accordingly so the resulting SoupRequest<Option<EntityFilterAst>> treats empty
ASTs as None.
In `@rust/cloud-storage/soup/src/inbound/axum_router.rs`:
- Around line 372-375: The utoipa path attribute for post_items_soup_ast should
be expanded to multiple lines to match other handlers and include the request
body type: update the #[utoipa::path(...)] on the post_items_soup_ast endpoint
to use multiline formatting and add request_body = PostSoupAstRequest while
keeping existing params and responses (status 200 -> SoupPage, 500 ->
ErrorResponse) so the OpenAPI docs fully describe the POST payload and improve
readability.
---
Duplicate comments:
In `@js/app/packages/service-clients/service-storage/openapi.json`:
- Around line 7059-7061: In the EntityFilterAst JSON schema update the
description for the "chanf" property to correct the typo by replacing "taht"
with "that" so it reads "the filters that should be applied to the channel
entity"; locate the "chanf" entry in EntityFilterAst within the OpenAPI JSON and
update its description string accordingly.
- Around line 3107-3109: The new OpenAPI operation with operationId
"post_items_soup_ast" for the path "/items/soup/ast" is missing a summary; add a
concise "summary" property (matching the style of nearby soup endpoints) to the
POST operation object to improve docs/SDK discoverability and consistency with
other soup endpoints.
In `@rust/cloud-storage/document_storage_service/src/api/items/soup.rs`:
- Around line 21-23: The file has a duplicate import of the same symbol
CursorWithVal (two identical use models_pagination::CursorWithVal; lines);
remove the redundant import so CursorWithVal is only imported once and ensure
the remaining imports (e.g., CursorVal and any other models_pagination uses)
remain untouched; locate the duplicate use statement for CursorWithVal in the
soup.rs imports and delete one occurrence.
🪄 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: 6a97ed60-c870-4987-8a64-63910090eb7d
⛔ Files ignored due to path filters (6)
js/app/packages/service-clients/service-storage/generated/schemas/entityFilterAst.tsis excluded by!**/generated/**js/app/packages/service-clients/service-storage/generated/schemas/index.tsis excluded by!**/generated/**js/app/packages/service-clients/service-storage/generated/schemas/postItemsSoupAstParams.tsis excluded by!**/generated/**js/app/packages/service-clients/service-storage/generated/schemas/postSoupAstRequest.tsis excluded by!**/generated/**js/app/packages/service-clients/service-storage/generated/schemas/postSoupAstRequestAllOf.tsis excluded by!**/generated/**js/app/packages/service-clients/service-storage/generated/zod.tsis excluded by!**/generated/**
📒 Files selected for processing (15)
js/app/packages/service-clients/service-storage/openapi.jsonrust/cloud-storage/channels/src/inbound/axum_router.rsrust/cloud-storage/document_storage_service/src/api/items/soup.rsrust/cloud-storage/document_storage_service/src/api/swagger.rsrust/cloud-storage/email/src/inbound/axum/previews_router.rsrust/cloud-storage/item_filters/src/ast.rsrust/cloud-storage/models_pagination/src/axum.rsrust/cloud-storage/notification/src/inbound/http/mod.rsrust/cloud-storage/notification_service/src/api/user_notification.rsrust/cloud-storage/soup/src/domain/models.rsrust/cloud-storage/soup/src/domain/ports.rsrust/cloud-storage/soup/src/domain/service.rsrust/cloud-storage/soup/src/inbound/axum_router.rsrust/cloud-storage/soup/src/inbound/axum_router/tests.rsrust/cloud-storage/soup/src/inbound/toolset/list_entities.rs
Comment on lines
+8903
to
+8921
| "PostSoupAstRequest": { | ||
| "allOf": [ | ||
| { | ||
| "$ref": "#/components/schemas/EntityFilterAst" | ||
| }, | ||
| { | ||
| "$ref": "#/components/schemas/Params" | ||
| }, | ||
| { | ||
| "type": "object", | ||
| "properties": { | ||
| "emailView": { | ||
| "type": "string", | ||
| "description": "the view of specific emails to display" | ||
| } | ||
| } | ||
| } | ||
| ] | ||
| }, |
Contributor
There was a problem hiding this comment.
PostSoupAstRequest schema is inconsistent with the Rust handler contract.
Line 8903 models a flattened request (EntityFilterAst + Params), but the backend handler expects a body with filters, sort, and limit fields (rust/cloud-storage/soup/src/inbound/axum_router.rs, Lines 361-370 and Lines 372-390). This can generate incorrect clients and invalid request payloads.
💡 Proposed schema alignment
"PostSoupAstRequest": {
- "allOf": [
- {
- "$ref": "#/components/schemas/EntityFilterAst"
- },
- {
- "$ref": "#/components/schemas/Params"
- },
- {
- "type": "object",
- "properties": {
- "emailView": {
- "type": "string",
- "description": "the view of specific emails to display"
- }
- }
- }
- ]
+ "type": "object",
+ "required": ["filters", "sort", "limit"],
+ "properties": {
+ "filters": {
+ "$ref": "#/components/schemas/EntityFilterAst"
+ },
+ "sort": {
+ "type": "array",
+ "items": {
+ "$ref": "#/components/schemas/SoupApiSort"
+ }
+ },
+ "limit": {
+ "type": "integer",
+ "format": "int64",
+ "minimum": 0
+ }
+ }
},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@js/app/packages/service-clients/service-storage/openapi.json` around lines
8903 - 8921, The OpenAPI schema PostSoupAstRequest currently flattens
EntityFilterAst and Params but must match the Rust handler contract that expects
a body object with explicit fields "filters", "sort", and "limit"; update the
PostSoupAstRequest definition (instead of referencing EntityFilterAst + Params)
to include properties filters, sort, and limit with types/structures that mirror
the handler's expected shapes, preserve any existing emailView property, and
remove the flattened inheritance so generated clients produce valid payloads
compatible with the rust/cloud-storage/soup inbound handler.
2915751 to
f90d272
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
rust/cloud-storage/soup/src/inbound/axum_router.rs (1)
373-385: 🧹 Nitpick | 🔵 TrivialConsider documenting the 400 response for AST expansion errors.
The
utoipa::pathattribute documents 200 and 500 responses, butSoupHandlerErr::ExpandErrreturns a 400 BAD_REQUEST (line 230). Since this endpoint specifically accepts AST filters that can fail expansion validation, documenting the 400 response improves API clarity for consumers.Suggested improvement
#[utoipa::path( post, operation_id = "post_items_soup_ast", path = "/items/soup/ast", params( ("cursor" = Option<String>, Query, description = "Base64 encoded cursor value."), ), request_body = PostSoupAstRequest, responses( (status = 200, body=SoupPage), + (status = 400, body=ErrorResponse, description = "Invalid filter arguments provided"), (status = 500, body=ErrorResponse), ) )]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/cloud-storage/soup/src/inbound/axum_router.rs` around lines 373 - 385, The OpenAPI documentation for the endpoint annotated by the utoipa::path on operation_id "post_items_soup_ast" currently lists only 200 and 500 responses but omits the 400 BAD_REQUEST returned by SoupHandlerErr::ExpandErr; update the responses list on that utoipa::path attribute to include a 400 response (e.g., (status = 400, body = ErrorResponse)) so AST expansion/validation errors are documented alongside PostSoupAstRequest, SoupPage and ErrorResponse.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@rust/cloud-storage/soup/src/inbound/axum_router.rs`:
- Around line 373-385: The OpenAPI documentation for the endpoint annotated by
the utoipa::path on operation_id "post_items_soup_ast" currently lists only 200
and 500 responses but omits the 400 BAD_REQUEST returned by
SoupHandlerErr::ExpandErr; update the responses list on that utoipa::path
attribute to include a 400 response (e.g., (status = 400, body = ErrorResponse))
so AST expansion/validation errors are documented alongside PostSoupAstRequest,
SoupPage and ErrorResponse.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: fe7b53a5-f946-4138-a65a-c6c761388019
⛔ Files ignored due to path filters (2)
js/app/packages/service-clients/service-storage/generated/schemas/entityFilterAst.tsis excluded by!**/generated/**js/app/packages/service-clients/service-storage/generated/zod.tsis excluded by!**/generated/**
📒 Files selected for processing (5)
js/app/packages/service-clients/service-storage/openapi.jsonrust/cloud-storage/document_storage_service/src/api/items/soup.rsrust/cloud-storage/item_filters/src/ast.rsrust/cloud-storage/soup/src/domain/models.rsrust/cloud-storage/soup/src/inbound/axum_router.rs
💤 Files with no reviewable changes (1)
- rust/cloud-storage/document_storage_service/src/api/items/soup.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pr exposes a new soup endpoint which accepts the raw soup ast values and returns a cursor over these values.
We also complete a long standing TODO for using OptionalFromRequestParts in the cursor.