feat(askgh): allow users to select additional search scopes in repo Ask GitHub view - #1436
feat(askgh): allow users to select additional search scopes in repo Ask GitHub view#1436msukkari wants to merge 3 commits into
Conversation
- Added repos and searchContexts props to LandingPage component - Implemented local storage for persisting selected search scopes - Current repo is pre-selected by default when visiting Ask GH page - Users can now add/remove search scopes using the SearchScopeSelector Fixes SOU-1496 Co-authored-by: Michael Sukkarieh <msukkari@users.noreply.github.com>
WalkthroughThe repository page now loads repositories and search contexts. ChangesAsk GH search scope integration
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant AskGHPage
participant RepositoryActions
participant LandingPage
participant LocalStorage
participant ChatBoxToolbar
participant ChatBox
AskGHPage->>RepositoryActions: Load repositories and search contexts
RepositoryActions-->>AskGHPage: Return search data
AskGHPage->>LandingPage: Pass repos and searchContexts
LandingPage->>LocalStorage: Restore selected scopes
LandingPage->>ChatBoxToolbar: Pass repositories and contexts
ChatBoxToolbar->>LocalStorage: Persist selected scopes
LandingPage->>ChatBox: Pass selected search contexts
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Co-authored-by: Michael Sukkarieh <msukkari@users.noreply.github.com>
|
@brendan-kellam your pull request is missing a changelog! |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit bc78eef. Configure here.
| if (!currentRepoIncluded) { | ||
| setSelectedSearchScopes([defaultRepoScope, ...selectedSearchScopes]); | ||
| } | ||
| }, [hasInitialized, selectedSearchScopes, repoName, defaultRepoScope, setSelectedSearchScopes]); |
There was a problem hiding this comment.
Current repo never auto-added
High Severity
The one-shot hasInitialized effect runs against the pre-hydration default ([defaultRepoScope]), so the current repo always looks included and the add path never runs. After useLocalStorage hydrates with { initializeWithValue: false }, stored scopes without this repo win, and soft navigations between Ask GH pages also skip re-checking because hasInitialized stays true. Visiting another repo's Ask GH page therefore does not auto-include that repo in the search scope.
Reviewed by Cursor Bugbot for commit bc78eef. Configure here.
![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
🧹 Nitpick comments (1)
packages/web/src/app/(app)/askgh/[owner]/[repo]/page.tsx (1)
74-75: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffUse API routes and
react-queryfor these GET requests.Move
getReposandgetSearchContextsbehind GET API routes. Fetch them withreact-query. Keep Server Actions for mutations.As per coding guidelines, “For GET data fetching, prefer API routes with
react-queryover server actions; use server actions for mutations such as POST, PUT, and DELETE instead of data fetching.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/web/src/app/`(app)/askgh/[owner]/[repo]/page.tsx around lines 74 - 75, Replace the direct getRepos and getSearchContexts calls in the page component with GET API routes, then fetch those routes through react-query hooks. Preserve the existing data and loading/error behavior, and keep server actions limited to mutations rather than GET data retrieval.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/web/src/app/`(app)/askgh/[owner]/[repo]/components/landingPage.tsx:
- Around line 60-85: Remove the hasInitialized state and initialization guard
from the effect. Update the selectedSearchScopes effect to run when repoName
changes and use the functional form of setSelectedSearchScopes, adding
defaultRepoScope only when the current repository is absent while preserving
restored scopes.
---
Nitpick comments:
In `@packages/web/src/app/`(app)/askgh/[owner]/[repo]/page.tsx:
- Around line 74-75: Replace the direct getRepos and getSearchContexts calls in
the page component with GET API routes, then fetch those routes through
react-query hooks. Preserve the existing data and loading/error behavior, and
keep server actions limited to mutations rather than GET data retrieval.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0978bf3d-fa1c-4b61-a91b-ea0ac0202b7b
📒 Files selected for processing (2)
packages/web/src/app/(app)/askgh/[owner]/[repo]/components/landingPage.tsxpackages/web/src/app/(app)/askgh/[owner]/[repo]/page.tsx
| // Use local storage for selected scopes, with the current repo as default | ||
| const [selectedSearchScopes, setSelectedSearchScopes] = useLocalStorage<SearchScope[]>( | ||
| ASKGH_SELECTED_SEARCH_SCOPES_LOCAL_STORAGE_KEY, | ||
| [defaultRepoScope], | ||
| { initializeWithValue: false } | ||
| ); | ||
|
|
||
| // Ensure the current repo is always included in selected scopes when visiting this page | ||
| // This handles the case where the user visits a different repo's Ask GH page | ||
| const [hasInitialized, setHasInitialized] = useState(false); | ||
| useEffect(() => { | ||
| if (hasInitialized) { | ||
| return; | ||
| } | ||
| setHasInitialized(true); | ||
|
|
||
| // Check if the current repo is already in the selected scopes | ||
| const currentRepoIncluded = selectedSearchScopes.some( | ||
| (scope) => scope.type === 'repo' && scope.value === repoName | ||
| ); | ||
|
|
||
| // If not, add it to the scopes | ||
| if (!currentRepoIncluded) { | ||
| setSelectedSearchScopes([defaultRepoScope, ...selectedSearchScopes]); | ||
| } | ||
| }, [hasInitialized, selectedSearchScopes, repoName, defaultRepoScope, setSelectedSearchScopes]); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
ast-grep outline 'packages/web/src/app/(app)/askgh/[owner]/[repo]/components/landingPage.tsx' --items all
rg -n -C 8 'initializeWithValue|hasInitialized|selectedSearchScopes|setSelectedSearchScopes' \
'packages/web/src/app/(app)/askgh/[owner]/[repo]/components/landingPage.tsx'
fd -a -i '^(package\.json|pnpm-lock\.yaml)$' . -x rg -n -C 2 'usehooks-ts'Repository: sourcebot-dev/sourcebot
Length of output: 6774
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== package manager lock section for usehooks-ts =="
python3 - <<'PY'
from pathlib import Path
for p in Path('.').rglob('pnpm-lock.yaml'):
print(f'--- {p} ---')
lines = p.read_text(encoding='utf-8').splitlines()
for i,l in enumerate(lines):
if '"usehooks-ts"' in l or '- usehooks-ts' in l or 'usehooks-ts@' in l:
for j in range(max(0,i-5), min(len(lines),i+15)):
print(f'{j+1}: {lines[j]}')
PY
echo "== local storage usage around scope merging =="
sed -n '40,145p' 'packages/web/src/app/(app)/askgh/[owner]/[repo]/components/landingPage.tsx'
echo "== dependency files mentioning usehooks-ts =="
fd -a -i '^(package\.json|pnpm-lock\.yaml|package-lock.json|yarn.lock|bun.lockb)$' . -x bash -c 'echo "--- $1 ---"; rg -n "usehooks-ts" "$1" || true' sh {}Repository: sourcebot-dev/sourcebot
Length of output: 6281
🌐 Web query:
usehooks-ts useLocalStorage initializeWithValue false behavior first returns initialValue then restores localStorage
💡 Result:
In the usehooks-ts library, the initializeWithValue: false option is designed primarily for Server-Side Rendering (SSR) environments to prevent hydration mismatches [1][2][3]. When initializeWithValue is set to false, the hook behaves as follows: 1. Initial Value Assignment: Upon the first render, the hook ignores any data currently in localStorage and instead initializes the state using the provided initialValue [4][5]. 2. Subsequent Synchronization: The hook includes a useEffect that runs after the initial mount [2]. This effect triggers a re-read of the localStorage key [2]. If data exists in localStorage, the state is updated to reflect that stored value, effectively "restoring" or synchronizing the hook with the persisted data once it is safe to access the browser environment [2]. This mechanism ensures that the server-rendered HTML matches the initial client-side render (using initialValue), avoiding errors where the server cannot access window.localStorage or produces a different initial value than the client [1][2][3]. Once the component is mounted on the client, the useEffect catches up and updates the state with the actual localStorage content [2].
Citations:
- 1: [BUG] useLocalStorage Hydration Mismatch — Need to setIsClient everywhere juliencrn/usehooks-ts#644
- 2: https://usehooks-ts.com/react-hook/use-read-local-storage
- 3: https://docsearch.algolia.com/mcp/docs/repo/juliencrn/usehooks-ts
- 4: https://usehooks-ts.com/react-hook/use-local-storage
- 5: https://ts-hooks-kit.netlify.app/react-hook/use-local-storage
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== packages/web package.json "
sed -n '1,240p' packages/web/package.json
echo "== yarn.lock usehooks-ts resolved package info =="
sed -n '23430,23445p' yarn.lock
echo "== find installed usehooks-ts package.json =="
fd -a -i '^package.json$' . \
-x sh -c 'if rg -q "\"usehooks-ts\"" "$1/yarn.lock" && rg -q "\"usehooks-ts\"" "$1/package.json"; then echo "--- $1 ---"; fi' sh {} \;
echo "== search useLocalStorage implementation in installed usehooks-ts =="
python3 - <<'PY'
from pathlib import Path
matches=[]
for f in Path('.').rglob('useLocalStorage.ts'):
p=str(f)
if 'node_modules' not in p:
continue
text=f.read_text(errors='ignore')
if 'initializeWithValue' in text:
matches.append(p)
for p in matches[:20]:
print("---", p, "---")
lines=open(p, errors='ignore').readlines()
for i,l in enumerate(lines,1):
if 'initializeWithValue' in l or 'useEffect' in l or 'addEventListener' in l:
print(f'{i}: {l.rstrip()}')
PYRepository: sourcebot-dev/sourcebot
Length of output: 11253
🌐 Web query:
github juliencrn usehooks-ts 3.1.1 packages use-local-storage source useLocalStorage initializeWithValue false
💡 Result:
In version 3.1.1 of the usehooks-ts library, the useLocalStorage hook includes an optional parameter called initializeWithValue within its options object to support Server-Side Rendering (SSR) [1][2]. When initializeWithValue is set to false, the hook will not attempt to read from localStorage during the initial render [1]. Instead, it returns the provided initial value, which prevents hydration mismatch errors in SSR environments where the server does not have access to the browser's localStorage API [1]. The signature of useLocalStorage in this version is: export function useLocalStorage( key: string, initialValue: T | ( => T), options: UseLocalStorageOptions = {},): [T, Dispatch<SetStateAction>, => void] By default, initializeWithValue is set to true [1]. To disable it, you can pass the option as follows: const [value, setValue] = useLocalStorage('my-key', initialValue, { initializeWithValue: false });
Citations:
- 1: https://usehooks-ts.com/react-hook/use-local-storage
- 2: https://npmx.dev/package-changelog/usehooks-ts/v/3.1.1
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
for f in Path('.').rglob('useLocalStorage.ts'):
p = str(f)
if 'node_modules' not in p and 'dist' not in p:
continue
text = f.read_text(errors='ignore')
lines = text.splitlines()
ok = any('initializeWithValue' in line for line in lines)
print(f'--- {p} ok={ok} line_count={len(lines)} ---')
if ok:
for i,l in enumerate(lines, 1):
if any(k in l for k in ['useStorageState', 'localStorage', 'initializeWithValue', 'addEventListener', 'useEffect']):
print(f'{i}: {l}')
# Print focused region around hook definition/body
for start in range(len(lines)):
if 'function useLocalStorage' in lines[start] or 'const useLocalStorage' in lines[start]:
end = min(len(lines), start + 160)
for j in range(start, end):
print(f'{j+1}: {lines[j]}')
break
PY
echo "== fetch upstream 3.1.1 useLocalStorage.ts =="
python3 - <<'PY'
import urllib.request
url='https://raw.githubusercontent.com/juliencrn/usehooks-ts/v3.1.1/packages/use-local-storage/src/useLocalStorage.ts'
try:
text=urllib.request.urlopen(url, timeout=20).read().decode()
for i,l in enumerate(text.splitlines(),1):
if any(s in l for s in ['initializeWithValue', 'useStorageState', 'localStorage', 'addEventListener', 'useEffect']):
print(f'{i}: {l}')
except Exception as e:
print('FETCH_ERROR', repr(e))
PYRepository: sourcebot-dev/sourcebot
Length of output: 379
🌐 Web query:
github juliencrn usehooks-ts v3.1.1 packages use-local-storage src useLocalStorage.ts initializeWithValue useEffect JSON.parse
💡 Result:
In the usehooks-ts library version 3.1.1, the useLocalStorage hook is designed to persist state in the browser's localStorage using JSON serialization [1]. The implementation handles initialization, reading, and synchronization through the following key mechanisms: initializeWithValue: The hook accepts an optional initializeWithValue parameter (defaulting to true) [1]. When set to true, the hook reads from localStorage during the initial state initialization [1]. If false, it ignores localStorage during the initial render and uses the provided initialValue instead, which helps prevent hydration mismatches in Server-Side Rendering (SSR) environments [1][2]. JSON.parse and Serialization: By default, the hook uses JSON.stringify for serialization and JSON.parse for deserialization [1]. The deserializer function is wrapped in a try-catch block; if JSON.parse fails, it logs an error and returns the provided initialValue as a fallback [1]. It also explicitly handles the string 'undefined' by returning the value undefined [1]. Users can override these defaults by providing custom serializer and deserializer functions in the options object [1]. useEffect and State Management: The hook utilizes useState to hold the current value and uses the localStorage API to persist updates [1]. While the provided source reflects the core logic of readValue being called during initialization via useState, the hook ensures that localStorage operations are safely executed in client-side environments by checking for the existence of the window object (or an equivalent server-side check) to avoid errors during server-side builds [1]. [1]
Citations:
- 1: https://usehooks-ts.com/react-hook/use-local-storage
- 2: https://npmx.dev/package-changelog/usehooks-ts/v/3.1.1
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
curl -fsSL --connect-timeout 15 https://registry.npmjs.org/usehooks-ts/3.1.1 \
| jq -r '.dist.tarball' \
| xargs -r curl -fsSL --connect-timeout 15 \
| tar -xzO --no-same-owner --wildcards '*useLocalStorage.ts' \
| sed -n '1,220p'Repository: sourcebot-dev/sourcebot
Length of output: 264
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
curl -fsSL --connect-timeout 15 https://registry.npmjs.org/usehooks-ts/3.1.1 \
| jq -r '.dist.tarball' \
| xargs -r curl -fsSL --connect-timeout 15 \
| tar -tz \
| grep -E 'use-hooks-ts|useLocalStorage|package' | head -80Repository: sourcebot-dev/sourcebot
Length of output: 310
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
tmp="$(mktemp -d)"
curl -fsSL --connect-timeout 15 https://registry.npmjs.org/usehooks-ts/3.1.1 \
| jq -r '.dist.tarball' \
| xargs curl -fsSL --connect-timeout 15 -o "$tmp/package.tgz"
echo "== build/useLocalStorage references =="
python3 - <<'PY'
import tarfile, re, subprocess, tempfile, sys
path=sys.argv[1]
with tarfile.open(path, "r:gz") as t:
names=[n for n in t.getnames() if "local" in n.lower() or n.endswith(("js","cts","d.ts","d.cts"))]
for n in names:
try:
data=t.extractfile(n).read().decode(errors="ignore")
except Exception as e:
print(n, "ERR", repr(e))
continue
if "initializeWithValue" in data or "useLocalStorage" in data:
print(f"--- {n} ---")
for i,line in enumerate(data.splitlines(),1):
if any(k in line for k in ["initializeWithValue", "useLocalStorage", "useEffect", "localStorage", "useStorageState"]):
print(f"{i}: {line}")
py "$tmp/package.tgz"
echo "== JS module snippet around useLocalStorage exports =="
python3 - <<'PY'
import tarfile, sys
path=sys.argv[1]
with tarfile.open(path, "r:gz") as t:
for n in ["package/dist/index.js","package/dist/index.cts","package/dist/index.cjs","package/dist/index.d.ts","package/dist/index.d.cts"]:
try:
data=t.extractfile(n).read().decode(errors="ignore")
except Exception:
continue
if "local" in data.lower():
print(f"--- {n} ---")
for i,line in enumerate(data.splitlines(),1):
if "local" in line.lower() or "export" in line and ("useLocalStorage" in line or "useReadLocalStorage" in line):
print(f"{i}: {line}")
text=' '.join(data.split())
start=text.find("useLocalStorage")
if start!=-1:
print("--- snippet ---")
print(text[max(0,start-800):start+1600])
py "$tmp/package.tgz"Repository: sourcebot-dev/sourcebot
Length of output: 376
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
tmp="$(mktemp -d)"
curl -fsSL -- connect-timeout 15 https://registry.npmjs.org/usehooks-ts/3.1.1 \
| jq -r '.dist.tarball' \
| xargs curl -fsSL --connect-timeout 15 -o "$tmp/package.tgz"
echo "== build/useLocalStorage references =="
python3 <<'PY' "$tmp/package.tgz"
import tarfile, sys
path=sys.argv[1]
with tarfile.open(path, "r:gz") as t:
for n in t.getnames():
try:
data=t.extractfile(n).read().decode(errors="ignore")
except Exception as e:
continue
if "initializeWithValue" in data or "useLocalStorage" in data:
print(f"--- {n} ---")
for i,line in enumerate(data.splitlines(),1):
if any(k in line for k in ["initializeWithValue", "useLocalStorage", "useEffect", "localStorage", "useStorageState"]):
print(f"{i}: {line}")
PY
echo "== JS module snippet around useLocalStorage exports =="
python3 <<'PY' "$tmp/package.tgz"
import tarfile, sys
path=sys.argv[1]
with tarfile.open(path, "r:gz") as t:
for n in ["package/dist/index.js","package/dist/index.cts","package/dist/index.cjs","package/dist/index.d.ts","package/dist/index.d.cts"]:
try:
data=t.extractfile(n).read().decode(errors="ignore")
except Exception:
continue
if "local" in data.lower():
print(f"--- {n} ---")
for i,line in enumerate(data.splitlines(),1):
if "local" in line.lower() or ("export" in line and ("useLocalStorage" in line or "useReadLocalStorage" in line)):
print(f"{i}: {line}")
text=' '.join(data.split())
start=text.find("useLocalStorage")
if start!=-1:
print("--- snippet ---")
print(text[max(0,start-800):start+1600])
PYRepository: sourcebot-dev/sourcebot
Length of output: 153
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
tmp="$(mktemp -d)"
curl -L https://codeload.github.com/juliencrn/usehooks-ts/tar.gz/v3.1.1 -o "$tmp/tar.gz"
tar -xzO -f "$tmp/tar.gz" "usehooks-ts-v3.1.1/packages/use-local-storage/src/useLocalStorage.ts" | sed -n '1,240p'
echo "== installed usehooks source if present =="
if [ -f node_modules/usehooks-ts/usehooks-ts-v3.1.1/packages/use-local-storage/src/useLocalStorage.ts ]; then
sed -n '1,240p' node_modules/usehooks-ts/usehooks-ts-v3.1.1/packages/use-local-storage/src/useLocalStorage.ts
else
echo "not present"
fiRepository: sourcebot-dev/sourcebot
Length of output: 577
Remove the initialization gate.
With useLocalStorage(..., { initializeWithValue: false }), the initial render uses the default scope before local storage restores. This effect then runs once on the default value and skips persisted scopes; after that, it cannot add the current repository when repoName changes. Run the merge on each repoName change with a functional setter.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/web/src/app/`(app)/askgh/[owner]/[repo]/components/landingPage.tsx
around lines 60 - 85, Remove the hasInitialized state and initialization guard
from the effect. Update the selectedSearchScopes effect to run when repoName
changes and use the functional form of setSelectedSearchScopes, adding
defaultRepoScope only when the current repository is absent while preserving
restored scopes.
3 participants
Fixes SOU-1496
Summary
This PR allows users to select additional search scopes (repositories and search contexts) in the Ask GitHub view (
/askgh/[owner]/[repo]). Previously, the search scope was hardcoded to only the current repository, and users could not modify it.Changes
page.tsx: Added calls togetRepos()andgetSearchContexts()to fetch available repositories and search contexts, passing them to theLandingPagecomponent.landingPage.tsx:reposandsearchContextsprops to theLandingPagePropsinterfaceaskGhSelectedSearchScopes)onSelectedSearchScopesChangehandler to allow users to modify scopesChatBoxandChatBoxToolbarcomponentsBehavior
Linear Issue: SOU-1496
Summary by CodeRabbit
New Features
Bug Fixes