Conversation
|
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:
WalkthroughAdds a JSON ChangesSearch API
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Router
participant api_search
participant SearchBackend
participant Database
Client->>Router: GET /api/search with QuerySearch
Router->>api_search: QuerySearch parameters
api_search->>SearchBackend: Execute Tantivy or Qdrant search
SearchBackend-->>api_search: Matching IDs
api_search->>Database: Load Case records
Database-->>api_search: CaseData records
api_search-->>Client: JSON SearchData
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/controller.rs`:
- Around line 378-407: Update the vsearch handling around the Qdrant
search_points call so failures return a non-2xx error response instead of the
normal success JSON. Remove the precomputed offset-plus-limit total and derive
truthful pagination metadata by requesting limit + 1 results, trimming the extra
item, and setting has_more from its presence; preserve the requested offset and
returned IDs.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 87b58462-d3f6-48d8-8c3b-7d760cdddbca
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
src/bin/main.rssrc/controller.rssrc/lib.rs
| let search_limit = limit + offset; | ||
| total = search_limit; | ||
| if let Ok(search_result) = client | ||
| .search_points( | ||
| SearchPointsBuilder::new( | ||
| &CONFIG.collection_name, | ||
| query_vec.into_iter().next().unwrap(), | ||
| search_limit as u64, | ||
| ) | ||
| .with_payload(false) | ||
| .limit(limit as u64) | ||
| .offset(offset as u64), | ||
| ) | ||
| .await | ||
| { | ||
| for point in &search_result.result { | ||
| let id = point | ||
| .id | ||
| .as_ref() | ||
| .unwrap() | ||
| .point_id_options | ||
| .as_ref() | ||
| .unwrap(); | ||
| if let PointIdOptions::Num(id) = id { | ||
| ids.insert(*id as u32); | ||
| } | ||
| } | ||
| } else { | ||
| tracing::error!("Qdrant search_points failed"); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Return truthful vsearch metadata and an error when Qdrant fails.
total is set to offset + limit before the request, so an empty result at offset 0 reports total: 20. A Qdrant failure only logs and returns that same HTTP 200 JSON shape, making an outage indistinguishable from a valid search. Return a non-2xx response on failure and use verified pagination metadata (for example, has_more from a limit + 1 query) rather than a fabricated total.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/controller.rs` around lines 378 - 407, Update the vsearch handling around
the Qdrant search_points call so failures return a non-2xx error response
instead of the normal success JSON. Remove the precomputed offset-plus-limit
total and derive truthful pagination metadata by requesting limit + 1 results,
trimming the extra item, and setting has_more from its presence; preserve the
requested offset and returned IDs.
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)
src/controller.rs (1)
200-238: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftVsearch still reports a fabricated
totaland swallows Qdrant failures.
total = search_limit(limit + offset) is set before the query executes, so e.g. an empty result at offset 0 still reportstotal: 20. Onsearch_pointsfailure, onlytracing::error!is logged —idsstays empty and the handler still returns a normal 200 JSON/HTML shape, making a Qdrant outage indistinguishable from "no results." This is the same issue raised on a previous commit of this PR and remains unresolved after thesearch_casesextraction.♻️ Suggested direction
- let search_limit = limit + offset; - total = search_limit; - if let Ok(search_result) = client + if let Ok(search_result) = client .search_points( SearchPointsBuilder::new( &CONFIG.collection_name, query_vec.into_iter().next().unwrap(), - search_limit as u64, + (limit + 1) as u64, ) .with_payload(false) - .limit(limit as u64) + .limit((limit + 1) as u64) .offset(offset as u64), ) .await { + // derive has_more from the extra item, trim before use ... } else { tracing::error!("Qdrant search_points failed"); + // propagate as a real error instead of a fake 200 }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/controller.rs` around lines 200 - 238, Update the vsearch branch in the controller so `total` reflects the actual Qdrant result rather than being preset to `limit + offset`, and propagate `search_points` failures instead of logging them and returning an empty successful response. Preserve normal pagination and result handling for successful searches, while making the handler return its existing error response shape for Qdrant failures.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/controller.rs`:
- Around line 360-376: Remove full_text from the serialized CaseData response
while preserving its availability for the CSV export path in search. Update the
CaseData serialization behavior and any related JSON search handling so HTML and
/api/search return only the fields they consume, while the CSV branch continues
using case.full_text.
In `@templates/search.html`:
- Around line 57-65: Update the suggestion link in the search_meta vsearch block
to submit the query parameter named search_type, so clicking it explicitly
selects vsearch; preserve the existing search value and surrounding display
conditions.
---
Outside diff comments:
In `@src/controller.rs`:
- Around line 200-238: Update the vsearch branch in the controller so `total`
reflects the actual Qdrant result rather than being preset to `limit + offset`,
and propagate `search_points` failures instead of logging them and returning an
empty successful response. Preserve normal pagination and result handling for
successful searches, while making the handler return its existing error response
shape for Qdrant failures.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 57d320ba-b921-41f7-bd0c-8d5677be2969
📒 Files selected for processing (2)
src/controller.rstemplates/search.html
| #[derive(Serialize)] | ||
| pub struct CaseData { | ||
| id: u32, | ||
| preview: String, | ||
| doc_id: String, | ||
| case_id: String, | ||
| case_name: String, | ||
| court: String, | ||
| case_type: String, | ||
| procedure: String, | ||
| judgment_date: String, | ||
| public_date: String, | ||
| parties: String, | ||
| cause: String, | ||
| legal_basis: String, | ||
| full_text: String, | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
CaseData.full_text is unconditionally serialized for every search hit, even though nothing but CSV export needs it.
The HTML template (templates/search.html) only renders case.preview, never case.full_text; the new /api/search JSON endpoint (Lines 354-358) serializes it for every result regardless, and since api_search never branches on search_meta.export, calling it with export=true raises the limit to EXPORT_LIMIT (10000 by default) and returns full document text for up to 10000 hits as JSON. Only the CSV branch of search (Lines 320-334) actually consumes case.full_text. Excluding it from serialization keeps CSV working while avoiding a large, unintended payload/bandwidth cost on the new JSON endpoint.
♻️ Suggested fix
#[derive(Serialize)]
pub struct CaseData {
id: u32,
preview: String,
doc_id: String,
case_id: String,
case_name: String,
court: String,
case_type: String,
procedure: String,
judgment_date: String,
public_date: String,
parties: String,
cause: String,
legal_basis: String,
+ #[serde(skip_serializing)]
full_text: String,
}📝 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.
| #[derive(Serialize)] | |
| pub struct CaseData { | |
| id: u32, | |
| preview: String, | |
| doc_id: String, | |
| case_id: String, | |
| case_name: String, | |
| court: String, | |
| case_type: String, | |
| procedure: String, | |
| judgment_date: String, | |
| public_date: String, | |
| parties: String, | |
| cause: String, | |
| legal_basis: String, | |
| full_text: String, | |
| } | |
| #[derive(Serialize)] | |
| pub struct CaseData { | |
| id: u32, | |
| preview: String, | |
| doc_id: String, | |
| case_id: String, | |
| case_name: String, | |
| court: String, | |
| case_type: String, | |
| procedure: String, | |
| judgment_date: String, | |
| public_date: String, | |
| parties: String, | |
| cause: String, | |
| legal_basis: String, | |
| #[serde(skip_serializing)] | |
| full_text: String, | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/controller.rs` around lines 360 - 376, Remove full_text from the
serialized CaseData response while preserving its availability for the CSV
export path in search. Update the CaseData serialization behavior and any
related JSON search handling so HTML and /api/search return only the fields they
consume, while the CSV branch continues using case.full_text.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@static/help.txt`:
- Around line 108-110: Update the optional-parameter list in help.txt to
document the supported export parameter used by QuerySearch and search_cases,
including its effect on the API result limit; if /api/search does not support
it, explicitly state that instead. Align the wording with the shared handler
contract in QuerySearch and search_cases from src/controller.rs.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 46a52e6f-cae1-485a-bf7c-b01cdb912ca1
📒 Files selected for processing (2)
src/controller.rsstatic/help.txt
🚧 Files skipped from review as they are similar to previous changes (1)
- src/controller.rs
| 可选参数： | ||
| - search_type: keyword（默认关键词搜索）/ vsearch（语义搜索，刑事案件可用） | ||
| - offset: 偏移量，默认0 |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Document the supported export parameter.
QuerySearch accepts export, and search_cases uses it to change the API result limit. Add it to this parameter list, or explicitly state that it is unsupported for /api/search, so the documentation matches the shared handler contract in src/controller.rs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@static/help.txt` around lines 108 - 110, Update the optional-parameter list
in help.txt to document the supported export parameter used by QuerySearch and
search_cases, including its effect on the API result limit; if /api/search does
not support it, explicitly state that instead. Align the wording with the shared
handler contract in QuerySearch and search_cases from src/controller.rs.
1 participant
Summary by CodeRabbit
/api/search.search_metaplus a list of full matching case details, including support for keyword search and (when enabled) vector search.search_metaconsistently for inputs, suggestions, results rendering, pagination, and export behavior.static/help.txtwith revised export examples and added/api/searchusage details, includingsearch_typeandoffsetguidance.