Fix backend-agent initialization for server-backed repositories#199
Fix backend-agent initialization for server-backed repositories#199madmansn0w wants to merge 3 commits into
Conversation
|
@madmansn0w is attempting to deploy a commit to the NexusCore Team on Vercel. A member of the Team first needs to authorize it. |
reversTeam
left a comment
There was a problem hiding this comment.
Thanks for the thorough PR, @madmansn0w! The problem statement and root-cause analysis are really clear. A few observations:
What looks good:
- The
isServerModeflag is a clean way to bifurcate the two paths. - Passing
baseUrldirectly intohandleServerConnectto avoid the React state timing race is the right call. - Guarding
startEmbeddingswithisServerModeearly-return is simple and effective. - The dependency arrays on
useCallbackare correctly updated throughout.
Issues / suggestions:
-
Inconsistent indentation in
initializeBackendAgent— The function body (lines starting atconst api = apiRef.current) is indented at 2 spaces instead of the 4 spaces used by the rest ofuseAppState.tsx. This looks like it was accidentally de-indented during editing. -
Inconsistent indentation in
switchRepo— Around theinitializeBackendAgentcall insideswitchRepo, the indentation is mixed (some lines at 10 spaces, the closing brace at 8). Should be consistently 6 spaces to match the surroundingtryblock. -
Duplicate
setServerBaseUrl(baseUrl)call — In the auto-connect flow (useEffect),setServerBaseUrl(baseUrl)is called on line ~94 insidehandleServerConnect, and then again on line ~94 of the.then()callback. The second call is redundant sincehandleServerConnectalready sets it. -
setEmbeddingStatus('ready')inswitchRepo— When switching repos in server mode, you set embedding status to'ready'directly. This is a reasonable shortcut since the backend handles search, but it could be confusing if other code checksembeddingStatus === 'ready'to mean "local embeddings are loaded." A comment explaining why this is safe would help future readers. -
Lock file churn — The
package-lock.jsondiff is large (~1000 lines) with esbuild version bumps. If these dependency updates aren't intentional, consider reverting the lock file to keep the diff focused. -
No tests — Understandable for a React state hook, but the PR mentions known follow-up issues (404s on
/api/query, 401 on auth). It would be good to track those as separate issues.
Overall this is a solid fix for a real pain point. The approach is sound — just needs the formatting cleanup and the duplicate setServerBaseUrl removed.
|
Addressed in this update:
Tracked follow-ups requested in review:
|
|
|
Please submit a new PR if this is still relevant |
Summary
This PR fixes the web app flow for server-backed repositories.
Previously, when connecting the web UI to a locally served GitNexus backend, the app still attempted to use the browser-local agent / embedding pipeline path. That caused failures such as:
Database not ready. Please load a repository first.Initializing AgentRoot cause
The web app had two different repository modes, but the initialization path was still mixing them:
Browser-ingested repo mode
Server-backed repo mode
However, server-backed repositories were still calling the local initialization path in some cases, and the initial server connect flow also had a race where backend agent initialization could happen before
serverBaseUrlstate was available.Changes
isServerModestate to app stateinitializeBackendAgent(...)to app stateResult
The web UI can now connect to a served backend repository without trying to run the browser-local embedding/database pipeline against a non-local repo.
This fixes the frontend/server-mode integration issue that produced:
Database not ready. Please load a repository first.Validation
Manually validated:
Follow-ups / known issues
Not fixed in this PR:
404for some/api/query-backed helper calls401 User not founddepending on local LLM settings