feat(search): D1 FTS5 search over videos with header search box#23
Conversation
|
Caution Review failedPull request was closed or merged during review WalkthroughThis pull request introduces full-text search functionality across the entire application stack. A database migration creates an FTS5 virtual table for videos with automatic synchronization triggers, the frontend adds a header search form and dedicated search results page, a new API endpoint handles FTS queries with pagination, and supporting tests and styling are included. Changes
Sequence DiagramsequenceDiagram
participant User
participant Browser as Frontend (Browser)
participant API as API Worker
participant DB as Database (D1)
User->>Browser: Types & submits search query
Browser->>Browser: Trim input, navigate to /search?q=query
Browser->>Browser: Mount Search page, extract q param
Browser->>API: GET /api/videos/search?q=query&page=1&limit=10
API->>API: Validate params (zod)
API->>API: buildFtsQuery() sanitize & build FTS expression
API->>DB: SELECT * FROM videos_fts JOIN videos JOIN user<br/>WHERE videos_fts MATCH 'query'<br/>ORDER BY rank LIMIT 10
DB->>API: Return matching video rows (id, title, description, etc.)
API->>Browser: JSON { videos: [...] }
Browser->>Browser: Render results grid with thumbnails & links
Browser->>User: Display search results
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsThese MCP integrations need to be re-authenticated in the Integrations settings: Sentry 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 70d2881109
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| <div className="ds-meta" style={{ marginTop: 4 }}> | ||
| {v.channel_username ? ( | ||
| <> | ||
| <Link to={`/channel/${v.channel_username}`}> |
There was a problem hiding this comment.
Placing the channel <Link> inside the outer card <Link> creates nested anchors, which is invalid HTML and leads to unreliable navigation behavior (e.g., clicking the channel name can trigger the parent watch-page navigation instead). This affects users specifically on search results where channel links are shown, and React will also emit DOM nesting warnings.
Useful? React with 👍 / 👎.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements full-text search for videos using SQLite FTS5, adding a new backend search endpoint, a database migration for indexing, and a frontend search interface. Several critical issues were identified regarding schema consistency: the migration and backend queries incorrectly reference a user table and name column instead of the existing users table and display_name column. Additionally, the search results page contains nested Link components, which results in invalid HTML and broken navigation behavior.
| SELECT v.id, v.title, v.description, COALESCE(u.name, '') | ||
| FROM videos v | ||
| LEFT JOIN user u ON u.id = v.user_id | ||
| WHERE v.deleted_at IS NULL; |
There was a problem hiding this comment.
The migration references a table named user and a column named name, but the existing schema defines the table as users and the column as display_name. This will cause the migration to fail.
SELECT v.id, v.title, v.description, COALESCE(u.display_name, '')
FROM videos v
LEFT JOIN users u ON u.id = v.user_id
WHERE v.deleted_at IS NULL;| new.id, | ||
| new.title, | ||
| new.description, | ||
| COALESCE((SELECT name FROM user WHERE id = new.user_id), '') |
There was a problem hiding this comment.
Incorrect table and column names. Should be users and display_name per src/db/schema.sql.
COALESCE((SELECT display_name FROM users WHERE id = new.user_id), '')| DELETE FROM videos_fts WHERE video_id = old.id; | ||
| INSERT INTO videos_fts (video_id, title, description, channel_name) | ||
| SELECT new.id, new.title, new.description, COALESCE(u.name, '') | ||
| FROM (SELECT 1) AS s LEFT JOIN user u ON u.id = new.user_id |
There was a problem hiding this comment.
Incorrect table name. Should be users.
FROM (SELECT 1) AS s LEFT JOIN users u ON u.id = new.user_id| CREATE TRIGGER IF NOT EXISTS user_name_videos_fts | ||
| AFTER UPDATE OF name ON user |
There was a problem hiding this comment.
The trigger is defined on the wrong table and column. It should be display_name on users.
CREATE TRIGGER IF NOT EXISTS user_name_videos_fts
AFTER UPDATE OF display_name ON users| BEGIN | ||
| DELETE FROM videos_fts WHERE video_id IN (SELECT id FROM videos WHERE user_id = new.id); | ||
| INSERT INTO videos_fts (video_id, title, description, channel_name) | ||
| SELECT v.id, v.title, v.description, new.name |
There was a problem hiding this comment.
Incorrect column reference. Should be new.display_name.
SELECT v.id, v.title, v.description, new.display_name| const { results } = await c.env.DB.prepare( | ||
| `SELECT v.id, v.user_id, v.title, v.description, v.stream_video_id, | ||
| v.thumbnail_url, v.view_count, v.created_at, v.updated_at, | ||
| u.name AS channel_name, u.username AS channel_username, |
There was a problem hiding this comment.
The column name does not exist on the users table. It should be display_name.
| u.name AS channel_name, u.username AS channel_username, | |
| u.display_name AS channel_name, u.username AS channel_username, |
| videos_fts.rank AS rank | ||
| FROM videos_fts | ||
| JOIN videos v ON v.id = videos_fts.video_id | ||
| LEFT JOIN user u ON u.id = v.user_id |
There was a problem hiding this comment.
The table name is users, not user.
| LEFT JOIN user u ON u.id = v.user_id | |
| LEFT JOIN users u ON u.id = v.user_id |
| <Link to={`/channel/${v.channel_username}`}> | ||
| {v.channel_name ?? v.channel_username} | ||
| </Link>{' '} |
There was a problem hiding this comment.
You are nesting a Link component inside another Link (the outer link starts at line 80). HTML does not allow nested anchor tags (<a>), which will lead to invalid DOM structures and broken navigation behavior.
2 participants
Summary
videos_ftsover title/description/channel name with triggers that keep it in sync on insert/update/delete and on creator rename. The update trigger only fires for indexed columns so view-count writes don't churn the index.GET /api/videos/search?q=returning ranked, soft-delete-filtered results joined to channel metadata. Query is sanitised and split into prefix-quoted FTS5 tokens (8 max) to makeqsafe./searchresults page using the existing Strand styles.Test plan
npm run lintnpm run type-checknpm testnpm run build/search?q=…once deployed against a D1 with the new migration applied.Closes ALO-150
Generated by Claude Code
Summary by CodeRabbit