feat(search): add search functionality#31
Conversation
|
Warning Rate limit exceeded@perfectra1n has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 6 minutes and 9 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughAdds a client-side in-page search feature (index and transcript modes) with templates, styles, and scripts; refactors the filter toolbar CSS to a flex layout; integrates a combined Search & Filter toolbar into transcripts and injects search UI into the index. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Page as Index Page
participant SearchUI as Search Component
participant Store as localStorage
Note over Page,SearchUI: Initialization (Index mode)
Page->>SearchUI: DOM ready → init()
SearchUI->>Store: load saved state
SearchUI->>SearchUI: build index (projects → sessions)
SearchUI-->>User: render input, options, results panel
Note over User,SearchUI: Searching
User->>SearchUI: type query / toggle options
SearchUI->>SearchUI: debounce → compile safe regex
SearchUI->>SearchUI: search index → highlight matches
SearchUI->>Store: persist state
SearchUI-->>User: update counts, enable nav
alt Navigate results
User->>SearchUI: next/prev
SearchUI->>SearchUI: move current highlight
end
alt Close panel
User->>SearchUI: Esc / close
SearchUI-->>User: hide results panel
end
sequenceDiagram
autonumber
actor User
participant Page as Transcript Page
participant Toolbar as Search & Filter Toolbar
participant SearchUI as Inline Search
participant Observer as MutationObserver
User->>Toolbar: toggle "Search & Filter"
Toolbar->>SearchUI: focus search input
Toolbar-->>User: show toolbar with toggles
Note over SearchUI,Observer: Transcript indexing
SearchUI->>SearchUI: build index (messages)
Observer->>SearchUI: notify on DOM/filter changes
SearchUI->>SearchUI: re-index visible items
User->>SearchUI: enter query / change options
SearchUI->>SearchUI: debounce → search → highlight
SearchUI-->>User: update counts, nav state
User->>Toolbar: close toolbar
Toolbar-->>User: hide UI (state persisted)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
claude_code_log/templates/components/filter_styles.css(1 hunks)claude_code_log/templates/components/search.html(1 hunks)claude_code_log/templates/components/search_inline.html(1 hunks)claude_code_log/templates/components/search_inline_script.html(1 hunks)claude_code_log/templates/components/search_results_panel.html(1 hunks)claude_code_log/templates/components/search_styles.css(1 hunks)claude_code_log/templates/index.html(2 hunks)claude_code_log/templates/transcript.html(6 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
claude_code_log/templates/**/*.html
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain Jinja2 HTML templates under claude_code_log/templates/ (e.g., transcript.html, index.html, components/)
Files:
claude_code_log/templates/components/search_inline_script.htmlclaude_code_log/templates/components/search_inline.htmlclaude_code_log/templates/components/search.htmlclaude_code_log/templates/transcript.htmlclaude_code_log/templates/components/search_results_panel.htmlclaude_code_log/templates/index.html
| <script> | ||
| {% include 'components/search.html' %} | ||
| </script> No newline at end of file |
There was a problem hiding this comment.
Remove the wrapper <script>; this include produces invalid JS.
search.html already emits its own <script> block (and, unless is_transcript is set, also emits markup). Wrapping that include in another <script> yields nested <script> tags and raw HTML inside a script, breaking execution the moment this template is used. Drop the outer wrapper and include search.html directly after setting is_transcript = True, or split the logic so only the IIFE is imported here.
-<script>
-{% include 'components/search.html' %}
-</script>
+{% set is_transcript = True %}
+{% include 'components/search.html' %}📝 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.
| <script> | |
| {% include 'components/search.html' %} | |
| </script> | |
| {% set is_transcript = True %} | |
| {% include 'components/search.html' %} |
🤖 Prompt for AI Agents
In claude_code_log/templates/components/search_inline_script.html around lines 1
to 3, the include is wrapped in a <script> tag which causes nested <script> tags
and invalid JS because components/search.html already emits its own <script>
(and markup unless is_transcript is set); remove the outer <script> wrapper and
either set is_transcript = True before including search.html so it emits only
the inline script, or change the include to import only the IIFE/JS portion from
a split template that contains no surrounding script/markup.
|
Hey @perfectra1n, thanks a lot for this! I'll check it out and give it a try later today, but if you're around, can you please rebase to resolve the conflicts (I just merged another PR) and see if the |
perfectra1n
force-pushed
the
feat/add-global-search-functionality
branch
from
2433434 to
dc3bf19
Compare
Improve spacing and layout in the search toolbar to prevent visual overlap between the search input clear button (✕) and the toolbar close button (✕).
perfectra1n
force-pushed
the
feat/add-global-search-functionality
branch
from
dc3bf19 to
bf745c2
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
claude_code_log/templates/components/search.html (2)
106-154: Case-sensitive search is broken due to pre-lowercased index.Every
searchTextentry is stored lowercased (lines 112, 131, 151), but the case-sensitive matching logic at line 350 attempts to compare this pre-lowercased text directly against the user's query without normalization. This means case-sensitive searches can never match correctly.Apply the fix suggested in the previous review:
- searchText: `${projectName} ${projectStats}`.toLowerCase(), + searchText: `${projectName} ${projectStats}`, originalText: `${projectName} ${projectStats}`- searchText: `${projectName} ${sessionText} ${sessionPreview} ${sessionMeta}`.toLowerCase(), + searchText: `${projectName} ${sessionText} ${sessionPreview} ${sessionMeta}`, originalText: `${sessionText} ${sessionPreview}`- searchText: `${header} ${content}`.toLowerCase(), + searchText: `${header} ${content}`, originalText: `${header} ${content}`
338-373: Regex search skips matches due to stale lastIndex.The regex pattern is created once with the global flag (line 325), then
.test()is called on it repeatedly in the loop (line 353). Because global regexes maintainlastIndexstate, subsequent items are searched starting from where the previous item left off, causing early matches to be missed.Apply the fix suggested in the previous review to reset
lastIndexbefore each test:const searchText = searchState.caseSensitive ? item.searchText : item.searchText.toLowerCase(); const searchQuery = searchState.caseSensitive ? query : query.toLowerCase(); - if (searchState.useRegex ? searchPattern.test(searchText) : searchText.includes(searchQuery)) { + let matchesItem = false; + if (searchState.useRegex) { + searchPattern.lastIndex = 0; + matchesItem = searchPattern.test(searchText); + } else { + matchesItem = searchText.includes(searchQuery); + } + + if (matchesItem) {
🧹 Nitpick comments (1)
test/test_template_rendering.py (1)
325-326: Consider a more targeted escaping verification.The test now permits up to 2
<script>tags to accommodate legitimate template scripts, but this couples user-content escaping verification to template structure. If a third script is added to templates, this test breaks even though escaping may still be correct.Instead, verify that the specific malicious content from the test data is escaped:
# Check that HTML is escaped assert "<script>" in html_content assert "&" in html_content assert """ in html_content # Should not contain unescaped HTML - assert ( - "<script>" not in html_content or html_content.count("<script>") <= 2 - ) # Allow for the markdown script and search script + # Verify the specific malicious content is escaped (not just counting all scripts) + assert "alert('xss')" not in html_content or "<script>alert('xss')</script>" in html_content
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
claude_code_log/templates/components/filter_styles.css(1 hunks)claude_code_log/templates/components/search.html(1 hunks)claude_code_log/templates/components/search_inline.html(1 hunks)claude_code_log/templates/components/search_inline_script.html(1 hunks)claude_code_log/templates/components/search_results_panel.html(1 hunks)claude_code_log/templates/components/search_styles.css(1 hunks)claude_code_log/templates/index.html(2 hunks)claude_code_log/templates/transcript.html(6 hunks)scripts/style_guide_output/index_style_guide.html(9 hunks)test/test_template_rendering.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- claude_code_log/templates/components/search_inline.html
- claude_code_log/templates/components/search_inline_script.html
- claude_code_log/templates/components/search_results_panel.html
- claude_code_log/templates/index.html
- claude_code_log/templates/components/search_styles.css
🧰 Additional context used
📓 Path-based instructions (3)
test/**/{test_*.py,*_test.py}
📄 CodeRabbit inference engine (CLAUDE.md)
test/**/{test_*.py,*_test.py}: Mark Textual TUI tests with @pytest.mark.tui to isolate their async event loop
Mark Playwright browser tests with @pytest.mark.browser to avoid event-loop conflicts
Files:
test/test_template_rendering.py
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Format Python code with ruff format
Lint and auto-fix with ruff check --fix
Perform static type checking with pyright (uv run pyright)
Files:
test/test_template_rendering.py
claude_code_log/templates/**/*.html
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain Jinja2 HTML templates under claude_code_log/templates/ (e.g., transcript.html, index.html, components/)
Files:
claude_code_log/templates/transcript.htmlclaude_code_log/templates/components/search.html
🔇 Additional comments (4)
claude_code_log/templates/components/filter_styles.css (1)
19-78: LGTM! Well-structured CSS refactoring.The migration from grid to flex-based layout is appropriate for this vertical toolbar, and the new components (toolbar header, close button, filter section) are cleanly styled with good accessibility considerations.
claude_code_log/templates/transcript.html (1)
27-59: LGTM! Clean integration of search into transcript toolbar.The combined Search & Filter toolbar is well-structured with proper semantic markup, event wiring, and UX considerations like auto-focusing the search input.
Also applies to: 103-103, 115-115, 171-171, 175-187, 316-318
scripts/style_guide_output/index_style_guide.html (1)
1-1661: Generated style guide demonstrates search feature integration.This generated output file shows the search functionality properly integrated into the index page with inlined styles and scripts. As a generated demonstration file, detailed line-by-line review is not necessary.
claude_code_log/templates/components/search.html (1)
705-736: LGTM! Proper re-indexing when filters change.The MutationObserver correctly watches for filter changes on transcript pages and debounces re-indexing appropriately.
The Ctrl+F handler was trying to open a filter toolbar that only exists on transcript pages, causing it to fail silently on the index page.
|
Hey @daaain, Before typing: After typing: and fixed the merge conflicts :) |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
claude_code_log/templates/components/search.html (1)
603-611: Regex creation in highlightText lacks error handling.The
highlightTextfunction creates a fresh regex (lines 606–608) but doesn't wrap it in try-catch. IfsearchState.useRegexis true and the user enters an invalid regex, line 607 will throw and crash the results rendering. Since validation already occurs inperformSearch(line 352–357), this should be safe in practice, but adding a guard would be defensive:try { const pattern = searchState.useRegex ? new RegExp(query, searchState.caseSensitive ? 'g' : 'gi') : new RegExp(query.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'), searchState.caseSensitive ? 'g' : 'gi'); return text.replace(pattern, match => `<span class="search-highlight">${match}</span>`); } catch (e) { return text; // Fallback: return unmodified text }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
claude_code_log/templates/components/search.html(1 hunks)claude_code_log/templates/components/search_styles.css(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
claude_code_log/templates/**/*.html
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain Jinja2 HTML templates under claude_code_log/templates/ (e.g., transcript.html, index.html, components/)
Files:
claude_code_log/templates/components/search.html
🔇 Additional comments (3)
claude_code_log/templates/components/search_styles.css (1)
1-371: CSS styling structure looks good. The search component styles are well-organized with clear sectioning, proper state management (.visible,.hidden,.current), and responsive adjustments. No critical issues identified.claude_code_log/templates/components/search.html (2)
665-670: ✓ Null-check for keyboard shortcut fix is in place.The Ctrl+F handler now safely checks if
filterToolbarexists (line 666) before accessing.classList. This resolves the crash that would occur on index pages. Good catch in the prior review.
422-473: Verify regex reuse in highlightInElement doesn't have lastIndex issues.The
highlightInElementfunction receivessearchPatternwhich may have already been used in the matching loop. WhilematchAllshould reset the regex internally, confirm that stalelastIndexstate isn't affecting highlighting. Consider passing a freshly created regex or resettingpattern.lastIndex = 0at line 440.
When using a global RegExp with .test(), the regex maintains an internal lastIndex that advances with each call. This caused the search to skip valid matches in items after the first one. Fixed by resetting searchPattern.lastIndex = 0 before each .test() call to ensure each item is searched from the beginning.
perfectra1n
force-pushed
the
feat/add-global-search-functionality
branch
from
425683b to
6ac8f66
Compare
…se-insensitive Removed the case-sensitive search option as it's rarely useful for conversation search and was adding unnecessary complexity. Changes: - Removed "Case sensitive" checkbox from both index and transcript search UIs - Removed caseSensitive from searchState - Simplified all regex patterns to always use 'gi' flag (case-insensitive) - Simplified search logic to always lowercase both query and searchText - Removed caseSensitive from localStorage save/restore - Removed caseSensitive event listener and DOM reference
perfectra1n
force-pushed
the
feat/add-global-search-functionality
branch
from
103fad0 to
b0c1ae3
Compare
|
Great, thanks a lot ✨ |
|
This is now released in https://github.com/daaain/claude-code-log/releases/tag/0.7.0 |
2 participants
let me know if you like/dislike anything about it, or if you're even interested in something like this. here's a quick screenshot:
Summary by CodeRabbit
New Features
Style
Tests