fix(history): 继承失败原因源批次支持远程子串搜索 - #15
Merged
Merged
Conversation
默认仍返回最近 100 批；输入关键词后按 start_time 全库检索，避免更早轮次搜不到。 Co-authored-by: Cursor <cursoragent@cursor.com>
📝 WalkthroughWalkthroughThe inherit-batch options flow now supports an optional ChangesRemote batch option search
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant HistoryPage
participant historyApi
participant historyEndpoint
participant get_inherit_batch_options
HistoryPage->>HistoryPage: Debounce source-batch input
HistoryPage->>historyApi: Request options with currentBatch and q
historyApi->>historyEndpoint: GET /history/inherit-batch-options
historyEndpoint->>get_inherit_batch_options: Apply exclusion and search
get_inherit_batch_options-->>historyEndpoint: Return batch options
historyEndpoint-->>historyApi: Return response
historyApi-->>HistoryPage: Update source-batch dropdown
🚥 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: 2
🧹 Nitpick comments (1)
backend/services/inherit_failure_reason_service.py (1)
75-98: 🚀 Performance & Scalability | 🔵 TrivialValidate the cost of full-library substring search.
The pattern begins with
%, so a normal B-tree index onstart_timecannot efficiently support this lookup. Combined with debounced frontend requests, short queries may repeatedly scan and sort a large history table. RunEXPLAINon production-sized data and consider a minimum query length or an appropriate substring-search index if needed.🤖 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 `@backend/services/inherit_failure_reason_service.py` around lines 75 - 98, Validate the q-filter path in get_inherit_batch_options with EXPLAIN against production-sized data, focusing on the leading-wildcard start_time substring search and its scan/sort cost. Based on the measured impact, either enforce an appropriate minimum q length before executing the search or add/use a suitable substring-search index, while preserving the existing filtering, ordering, distinctness, and result limit.
🤖 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 `@frontend/src/pages/history/HistoryPage.tsx`:
- Around line 318-319: Update the inherit-batch search flow around
inheritBatchSearchTimerRef to clear any pending timer during component unmount
cleanup and whenever the modal closes after successful confirmation, not only in
the cancel handler. Preserve the existing debounce behavior while preventing
callbacks from issuing requests or updating state after closure or unmount.
- Around line 960-983: Update fetchInheritBatchOptions and
handleInheritBatchSearch to track a request generation (or abort the previous
request) so stale remote responses cannot update inheritBatchOptions or loading
state after a newer search begins. Guard both successful and error/finally state
updates, while preserving the existing debounce and error-display behavior for
the latest request.
---
Nitpick comments:
In `@backend/services/inherit_failure_reason_service.py`:
- Around line 75-98: Validate the q-filter path in get_inherit_batch_options
with EXPLAIN against production-sized data, focusing on the leading-wildcard
start_time substring search and its scan/sort cost. Based on the measured
impact, either enforce an appropriate minimum q length before executing the
search or add/use a suitable substring-search index, while preserving the
existing filtering, ordering, distinctness, and result limit.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 61e314b3-b8da-49a1-b0d4-58a3302bc0ce
📒 Files selected for processing (5)
backend/api/v1/history.pybackend/services/inherit_failure_reason_service.pyfrontend/src/pages/history/HistoryPage.tsxfrontend/src/services/index.tsspec/10_inherit_failure_reason_spec.md
Comment on lines
+318
to
+319
| /** 源批次远程搜索防抖 */ | ||
| const inheritBatchSearchTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Clean up pending search timers on every close path.
The timer is cleared only by the cancel handler. If the user confirms or navigates away before 300 ms, the callback still performs a request and may update state for a closed or unmounted page. Add unmount cleanup and clear the timer when the modal closes successfully as well.
Also applies to: 976-983
🤖 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 `@frontend/src/pages/history/HistoryPage.tsx` around lines 318 - 319, Update
the inherit-batch search flow around inheritBatchSearchTimerRef to clear any
pending timer during component unmount cleanup and whenever the modal closes
after successful confirmation, not only in the cancel handler. Preserve the
existing debounce behavior while preventing callbacks from issuing requests or
updating state after closure or unmount.
Comment on lines
+960
to
+983
| const fetchInheritBatchOptions = async (q?: string, showError = false) => { | ||
| setInheritBatchOptionsLoading(true); | ||
| try { | ||
| const res = await historyApi.inheritBatchOptions(currentBatch, q); | ||
| setInheritBatchOptions(res.batches ?? []); | ||
| } catch (e: unknown) { | ||
| setInheritBatchOptions([]); | ||
| if (showError) { | ||
| const err = e as { response?: { data?: { detail?: string } }; message?: string }; | ||
| message.error(err?.response?.data?.detail || err?.message || "获取批次选项失败"); | ||
| } | ||
| } finally { | ||
| setInheritBatchOptionsLoading(false); | ||
| } | ||
| }; | ||
|
|
||
| const handleInheritBatchSearch = (input: string) => { | ||
| if (inheritBatchSearchTimerRef.current) { | ||
| clearTimeout(inheritBatchSearchTimerRef.current); | ||
| } | ||
| inheritBatchSearchTimerRef.current = setTimeout(() => { | ||
| const trimmed = input.trim(); | ||
| void fetchInheritBatchOptions(trimmed || undefined); | ||
| }, 300); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Ignore stale remote-search responses.
Debouncing does not cancel requests already in flight. If an older query resolves after a newer query, its results overwrite the current options; with filterOption={false}, users can see or select batches unrelated to the current search.
Track a request generation or abort the previous request, guarding both options and loading-state updates.
Proposed request-generation guard
+ const inheritBatchRequestIdRef = useRef(0);
+
const fetchInheritBatchOptions = async (q?: string, showError = false) => {
+ const requestId = ++inheritBatchRequestIdRef.current;
setInheritBatchOptionsLoading(true);
try {
const res = await historyApi.inheritBatchOptions(currentBatch, q);
+ if (requestId !== inheritBatchRequestIdRef.current) return;
setInheritBatchOptions(res.batches ?? []);
} catch (e: unknown) {
+ if (requestId !== inheritBatchRequestIdRef.current) return;
setInheritBatchOptions([]);
// existing error handling
} finally {
- setInheritBatchOptionsLoading(false);
+ if (requestId === inheritBatchRequestIdRef.current) {
+ setInheritBatchOptionsLoading(false);
+ }
}
};📝 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
| const fetchInheritBatchOptions = async (q?: string, showError = false) => { | |
| setInheritBatchOptionsLoading(true); | |
| try { | |
| const res = await historyApi.inheritBatchOptions(currentBatch, q); | |
| setInheritBatchOptions(res.batches ?? []); | |
| } catch (e: unknown) { | |
| setInheritBatchOptions([]); | |
| if (showError) { | |
| const err = e as { response?: { data?: { detail?: string } }; message?: string }; | |
| message.error(err?.response?.data?.detail || err?.message || "获取批次选项失败"); | |
| } | |
| } finally { | |
| setInheritBatchOptionsLoading(false); | |
| } | |
| }; | |
| const handleInheritBatchSearch = (input: string) => { | |
| if (inheritBatchSearchTimerRef.current) { | |
| clearTimeout(inheritBatchSearchTimerRef.current); | |
| } | |
| inheritBatchSearchTimerRef.current = setTimeout(() => { | |
| const trimmed = input.trim(); | |
| void fetchInheritBatchOptions(trimmed || undefined); | |
| }, 300); | |
| const inheritBatchRequestIdRef = useRef(0); | |
| const fetchInheritBatchOptions = async (q?: string, showError = false) => { | |
| const requestId = ++inheritBatchRequestIdRef.current; | |
| setInheritBatchOptionsLoading(true); | |
| try { | |
| const res = await historyApi.inheritBatchOptions(currentBatch, q); | |
| if (requestId !== inheritBatchRequestIdRef.current) return; | |
| setInheritBatchOptions(res.batches ?? []); | |
| } catch (e: unknown) { | |
| if (requestId !== inheritBatchRequestIdRef.current) return; | |
| setInheritBatchOptions([]); | |
| if (showError) { | |
| const err = e as { response?: { data?: { detail?: string } }; message?: string }; | |
| message.error(err?.response?.data?.detail || err?.message || "获取批次选项失败"); | |
| } | |
| } finally { | |
| if (requestId === inheritBatchRequestIdRef.current) { | |
| setInheritBatchOptionsLoading(false); | |
| } | |
| } | |
| }; | |
| const handleInheritBatchSearch = (input: string) => { | |
| if (inheritBatchSearchTimerRef.current) { | |
| clearTimeout(inheritBatchSearchTimerRef.current); | |
| } | |
| inheritBatchSearchTimerRef.current = setTimeout(() => { | |
| const trimmed = input.trim(); | |
| void fetchInheritBatchOptions(trimmed || undefined); | |
| }, 300); |
🤖 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 `@frontend/src/pages/history/HistoryPage.tsx` around lines 960 - 983, Update
fetchInheritBatchOptions and handleInheritBatchSearch to track a request
generation (or abort the previous request) so stale remote responses cannot
update inheritBatchOptions or loading state after a newer search begins. Guard
both successful and error/finally state updates, while preserving the existing
debounce and error-display behavior for the latest request.
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
2 participants
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.
默认仍返回最近 100 批；输入关键词后按 start_time 全库检索，避免更早轮次搜不到。
Summary by CodeRabbit