Fix search broken when HTML saved with different filename#106
Fix search broken when HTML saved with different filename#106cboos merged 2 commits intodaaain:mainfrom
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughSet page-type detection to runtime: Changes
Sequence Diagram(s)sequenceDiagram
participant Init as initSearch()
participant DOM as Document
participant State as searchState
participant Index as IndexBuilder/Results
participant Observer as setupTranscriptFilterObserver()
Init->>DOM: querySelector('.project-list')
DOM-->>Init: element or null
Init->>State: set isIndexPage = (found)
alt isIndexPage == true
Init->>Index: build index / initialize search UI
Index-->>State: index ready
else isIndexPage == false
Init->>Observer: call setupTranscriptFilterObserver()
Observer->>Observer: attach MutationObserver (filters/classes)
Observer->>Index: trigger rebuild index + performSearch() on filter changes
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 the current code and only fix it if needed.
Inline comments:
In `@claude_code_log/html/templates/components/search.html`:
- Line 57: The isIndexPage detection currently uses
document.querySelectorAll('.project-card').length > 0 which fails when the index
contains zero projects; update the logic in the code that sets isIndexPage to
check for a stable container marker (for example
document.querySelector('.project-list') !== null) or combine both checks (e.g.,
document.querySelector('.project-list') !== null ||
document.querySelectorAll('.project-card').length > 0) so an empty index still
returns true; modify the assignment where isIndexPage is defined to use the new
combined/stable check.
🪄 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
Run ID: e869cd67-b737-4481-8a62-105f0e599a0a
📒 Files selected for processing (1)
claude_code_log/html/templates/components/search.html
moshethebeadle
force-pushed
the
fix/isIndexPage-detection
branch
from
af47fa1 to
5ac133d
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
claude_code_log/html/templates/components/search.html (1)
77-79:⚠️ Potential issue | 🟠 Major
.project-card-only detection still breaks empty index pagesLine 78 still treats an empty index page as non-index, so search falls into transcript logic when there are zero projects. Use a stable marker that exists even when the project list is empty.
Proposed fix
- searchState.isIndexPage = document.querySelectorAll('.project-card').length > 0; + // Works for both populated and empty index pages + searchState.isIndexPage = !!document.querySelector('.project-list, .project-card');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@claude_code_log/html/templates/components/search.html` around lines 77 - 79, The detection using document.querySelectorAll('.project-card') fails on empty index pages; update the assignment to searchState.isIndexPage in the search.html snippet to check for a stable container or marker that exists even when there are zero projects (e.g., a wrapper element like '.project-list' or an ID/data-attribute added to the index template) instead of '.project-card' so empty indexes are still recognized as index pages; modify the selector used in the searchState.isIndexPage line accordingly (replace '.project-card' with the stable marker you have in the templates).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@claude_code_log/html/templates/components/search.html`:
- Line 57: The pre-init check runs because searchState.isIndexPage is
initialized to null, so the top-level condition if (!searchState.isIndexPage)
triggers before initSearch() sets the real value; change the initialization of
isIndexPage from null to undefined (or remove the explicit null) and update the
top-level check to explicitly test for false (e.g., if (searchState.isIndexPage
=== false)) so the transcript-only observers only register when the page is
known to be a non-index page; adjust references to isIndexPage, the searchState
object, and the initSearch() flow accordingly.
---
Duplicate comments:
In `@claude_code_log/html/templates/components/search.html`:
- Around line 77-79: The detection using
document.querySelectorAll('.project-card') fails on empty index pages; update
the assignment to searchState.isIndexPage in the search.html snippet to check
for a stable container or marker that exists even when there are zero projects
(e.g., a wrapper element like '.project-list' or an ID/data-attribute added to
the index template) instead of '.project-card' so empty indexes are still
recognized as index pages; modify the selector used in the
searchState.isIndexPage line accordingly (replace '.project-card' with the
stable marker you have in the templates).
🪄 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
Run ID: 9b746aaf-7d96-4cef-8b84-2444b171f724
📒 Files selected for processing (1)
claude_code_log/html/templates/components/search.html
The isIndexPage detection relied on window.location.pathname matching specific patterns (index.html, projects/, etc.). When the generated HTML is downloaded or saved with a different filename, the detection fails and search indexes zero items, breaking search entirely. Replace URL-based detection with DOM-based detection by checking for the .project-list container element. The check is deferred to initSearch() (which runs on DOMContentLoaded) so the DOM elements exist by the time it executes. Using .project-list rather than .project-card ensures it works even when the index has zero projects. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
moshethebeadle
force-pushed
the
fix/isIndexPage-detection
branch
from
5ac133d to
d1e814e
Compare
|
Hey @moshethebeadle, thanks a lot for this fix! I had a look and all the other usage of I approved the tests and the snapshots will need to be updated once you're ready to get them green. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/__snapshots__/test_snapshot_html.ambr`:
- Around line 10928-10937: The snapshot contains corrupted/truncated JavaScript
in the ancestor filtering logic: restore the truncated method call on classList
to use cls.startsWith('d-') when building ancestorIds and remove the duplicated
lines so the filter callback returns lastAncestor === targetId exactly once;
specifically fix the code around the ancestorIds computation and the isFolded
check (refer to symbols classList, cls.startsWith, ancestorIds, lastAncestor,
targetId, isFolded) to match the original template logic from transcript.html so
the snapshot parses correctly.
- Around line 7300-7305: The snapshot test file test_snapshot_html.ambr contains
corrupted CSS around the .system-error rule (unclosed comments and stray brace);
regenerate the failing snapshot so it matches the source template's correct CSS
(ensure the comment blocks are closed and the .system-error rule is intact as in
the template), then run the snapshot update command (e.g., the test runner's
snapshot update) to replace the corrupted lines with the correct content.
🪄 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
Run ID: d2f762f3-bf6a-4101-b82c-4417bd67d353
📒 Files selected for processing (2)
claude_code_log/html/templates/components/search.htmltest/__snapshots__/test_snapshot_html.ambr
🚧 Files skipped from review as they are similar to previous changes (1)
- claude_code_log/html/templates/components/search.html
| /* System error messages } | ||
|
|
||
| /* Left-aligned m .system-error { | ||
| margin-left: 0; | ||
| margin-right: 8em; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the source template for the .system-error CSS to verify it's not corrupted there
rg -n -A3 -B3 'system-error' --type htmlRepository: daaain/claude-code-log
Length of output: 6076
🏁 Script executed:
sed -n '7295,7310p' test/__snapshots__/test_snapshot_html.ambrRepository: daaain/claude-code-log
Length of output: 454
Regenerate snapshot - corrupted CSS content
The snapshot file contains malformed CSS at these lines. Source templates show correct content (e.g., line 426 in transcript_style_guide.html has properly formatted .system-error rule with closed comment), but the snapshot captured corrupted/truncated versions:
- Line 7300: Unclosed comment
/* System error messages }with stray} - Line 7302: Truncated comment text
/* Left-aligned mfollowed by.system-error {
Regenerate this snapshot to fix the corruption.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/__snapshots__/test_snapshot_html.ambr` around lines 7300 - 7305, The
snapshot test file test_snapshot_html.ambr contains corrupted CSS around the
.system-error rule (unclosed comments and stray brace); regenerate the failing
snapshot so it matches the source template's correct CSS (ensure the comment
blocks are closed and the .system-error rule is intact as in the template), then
run the snapshot update command (e.g., the test runner's snapshot update) to
replace the corrupted lines with the correct content.
The `if (!searchState.isIndexPage)` block ran at parse time when isIndexPage was still null, causing transcript-only MutationObservers to register on index pages too. Extract the observer setup into setupTranscriptFilterObserver() and call it from initSearch() after isIndexPage is properly determined. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
cboos
force-pushed
the
fix/isIndexPage-detection
branch
from
be1e932 to
e92f2c5
Compare
|
I apologize for the confusion with Git. I didn't intend to push to the branch I tested everything locally, and it appears to be working fine. |
3 participants
Summary
isIndexPagedetection in search relied onwindow.location.pathnamematching specific URL patterns (index.html,projects/, etc.)document.querySelectorAll('.project-card').length > 0), which correctly identifies the page type regardless of filename or serving contextTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Style