Skip to content

feat(frontend): KB full-text search result viewer#3920

Merged
mrveiss merged 1 commit intoDev_new_guifrom
issue-3296
Apr 7, 2026
Merged

feat(frontend): KB full-text search result viewer#3920
mrveiss merged 1 commit intoDev_new_guifrom
issue-3296

Conversation

@mrveiss
Copy link
Copy Markdown
Owner

@mrveiss mrveiss commented Apr 7, 2026

Summary

  • Creates KBSearchResultPanel.vue — a split-pane component with a ranked results list (left) and inline read-only document viewer (right)
  • Replaces the old flat results list and BaseModal document viewer in KnowledgeSearch.vue with the new panel
  • Keyboard navigation: ArrowDown/ArrowUp move selection, Enter emits select, Escape emits close
  • Query-term highlighting in both snippets and full document content via HTML-escaped <mark class="kb-highlight"> spans
  • Loading skeleton shown while loading prop is true; document fetched via KnowledgeRepository.getDocument() if content is truncated

Test plan

  • Run a search and verify ranked results appear with title, relevance %, and highlighted snippet
  • Click or arrow-key to a result and verify full document content loads in the right pane
  • Verify query terms are highlighted (yellow mark) in both snippet and full content
  • Press Escape and verify the panel closes (search state resets)
  • Verify console.* is absent — all logging via createLogger('KBSearchResultPanel')

Closes #3296

🤖 Generated with Claude Code

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@mrveiss
Copy link
Copy Markdown
Owner Author

mrveiss commented Apr 7, 2026

Code review

Found 2 issues:

  1. Unnecessary as any cast bypasses TypeScript type safetygetDocument() returns Promise<KnowledgeDocument> and KnowledgeDocument.content is typed as string (non-optional). The cast (full as any)?.content silently removes compile-time checks for no reason. Drop the cast: viewerContent.value = full.content ?? document.content ?? ''

  1. KnowledgeRepository duplicated in child component (CLAUDE.md says "Reuse Existing Code — import and call existing utilities; never copy-paste") — KBSearchResultPanel instantiates its own new KnowledgeRepository() at line 226 while the parent KnowledgeSearch.vue already owns an identical instance at its line 345. Both will fire independent HTTP requests for the same document. The correct pattern is to accept the already-fetched content as a prop or bubble a fetch event to the parent to reuse its existing repo instance.

const activeResult = computed<SearchResult | null>(() =>

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

@mrveiss mrveiss merged commit 4af4e78 into Dev_new_gui Apr 7, 2026
1 of 3 checks passed
@mrveiss mrveiss deleted the issue-3296 branch April 7, 2026 21:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant