Skip to content

Revive search, drive typecheck to zero, and repo hygiene#4

Merged
drftstatic merged 3 commits into
mainfrom
claude/repo-evaluation-9114e6
Jul 19, 2026
Merged

Revive search, drive typecheck to zero, and repo hygiene#4
drftstatic merged 3 commits into
mainfrom
claude/repo-evaluation-9114e6

Conversation

@drftstatic

Copy link
Copy Markdown
Owner

Summary

Reviving the repo after ~11 months idle. It installed, built, and booted on the first try, but the core feature — search — was completely broken in the UI, npm run check failed with 28 type errors, and several hygiene issues had accumulated. This PR fixes all three, verified end-to-end in the browser.

What changed

1. Search works again (the headline fix)

Search requests were aborting themselves before results could render. The client search layer had piled up complexity that fought itself:

  • home.tsx and ResultsGrid each ran their own React Query for the same search, with different query keys
  • a module-global AbortController cancelled requests across those unrelated callers
  • a recursive "precache popular queries" feature fired 3 extra searches through the same pipeline on every user search (also burning Archive.org rate limit)
  • any AbortError was then treated as a fatal "Search failed" state

Fix is a net simplification (−229 / +90 lines in this commit):

  • New useVideoSearch hook is the single owner of the search query; both consumers share one query key so React Query dedupes to one request
  • searchVideos now takes React Query's own AbortSignal and passes it to fetch, distinguishing retryable timeouts from real cancellation
  • Deleted the global controller, the debounce wrapper, and the precache machinery

2. npm run check now passes clean (was 28 errors)

Store and components had drifted apart. Real bugs flushed out in the process:

  • metadata-service: two cleanupExpiredCache implementations existed — the memory-only one silently overwrote the database one, so DB cache cleanup never ran. Merged into one that cleans both; added db-null guards so running without DATABASE_URL is typesafe.
  • CommandPalette rewired to the current store API (setSearchState), add-to-queue now resolves a stream URL first, and the stale trashteam/nulltone theme toggle replaced with a cycle through the real 11 brand skins.
  • PreviewWindow: a local video was shadowing the video prop, breaking a debug log and referencing identifier on an HTMLVideoElement.
  • Removed dead MarioPipeEffect (unimported, referenced nonexistent store state).
  • tsconfig: set target: ES2022 to match the actual runtime (fixes the Map-iteration error).
  • Misc: lucide title props wrapped in <span>, toast retry action as a proper ToastAction element (error-handler.ts.tsx), tuple type for emergency-mix segmentLength, null-vs-undefined in PreviewPanel.

3. Hygiene

  • Stop tracking .env (template only — .env.example remains) and video-cache/ (~96 MB of runtime-cached sample videos). Both now gitignored along with local.db. Files stay on disk; a full history rewrite is left as a separate decision.
  • trust proxy: true1. true let any client spoof X-Forwarded-For and bypass the Archive.org rate limiter — express-rate-limit refused to run with it and logged a ValidationError on every request. Warning is now gone.
  • Removed unused devDependencies: @tailwindcss/vite (v4 plugin never wired up next to Tailwind v3), both @replit vite plugins (already commented out of vite.config), and unreferenced tw-animate-css.
  • Added GitHub Actions CI (typecheck + build on push/PR) so drift can't accumulate invisibly again.

Verification

  • npm run check0 errors
  • npm run build → succeeds (the pre-existing CSS minify warnings are unrelated and untouched)
  • Ran the app in-browser: searched "television" (112 results), "kaleidoscope" (2), "apollo moon" (5 NASA clips) — results render with thumbnails and the empty/failed states behave. The ValidationError on every request is gone.

Notes for the reviewer

  • I did not rewrite git history to purge the large videos from past commits — that's a force-push decision for you to make. The files are untracked going forward.
  • The four-way deployment split (Replit / Vercel / Docker / nginx) and the fact that nothing loads .env at runtime are still open — flagged in the evaluation but out of scope for this "low-hanging fruit" pass.
  • CSS minify warnings during build predate this PR and weren't touched.

🤖 Generated with Claude Code

drftstatic and others added 3 commits July 18, 2026 17:38
Search requests were aborting themselves: home.tsx and ResultsGrid ran
duplicate queries with different keys, a module-global AbortController
cancelled across callers, and a recursive popular-query precache fired
three extra searches through the same pipeline. Any AbortError was then
treated as a fatal search failure.

- Add useVideoSearch hook as the single owner of the search query;
  both consumers share one query key so React Query dedupes to one fetch
- Pass React Query's abort signal through searchVideos to fetch,
  distinguishing retryable timeouts from real cancellation
- Remove the global controller, debounce wrapper, and precache machinery
  (also stops burning Archive.org rate limit on background searches)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
npm run check now passes clean (was 28 errors). Notable real fixes:

- metadata-service: two cleanupExpiredCache implementations existed; the
  memory-only one silently overwrote the database one, so DB cleanup
  never ran. Merged into one that cleans both. Added db-null guards so
  running without DATABASE_URL is typesafe.
- CommandPalette: rewired to the current store API (setSearchState),
  fixed add-to-queue to resolve a stream URL first, and replaced the
  stale trashteam/nulltone theme toggle with a cycle through the real
  11 brand skins.
- PreviewWindow: local videoEl was shadowing the video prop, breaking
  the debug log and referencing identifier on an HTMLVideoElement.
- Removed dead MarioPipeEffect component (unimported, referenced
  nonexistent store state).
- tsconfig: set target ES2022 to match the actual runtime.
- Misc: lucide title props wrapped in spans, toast retry action as a
  proper ToastAction element (error-handler.ts -> .tsx), tuple type for
  emergency mix segmentLength, null-vs-undefined in PreviewPanel.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…dd CI

- Stop tracking .env (template only, .env.example remains) and
  video-cache/ (~96MB of runtime-cached sample videos); both gitignored
  now along with local.db. Files remain on disk; history rewrite left
  as a separate decision.
- trust proxy: true -> 1. 'true' let any client spoof X-Forwarded-For
  and bypass the Archive.org rate limiter (express-rate-limit refused
  to run with it and logged a ValidationError on every request).
- Remove unused devDependencies: @tailwindcss/vite (v4 plugin never
  wired up alongside Tailwind v3), both @replit vite plugins (already
  commented out of vite.config), and unreferenced tw-animate-css.
- Add GitHub Actions CI running typecheck + build on push/PR so drift
  can't accumulate invisibly again.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@vercel

vercel Bot commented Jul 19, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
staticbuffet Ready Ready Preview, Comment Jul 19, 2026 12:59am

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the video search functionality by introducing a centralized useVideoSearch hook that leverages React Query for request deduplication and state synchronization. It also cleans up unused dependencies, improves proxy trust settings in the server, and refactors metadata caching and cleanup. A review comment highlights issues in the new useVideoSearch hook, specifically a stale closure in the results synchronization useEffect due to missing dependencies, and a failure to sync the loading state back to the store during active fetches.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +40 to +57
useEffect(() => {
if (!data) return;
const docs = (data as any).docs || [];
if ((searchState.page || 1) > 1) {
setSearchResults([...useStore.getState().searchResults, ...docs]);
} else {
setSearchResults(docs);
}
setTotalResults((data as any).numFound || 0);
setLoading(false);
}, [data]);

useEffect(() => {
if (error) {
setLoading(false);
console.error('Search error:', error);
}
}, [error]);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

There are two issues with the store synchronization in this hook:

  1. Stale Closure / Missing Dependencies: The first useEffect references searchState.page but only lists [data] in its dependency array. This violates the react-hooks/exhaustive-deps rule. To avoid stale closures and prevent premature execution when searchState.page changes (before the query resolves), we should read the page directly from the store using useStore.getState().searchState.page.
  2. Missing Loading State Sync: The store's loading state is never set to true when a new search or page fetch starts; it is only manually set to false when data or an error is received. This breaks UI elements like the "Loading more results..." spinner at the bottom of the ResultsGrid (which relies on the store's isLoading state).

We can solve both issues elegantly by syncing the query's isFetching state directly to the store via a dedicated useEffect, and removing the manual setLoading(false) calls.

  // Sync loading state to the store
  useEffect(() => {
    setLoading(query.isFetching);
  }, [query.isFetching, setLoading]);

  // Sync results into the store: replace on page 1, append on later pages
  useEffect(() => {
    if (!data) return;
    const docs = (data as any).docs || [];
    const currentPage = useStore.getState().searchState.page || 1;
    if (currentPage > 1) {
      setSearchResults([...useStore.getState().searchResults, ...docs]);
    } else {
      setSearchResults(docs);
    }
    setTotalResults((data as any).numFound || 0);
  }, [data, setSearchResults, setTotalResults]);

  useEffect(() => {
    if (error) {
      console.error('Search error:', error);
    }
  }, [error]);

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bcb04a8a1b

ℹ️ 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".

Comment on lines +43 to +44
if ((searchState.page || 1) > 1) {
setSearchResults([...useStore.getState().searchResults, ...docs]);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Prevent duplicate appends from each search observer

When searchState.page > 1 (for example, Lucky Dip deliberately chooses pages 1–3), both Home and ResultsGrid invoke this hook. React Query shares the fetch, but each hook instance runs this effect; the first appends docs, then the second reads the already-updated Zustand store and appends the same page again. This corrupts the displayed result list with duplicate clips (and is especially visible after Lucky Dip has populated its selected results). Move store synchronization to one component/observer, or make the hook's synchronization idempotent.

Useful? React with 👍 / 👎.

@drftstatic
drftstatic merged commit 151ed6c into main Jul 19, 2026
2 of 3 checks passed
drftstatic added a commit that referenced this pull request Jul 19, 2026
Revive search, drive typecheck to zero, and repo hygiene
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