Repository navigation
fix(search): open matched records and deduplicate sources - #1345
Merged
lfnovo merged 3 commits intoSep 11, 2026
Merged
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 4 files
Confidence score: 3/5
- In
frontend/src/components/sources/AddExistingSourceDialog.tsx,fn::text_searchassigns an insight hit’sparent_idto the insight ID rather than its owning source ID, so source and insight results are not grouped and both can remain visible; use the owning source ID for grouping.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="frontend/src/components/sources/AddExistingSourceDialog.tsx">
<violation number="1" location="frontend/src/components/sources/AddExistingSourceDialog.tsx:96">
P2: For this dialog's text search, `fn::text_search` sets an insight's `parent_id` to the insight ID, not its owning source ID. `sourceIds` therefore does not group a source hit with its insight, so both entries still render; fix the text-search parent ID contract or deduplicate using the owning source relation before relying on this set.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| const sources = response.results.map(r => ({ | ||
| const sourceIds = new Set<string>() | ||
| const sources = response.results.filter(r => { | ||
| if (sourceIds.has(r.parent_id)) return false |
Contributor
There was a problem hiding this comment.
P2: For this dialog's text search, fn::text_search sets an insight's parent_id to the insight ID, not its owning source ID. sourceIds therefore does not group a source hit with its insight, so both entries still render; fix the text-search parent ID contract or deduplicate using the owning source relation before relying on this set.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/components/sources/AddExistingSourceDialog.tsx, line 96:
<comment>For this dialog's text search, `fn::text_search` sets an insight's `parent_id` to the insight ID, not its owning source ID. `sourceIds` therefore does not group a source hit with its insight, so both entries still render; fix the text-search parent ID contract or deduplicate using the owning source relation before relying on this set.</comment>
<file context>
@@ -91,9 +91,12 @@ export function AddExistingSourceDialog({
- const sources = response.results.map(r => ({
+ const sourceIds = new Set<string>()
+ const sources = response.results.filter(r => {
+ if (sourceIds.has(r.parent_id)) return false
+ sourceIds.add(r.parent_id)
+ return true
</file context>
Contributor
Author
There was a problem hiding this comment.
This PR is intentionally the frontend follow-up to #1339. Current main still returns the insight ID in parent_id; #1339 migration 25 changes it to source.id, which is the contract used by this Set and regression. Duplicating that migration here would conflict with the contributors open PR, so the required merge order is #1339 followed by #1345.
lfnovo
enabled auto-merge (squash)
September 11, 2026 00:05
Merged
3 of 4 tasks
HariHaranDFX
added a commit
to HariHaranDFX/open-notebook
that referenced
this pull request
Sep 18, 2026
Backports the intent of upstream lfnovo#1345 (ae7b96d) to our WP3 search components. ## Two independent fixes ### 1. Search rows open the record they matched on `SearchResultRow` parsed the row's target from `result.parent_id` — so a hit on a source insight (id=source_insight:X, parent_id=source:Y) opened the parent SOURCE preview instead of the insight itself. Users who searched for text inside an insight got the wrong pane. Renamed `parseParentId` -> `parseResultTarget` (parseParentId kept as an alias for a soft migration) and switched `SearchResultRow` and `SearchWorkspace.isActive` to use `result.id`. Source-body chunks already have id == parent_id, so the previous UX for source hits is unchanged. ### 2. AddExistingSourceDialog dedupes by parent_id The search returns one row per matching CHUNK, so a source that matches by title + full-text + insight would render 3 duplicate rows in the picker. Added a `Set<parent_id>` filter so each source appears once. ## Verification - New unit test: `SearchWorkspace > opens an insight match into the insight preview, not the parent source (upstream lfnovo#1345)` — asserts openPreview is called with (`source_insight`, `i-1`), NOT (`source`, `s-1`). - New unit test: `AddExistingSourceDialog > dedupes search results by parent_id (upstream lfnovo#1345)` — feeds 4 rows sharing 2 parent_ids and asserts 2 unique visible rows. - Full frontend suite: **102 files / 586 tests / 0 failures** (+2 net new tests vs prior 584). - `npm run lint`: 0 errors (pre-existing warnings unchanged).
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
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.
Description
Context
Search results expose two identities:
idis the record that matched, whileparent_idis the source that owns it. The search page usedparent_idfor navigation, so an insight hit opened its parent source instead of the matched insight. After #1339 makes text and vector search agree on parent identity, the add-existing-source dialog can also receive several hits for the same source.Changes
result.id.parent_idbefore rendering sources in the add-existing dialog.User impact
Selecting an insight search result now opens that insight directly. Searching for an existing source shows each source once, even when both the source and one of its insights match.
Related Issue
Fixes #1341
Type of Change
Verification
Base-red checks on the untouched
mainrevision reproduced both regressions: the search-page test receivedsource / parent-source, and the dialog test rendered both rows with a duplicate React key.npm test— 28 files passed, 158 tests passed.npm run lint— 0 errors; 7 existing warnings in unchanged files.npm run build— production build completed successfully, including TypeScript and all 15 static pages.git diff --check— passed.Design Alignment
The frontend now uses the two existing response fields for their defined roles and deduplicates with a local
Set; no API, state-management, or dependency change is needed.Checklist
Code Quality
Testing
Documentation
Database Changes
Breaking Changes
Screenshots
Pre-Submission Verification
CONTRIBUTING.md, the frontend rules, andVISION.md.